PHP 7.4: New RemovedCurlyBraceArrayAccess sniff #855
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
jrfnl
force-pushed
the
php-7.4/deprecated-curly-brace-syntax
branch
from
5eae43f
to
23413d4
Compare
wimg
reviewed
Aug 28, 2019
PHPCompatibility/Sniffs/Syntax/NewArrayStringDereferencingSniff.php
Outdated
Show resolved
Hide resolved
wimg
reviewed
Aug 28, 2019
> The array and string offset access syntax using curly braces is deprecated.
> Use $str[$idx] instead of $str{$idx}.
Refs:
* https://wiki.php.net/rfc/deprecate_curly_braces_array_access
* https://github.com/php/php-src/blob/ef165b4422f4cef60f963833dddffa26fe1b2759/UPGRADING#L351-L353
* php/php-src#4416
* php/php-src@d574df6
## Implementation notes
Based on a lot of testing, I have come to the conclusion that the curly braces for array access worked in quite a lot of cases, though mostly since PHP 7.0.
The other sniffs which should take curly brace access into account - `NewArrayStringDereferencing`, `NewClassMemberAccess` and `NewFunctionArrayDereferencing` - have been adjusted in separate PRs.
This PR can only be merged after those PRs have been merged as it re-uses logic from those sniffs.
### Other tests
Based on the above mentioned tests, I also found that - yes, curly brace array access is supported on constants, but only when the first set of braces is square brackets.
See:
* https://3v4l.org/WIUHk
* https://3v4l.org/OWSq6
* https://3v4l.org/e6YWk
## Auto-fixing
In contrast to any other PHPCompatibility sniff, this sniff contains an auto-fixer.
For larger codebases, this issue can be quite time-consuming to fix, while fixing this automatically is trivial.
This also makes this sniff a fully fledged alternative to the [migration script](https://gist.github.com/theodorejb/763b83a43522b0fc1755a537663b1863) provided by PHP itself.
**Important**: At this moment, the PHPCompatibility unit test suite does not contain a mechanism to test the fixer.
A typical `.fixed` file which would be expected by the PHPCS native test suite to verify the fixer results _is_ included with this PR though.
## Tests run
I have run the sniff, as well as the fixer, over a number of medium to large codebases and have found _no_ false positives.
I have compared the results of a few of these projects with [the list compiled by Nikita](https://gist.github.com/nikic/b5f811e0423bf051f4492cd6e0c0273e) and for the compared results, the outcome is 100% the same.
jrfnl
force-pushed
the
php-7.4/deprecated-curly-brace-syntax
branch
from
23413d4
to
e89af92
Compare
wimg
approved these changes
Aug 28, 2019
delete-merged-branch
bot
deleted the
php-7.4/deprecated-curly-brace-syntax
branch
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.
Refs:
Implementation notes
Based on a lot of testing, I have come to the conclusion that the curly braces for array access worked in quite a lot of cases, though mostly since PHP 7.0.
The other sniffs which should take curly brace access into account -
NewArrayStringDereferencing,NewClassMemberAccessandNewFunctionArrayDereferencing- have been adjusted in separate PRs. See: #851, #852, #853This PR can only be merged after those PRs have been merged as it re-uses logic from those sniffs.
Other tests
Based on the above mentioned tests, I also found that - yes, curly brace array access is supported on constants, but only when the first set of braces is square brackets.
See:
Auto-fixing
In contrast to any other PHPCompatibility sniff, this sniff contains an auto-fixer.
For larger codebases, this issue can be quite time-consuming to fix, while fixing this automatically is trivial.
This also makes this sniff a fully fledged alternative to the migration script provided by PHP itself.
Important: At this moment, the PHPCompatibility unit test suite does not contain a mechanism to test the fixer.
A typical
.fixedfile which would be expected by the PHPCS native test suite to verify the fixer results is included with this PR though.Tests run
I have run the sniff, as well as the fixer, over a number of medium to large codebases and have found no false positives.
I have compared the results of a few of these projects with the list compiled by Nikita and for the compared results, the outcome is 100% the same.
Related to #808