Generic/RequireStrictTypes: various bugfixes #3720
Closed
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
These tests safeguard the following, which is already handled correctly by the sniff: * Execution directives are case-insensitive in PHP. * The sniff should ignore docblocks at the top of the file.
…ment While hopefully unlikely in real life, the sniff should handle parse errors correctly. Along the same lines, live coding should be handled correctly and a `declare()` statement should only be examined once the statement has been completed. This updates the sniff to handle both situations correctly. Without these fixes, no error would be thrown in test case file 5 or 6 (false negative), while an error would be thrown in test case file 7 (false positive). Includes unit tests. Includes minor efficiency tweak to only start looking for a "next" token _after_ the current token.
…ments PHP allows for multiple directives to be passed in one `declare()` statement. The sniff, however, did not allow for this, which could lead to false positives. Fixed now. Includes unit tests.
Strict types is disabled when the value for the `strict_types` directive is `0`. As this sniff is supposed to be about enforcing the use of `strict_types`, `strict_types` declarations which turn the feature off should be flagged as well. Implemented with a separate error code to allow for selectively turning this warning off. Includes dedicated tests.
jrfnl
force-pushed
the
feature/generic-requirestricttypes-various-bugfixes
branch
from
8decdcc
to
a431596
Compare
|
Closing as replaced by PHPCSStandards/PHP_CodeSniffer#51 |
|
FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release. As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo). |
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.
Generic/RequireStrictTypes: add extra tests
These tests safeguard the following, which is already handled correctly by the sniff:
Generic/RequireStrictTypes: bug fix - limit token search to the statement
While hopefully unlikely in real life, the sniff should handle parse errors correctly.
Along the same lines, live coding should be handled correctly and a
declare()statement should only be examined once the statement has been completed.This updates the sniff to handle both situations correctly.
Without these fixes, no error would be thrown in test case file 5 or 6 (false negative), while an error would be thrown in test case file 7 (false positive).
Includes unit tests.
Includes minor efficiency tweak to only start looking for a "next" token after the current token.
Generic/RequireStrictTypes: bug fix - allow for multi directive statements
PHP allows for multiple directives to be passed in one
declare()statement.The sniff, however, did not allow for this, which could lead to false positives.
Fixed now.
Includes unit tests.
Generic/RequireStrictTypes: add warning for when value is 0
Strict types is disabled when the value for the
strict_typesdirective is0.As this sniff is supposed to be about enforcing the use of
strict_types,strict_typesdeclarations which turn the feature off should be flagged as well.Implemented with a separate error code to allow for selectively turning this warning off.
Includes dedicated tests.
Future Scope
This sniff isn't very efficient as it will walk the whole file until a
declare()statement is found or until the end of the file is reached.Declare statements for
strict_typesMUST be the first statement in a file, so IMO there are two options:declare()statement and flag anystrict_typesdeclarations which are not the first statement in a file with a separate error code.In both cases, "view" files containing a mix of PHP and HTML should be taken into account, similarly, hashbang lines should be taken into account.