Commit 92938bd
committed
Don't stop scan on invalid inline property annotation
Follow up on 3629, which was merged for PHPCS 3.8.0.
PR 3629 added logic to throw a "Ruleset invalid. Property \"$propertyName\" does not exist on sniff ..." error.
This error is intended for the command-line when reading the `phpcs.xml.dist` ruleset file.
However, this error could _also_ be encountered if an inline `// phpcs:set ...` annotation would try to set a non-existent property.
While the use of `// phpcs:set` is typically reserved for sniff test case files, there is nothing stopping end-users from using the annotation.
The net-effect would be:
* The `Ruleset::setSniffProperty()` throws a `RuntimeException`.
* This exception is then passed to `File::addMessage()` where it is **not** thrown as the line on which the error is being thrown is an annotation line.
* The scan of the file stops dead in its tracks as a `RuntimeException` was encountered.
* The end-user doesn't know the file does not finish scanning as no `Internal` error is shown for the file.
To me, this is counter-intuitive and counter-productive as it may give people a false sense of security (CI is green, while in reality files are not being scanned).
To fix this, I propose the following:
* Collect all `// phpcs:set` related inline annotations encountered while scanning.
* Do **not** stop the file scan for these errors.
* Add a warning with information about the incorrect annotations on line 1 once the file has finished scanning.
Includes a test via the `Generic.PHP.BacktickOperator` sniff.1 parent 77cee16 commit 92938bd
File tree
3 files changed
+29
-2
lines changed- src
- Files
- Standards/Generic/Tests/PHP
3 files changed
+29
-2
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
343 | 343 | | |
344 | 344 | | |
345 | 345 | | |
| 346 | + | |
346 | 347 | | |
347 | 348 | | |
348 | 349 | | |
| |||
411 | 412 | | |
412 | 413 | | |
413 | 414 | | |
414 | | - | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
415 | 424 | | |
416 | 425 | | |
417 | 426 | | |
| |||
536 | 545 | | |
537 | 546 | | |
538 | 547 | | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
539 | 555 | | |
540 | 556 | | |
541 | 557 | | |
| |||
Lines changed: 7 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
Lines changed: 5 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
29 | 32 | | |
30 | 33 | | |
31 | 34 | | |
| |||
40 | 43 | | |
41 | 44 | | |
42 | 45 | | |
| 46 | + | |
43 | 47 | | |
44 | 48 | | |
45 | 49 | | |
| |||
0 commit comments