Custom localfile #3956
Closed
Custom localfile #3956
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tabsintoken-add-tests Tokenizer::replaceTabsInToken(): add tests
.. to document and safeguard the existing behaviour. Includes skipping one particular assertion on PHP 5.4. This assertion would fail on PHP 5.4 with PHPUnit 4 as PHPUnit polyfills the `T_YIELD` token too, but with a different value, which causes the token 'type' to be set to `UNKNOWN`. This issue _only_ occurs when running the tests, not when running PHPCS outside of a test situation, so this is not a real problem when running PHPCS on PHP 5.4. For reference: the PHPUnit polyfilled token is declared in the PHP_CodeCoverage_Report_HTML_Renderer_File class in vendor/phpunit/php-code-coverage/src/CodeCoverage/Report/HTML/Renderer/File.php
By moving the check a `T_STRING` "yield" keyword up and always adjusting the token in the original token stream, we can remove a lot of duplicate code.
On PHP 5.4, where PHP doesn't natively have the `T_YIELD` token yet, a `yield` function name for a function declared to return by reference would be tokenized as `T_YIELD`, instead of `T_STRING` due to the `T_YIELD` polyfill happening _after_ the context sensitive keyword check has already run. By moving the check for a `T_STRING` "yield" keyword up to above the check for context sensitive keywords, this bug is fixed. Includes additional test.
…ld-from-add-tests+minor-bug-fix Tokenizer/PHP: add tests for tokenization of yield and yield from + minor bug fix
Improve the `YieldTest` by also verifying the _contents_ of the matched token. This is particularly needed for the `yield from` tests, where we need to verify that `yield` and `from` keywords are joined into one token on PHP 5.4-5.6.
…sts-iterate Tokenizers/YieldTest: improve tests
* Add extra tests to improve code coverage * Add comment to last test in case file 3 to clarify that it must be the last in the file.
Doing this to be able to create tests with syntax errors in additional, separate test case files.
This sniff only listens to `T_CLASS`. It does not listen to `T_ANON_CLASS`. So the removed code comment is incorrect. The if condition is still valid to bail early when live coding, but it does not ever apply to anonymous classes.
…ests Test improvements: * Adjust some existing tests to have comments and new lines in unexpected places. * Adjust some existing tests to include extended classes and implemented interfaces. * Add tests with `final` and `readonly` class modifier keywords. * Make some tests more descriptive by making the class name reflect what is being tested. * Add a few comments to pre-existing tests to clarify why they exist. * Add a separate test case file with a live coding/parse error test. * Remove some code snippets which weren't testing anything. Clean up the test file: * Remove redundant whitespace in the test case file. * Use proper punctuation in comments in test case file.
…s-name-prefix Generic/AbstractClassNamePrefix: improve code coverage
unreachable condition This commit reverts the changes to the `Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence` sniff introduced in PHPCSStandards/PHPCSExtra@7b38efb. The sniff's tests remain relevant, so they were preserved. The original commit was added to fix false positives that the sniff was triggering when handling boolean operators inside a match (see PHPCSStandards/PHPCSExtra#271 (review) -1634348864 and PHPCSStandards/PHPCSExtra#271 (comment) ). Example: ```php match (true) { $a || ($b && $c) => true, }; ``` I believe the false positive was actually caused by a bug in `File::findStartOfStatement()`. This bug was then fixed in PHPCSStandards/PHP_CodeSniffer@b82438f which rendered the changes to the sniff itself unnecessary and the removed condition unreachable. Before this fix, when processing the code example above, `File::findStartOfStatement()` returned the variable `$a` as the start of the statement for the `&&` boolean operator. This meant that `$previous` would be set to `||` and the removed condition would be needed to ensure the sniff would bail instead of triggering an error. After this fix, `File::findStartOfStatement()` returns `$b` as the start of the statement and then `$previous` is set to `false` and the sniff bails before reaching the removed condition. Including `Tokens::$blockOpeners` in `RequireExplicitBooleanOperatorPrecedenceSniff::$searchTargets` was necessary only for the removed condition, so it was removed as well.
* This sniff only listens to `T_FUNCTION`. It does not listen to
`T_FN` or `T_CLOSURE`. So the inline code comments were incorrect. The if
conditions are still valid to bail early when live coding, but they do not
ever apply to closures or arrow functions (anymore - it was possible in the past though).
* The inline comments regarding leading underscores was inaccurate as the code
removes all leading underscores, not just the first.
Doing this to be able to create tests with syntax errors on separate files.
…essary return The `return;` is unnecessary as the last statement in a method. Test improvements: * Add separate test case files with live coding/parse error tests. * Add tests verifying sand safeguarding that the name of methods in interfaces is also checked. * Add tests covering the public `strict` property which can be toggled from within rulesets.
…nction-name Generic/CamelCapsFunctionName: improve code coverage
New `Generic.Strings.UnnecessaryHeredoc` sniff which encourages the use of nowdocs instead of heredocs, when there is no interpolation or expressions in the body text. This sniff will hopefully help with the PERCS work as it intends to cover the following PER rule: > A nowdoc SHOULD be used wherever possible. Heredoc MAY be used when a nowdoc does not satisfy requirements. Includes fixer. Includes tests. Includes metrics. Includes XML docs.
This commit fixes a minor bug in `Generic.NamingConventions .ConstructorName` when checking if a given class has a parent. The code was checking if the lower case version of the value returned by `File::findExtendedClassName()`` is `false`. The problem is that `strtolower()` never returns `false`, it always returns a `string`. Thus, the condition would never evaluate to `true` and the sniff would not bail at this point when a given class has no parent. This did not cause any issues to the sniff, even for invalid code, as there is not a scenario where a class method can have a `T_DOUBLE_COLON` token followed by a `T_STRING` token with an empty `content`. This is true even for empty strings as PHPCS includes the quotes in the `content`.
- Remove redundant tests. - Remove unnecessary whitespaces. - Use proper punctuation in a code comment. - Make some tests more descriptive by making the class name reflect what is being tested.
Doing this to be able to create tests with syntax errors in additional, separate test case files.
- Add a separate test case file with a live coding/parse error test. - Add a new test with comments and new lines in unexpected places and parent interfaces.
This commit just adds an inline comment documenting why the sniff bails early if there is no interface name. Doing this to follow the same pattern used by other similar sniffs like AbstractClassName https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/8a7ee56347bb4017ffc778a42a532cbb1651a20b/src/Standards/Generic/Sniffs/NamingConventions/AbstractClassNamePrefixSniff.php#L48
#627) This commit changes `Squiz.Commenting.PostStatementComment` to use a different error code when an annotation is found. This will allow ruleset maintainers to selectively exclude the flagging of trailing annotations from their ruleset. For example, for PHPCS itself, this will make it possible to use the `// @codeCoverageIgnore` annotation where needed. An annotation is defined as any comment that starts with an optional space, followed by a `@` and any character, as long as it is not a whitespace. For now, only `//` comments are supported as this is the only type of comment supported by this sniff. This change only applies to PHP code as support for JS will be dropped soon, and JS doesn't seem to follow the same convention of annotations starting with `@`.
Includes adding a few PHPUnit annotations (as we now can).
…or-tweak PHPCS ruleset: allow for trailing annotations
Generic/ConstructorName: fix minor bug
…nd comments When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the _next_ token after a double colon, while the next token may be whitespace or a comment, which should be ignored. Fixed now by making the sniff check for the _next non empty_ token instead. Includes tests.
…ther class When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the _next_ token after a double colon, disregarding completely the class the method is being called on. This would lead to false positives for method calls to other classes, which would just happen to have a method named the same as the class being extended. This commit fixes this by verifying that the previous non-empty token before the double colon is either a hierarchy keyword or the name of the class being extended and only throwing an error if that's the case. Includes tests.
As things were, the `$startIndex` would be the `function` keyword in the function declaration, which means that the complete declaration (parameters, defaults etc) would be walked, while we only really want to look _inside_ the function body. Fixed by starting the `while` loop on the `scope_opener` instead. Additionally, while the `$startIndex` would be moved forward on each loop, it would still examine one token too much (`scope_opener` cannot be a `T_DOUBLE_COLON`, `T_STRING` after `T_DOUBLE_COLON` cannot be a `T_DOUBLE_COLON`). Also fixed now.
For release this Wednesday (or earlier).
Changelog for the 3.11.2 release
Apparently, by default, the GH CLI doesn't show any output when run from a GH Actions step. This can be confusing and it makes debugging the workflow harder as, in case of failure, it is unclear what the step failed on. The `GH_FORCE_TTY` environment variable (set to any value) should enable the normal output for the GH CLI command, which should fix this. Refs: * cli/cli#10047 * https://cli.github.com/manual/gh_help_environment
Tests taken from PHPCSStandards/PHP_CodeSniffer#630 (comment) Props michalbundyra
…release-tweak GH Actions/verify-release: show output for release attestations
…-constructor Ruleset::__construct(): add tests
…enizer-dnf-extra-tests Tokenizer/PHP: add extra tests for DNF type tokenization
There is no inline version of a `switch` in PHP and JS so there is no reason for this sniff to listen to `T_SWITCH` tokens. Before this change, the sniff would bail early on the line below as a switch always has a scope opener, so this change should not impact in any way the behavior of the sniff. https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/9a0c2546ea2fa7aac19881da7b655cc5f022bc10/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php#L71 The InlineControlStructure sniff started listening for `T_SWITCH` tokens since the commit that added it to PHPCS: PHPCSStandards/PHP_CodeSniffer@ad96eb#diff-e1ea4eabd79d6324057bbf726a27074250478f87d92c11a3725a97f0afbd5513R50 Some of the sniff tests using the `T_SWITCH` token were added to protect against regressions for changes in the tokenizer. For those, dedicated tokenizer tests will be created in subsequent commits. Those commits will also update the related sniff tests to use different control structures other than `T_SWITCH` when appropriate. The modified JS test lost part of its value over time as now the sniff bails early if there is no opening parenthesis after the control structure. Ideally, it should be moved to a tokenizer test, but since there are no tests for the JS tokenizer and support will be dropped in PHPCS 4.0, I opted to simply use a control structure other than T_SWITCH. This test was originally added in b3f4f83. References: - PHP: https://www.php.net/manual/en/control-structures.switch.php - JS: https://tc39.es/ecma262/multipage/ecmascript-language-statements-and-declarations.html
This commit copies, with some non-structural modifications, a sniff test to the tokenizer tests. The original test was added in b24b96b, part of #497. b24b96b introduced a test to InlineControlStructureUnitTest.php as there were no Tokenizer tests back then. It was added to ensure that Tokenizer::recurseScopeMap() would correctly add scope information to the `T_SWITCH` token when handling a `switch` that uses the alternative syntax. This commit also adds a test to ensure the scope is correctly set when using the normal switch syntax. This was not part of b24b96b, but it is relevant anyway, as no other tokenizer test covers this part. Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH` token anymore (see baa4f65), the original test added in b24b96b became useless and thus was refactored to use a different control structure. The sniff test was kept as testing code that uses a control structure, and opening and closing PHP tags is an interesting case not covered in other tests.
This commit copies, with some non-structural modifications, a sniff test to the tokenizer tests. Two new tokenizer tests are created as the result of this copy. One to ensure that the scope of `T_CASE` is correctly set when the case shares the scope closer with `T_SWITCH` (no `T_BREAK`). And another to ensure that the scope of `T_IF` is correctly set when there is a nested `T_SWITCH` and `T_CASE` without a `T_BREAK`. The original sniff test was added in fddc61a, which is part of #497. fddc61a introduced a test to InlineControlStructureUnitTest.php as there were no Tokenizer tests back then. The test was added to ensure that Tokenizer::recurseScopeMap() correctly adds scope information to the `T_CASE` token when it shares the closer with `T_SWITCH` and to `T_IF` when a nested `T_CASE` shares the scope closer with T_SWITCH`. Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH` token anymore (see baa4f65), the original test added in fddc61a became useless and thus was refactored to use a different control structure. This new version of the sniff test was kept as testing code that uses nested alternative syntax control structures is still valid and is not covered in other tests.
This commit copies, with some modifications, a sniff test to the tokenizer tests. It ensures that the scope opener is set correctly for T_SWITCH when there is a closure in the condition. This safeguards against regressions related to 30c618e, which fixed https://github .com//issues/543. 30c618e originally added a test to InlineControlStructureUnitTest.php as there were no Tokenizer tests back then. Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH` token anymore (see baa4f65), the original test added in 30c618e became useless and thus was refactored to use a different control structure. This new version of the sniff test was kept as testing code that uses a closure in the condition is still an interesting case and is not covered in other tests.
This commit expands the existing `Tokenizer::recurseScopeMap()` tests for the `case` keyword when used in a switch statement to check the value of the `scope_condition`, `scope_opener` and `scope_closer` indexes besides checking if they are set. This is needed for a new test case that will be added in a subsequent commit.
This commit copies a sniff test from InlineControlStructureUnitTest.php to the `Tokenizer::recurseScopeMap()` tests for the case keyword. This test was added in b498dbe before the Tokenizer tests were created. It ensures that the scope for the `T_CASE` token is correctly set when there is an if/elseif/else with and without braces inside it. Note that this test currently does not fail even when the changes to the tokenizer applied in b498dbe are reverted. I'm assuming that a subsequent commit further improved the tokenizer and made the changes from b498dbe redundant, but I was not able to find the specific commit using `git bissect`. That being said, I was able to confirm that before b498dbe, the scope closer for the `T_CASE` token was incorrectly set to `T_RETURN` instead of `T_BREAK`. The original sniff test was kept without any modification as testing if/elseif/else with and without curly braces inside another control structure is still valid.
This commit copies a sniff test from InlineControlStructureUnitTest.php to the Tokenizer::recurseScopeMap() tests for the `case` keyword. The copied test was added in 65ef053 before the Tokenizer tests were created. It ensures that the scope for the `T_CASE` token is correctly set when there is an inline control structure inside it with more than three lines. The original sniff test was modified to use `T_FOR` instead of `T_SWITCH`, but its purpose was not altered. The test is about adding braces to an `if` that returns a nested function call.
…remove-switch Generic/InlineControlStructure: stop listening for T_SWITCH
…-update-copyright-year Squiz/FileComment: update year in test case fixed file
…ding During live coding or for code containing a parse error, it was possible for the tokenizer to run into an `Undefined array key "parenthesis_closer"` error when trying to verify arrow functions. As this error happens during the tokenization, this resulted in PHPCS silently not scanning the file - without even showing the error notice. Fixed now. Includes tests.
…Spacing This commit resolves the a fixer conflict between `Squiz.Functions.FunctionDeclarationArgumentSpacing` and `Squiz.WhiteSpace.SuperfluousWhitespace`. This is done by changing the logic within the `FunctionDeclarationArgumentSpacing` sniff to no longer look at (the length of) only one `T_WHITESPACE` token, but instead consider all `T_WHITESPACE` tokens between the type and parameter tokens, and validate on their content (not just length). As part of this, the error message has been changed slightly in some circumstances. The error code has not been changed. * When there are only space characters between the two tokens, the error message remains unchanged. * When there are tabs or newlines included between the two tokens, these are rendered as part of the error message.
…iz.Functions.FunctionDeclarationArgumentSpacing Fix fixer conflict: `PSR12` / `Squiz.Functions.FunctionDeclarationArgumentSpacing`
…vent-undefined-array-key-notice Tokenizer/PHP: prevent an "Undefined array key" notice during live coding
…ace line (#784) The fixer in Squiz.WhiteSpace.FunctionSpacing gets confused when there is a doc comment on the closing brace line. Generally speaking this sniff doesn't care about other content on the same line as the function declaration. But in the case of a "trailing" docblock, this meant that the sniff was adding the new lines _within_ the doc block, which, in turn, meant it would still not see the "blank lines after" in the next loop, as the added lines would be seen as doc comment lines, not blank lines. Hence, the fixer conflict. Fixed now.
|
This repo is abandoned. See: #3932 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I'm the author of package PHPCS-Ignores that allows to ignore PHPCS errors/warning in the similar way as PHPStan - you have a file with listed ignores.
To make this happen, I need some extra logic in
LocalFileclass. I'm using my ownLocalFilethat extends the original one.LocalFileis hardcoded inFileList. To force PHP_CodeSniffer to use my own class, I do some magic via stream wrapper that updateFileList.phpon the fly. I was thinking if it would be possible to allow this without a hacking throughConfigclass.I prepared a small example that works for me. I'm able to update config entry in bootstrap file, that you must use if you want to use my package.
This is just proof of work, I will add tests later if this change will be approved.
Suggested changelog entry
Internals: can use own LocalFile class.
Types of changes
PR checklist
package.xmlfile.