New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHP 8.0 | New NewAttributes
sniff
#1279
Merged
Merged
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
39 tasks
PHP 8.0 introduces Attributes as a form of structured, syntactic metadata to declarations of classes, properties, functions, methods, parameters and constants. Attributes allow to define configuration directives directly embedded with the declaration of that code. The syntax for attributes in PHP has changed a number of times via various RFC amendments. As a result of that, it took a little while as well before the final tokenization of attributes in PHPCS was known, but upstream PR squizlabs/PHP_CodeSniffer 3203 has now (finally) been merged and released as part of PHPCS 3.6.0. This PR introduces a new sniff to detect the use of attributes. This new sniff _only_ focusses on detecting the _syntax_ for attributes being used - `#[...]`. It does not verify whether the _contents_ within the attribute is valid, nor whether the attribute is used in a _supported location_. Invalid attributes like that would be a parse error in PHP 8.0+. It can be argued that single-line attributes should not throw an error, but a warning, as those will not necessarily cause problems when used in code which also needs to run on PHP 7, as they will just be ignored and treated as a comment. This is however not the case for multi-line or in-line attributes, which would cause parse error in PHP < 8.0, so for consistency, I've chosen to let the sniff always throw an error. In rare cases, specifically for the code sample in the last test case, the PHPCS tokenizer cannot prevent the rest of the file becoming mangled on PHP < 8.0. This is similar to the tokenizer problems caused by indented heredocs/nowdocs as introduced in PHP 7.3, where the rest of the file after the new syntax will be tokenized incorrectly. In that case, the sniff will still (correctly) throw an error for the attribute, however detection of (other) issues in the rest of the file will be broken. Also see: squizlabs/PHP_CodeSniffer 3294 As the attributes feature is likely to be expanded in the future, the sniff has been placed in a new dedicated category which will allow for additional attribute related sniffs to be added at a later point in time. If and when new attribute related features are added to PHP, validation of whether an attribute is valid in PHP 8.0 may need to be added to this sniff, but this is a moot point until that time. Includes unit tests. Refs: * https://wiki.php.net/rfc/attributes_v2 * https://wiki.php.net/rfc/attribute_amendments * https://wiki.php.net/rfc/shorter_attribute_syntax * https://wiki.php.net/rfc/shorter_attribute_syntax_change * https://www.php.net/manual/en/language.attributes.php
jrfnl
force-pushed
the
php-8.0/new-newattributes-sniff
branch
from
April 11, 2021 16:01
5ed717d
to
900d1a2
Compare
elazar
reviewed
Apr 13, 2021
elazar
approved these changes
Apr 13, 2021
Thanks for reviewing critically @elazar ! I much rather have questions which turn out to be already covered, than that something is missed in the PR, so I really appreciate your critical look. |
... as per the discussion and similar test case upstream in squizlabs/PHP_CodeSniffer 3294.
jrfnl
force-pushed
the
php-8.0/new-newattributes-sniff
branch
from
April 13, 2021 04:51
16296c3
to
1ab6aeb
Compare
wimg
approved these changes
May 9, 2021
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.
PHP 8.0 introduces Attributes as a form of structured, syntactic metadata to declarations of classes, properties, functions, methods, parameters and constants. Attributes allow to define configuration directives directly embedded with the declaration of that code.
The syntax for attributes in PHP has changed a number of times via various RFC amendments.
As a result of that, it took a little while as well before the final tokenization of attributes in PHPCS was known, but upstream PR squizlabs/PHP_CodeSniffer#3203 has now (finally) been merged and released as part of PHPCS 3.6.0.
This PR introduces a new sniff to detect the use of attributes.
This new sniff only focusses on detecting the syntax for attributes being used -
#[...]
.It does not verify whether the contents within the attribute is valid, nor whether the attribute is used in a supported location.
Invalid attributes like that would be a parse error in PHP 8.0+.
It can be argued that single-line attributes should not throw an error, but a warning, as those will not necessarily cause problems when used in code which also needs to run on PHP 7, as they will just be ignored and treated as a comment.
This is however not the case for multi-line or in-line attributes, which would cause parse error in PHP < 8.0, so for consistency, I've chosen to let the sniff always throw an error.
In rare cases, specifically for the code sample in the last test case, the PHPCS tokenizer cannot prevent the rest of the file becoming mangled on PHP < 8.0.
This is similar to the tokenizer problems caused by indented heredocs/nowdocs as introduced in PHP 7.3, where the rest of the file after the new syntax will be tokenized incorrectly.
In that case, the sniff will still (correctly) throw an error for the attribute, however detection of (other) issues in the rest of the file will be broken.
Also see: squizlabs/PHP_CodeSniffer#3294
As the attributes feature is likely to be expanded in the future, the sniff has been placed in a new dedicated category which will allow for additional attribute related sniffs to be added at a later point in time.
If and when new attribute related features are added to PHP, validation of whether an attribute is valid in PHP 8.0 may need to be added to this sniff, but this is a moot point until that time.
Includes unit tests.
Refs:
Related to #809