Skip to content
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

Fix File::isReference() method #1609

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 13, 2017

The File::isReference() method misidentified a number of situations where the T_BITWISE_AND token was encountered, most notably:

  • An array assignment of a calculated value with a bitwise and operator in it ,was being misidentified as a reference.
  • A calculated default value for a function parameter with a bitwise and operator in it, was being misidentified as a reference.
  • New by reference was not recognized as a reference.
  • References to class properties with self::, parent::, static::, namespace\Class::, classname:: were not recognized as references.

This PR fixes these cases and adds dedicated unit test files for this method.

Fixes #1604

As part of this fix, the File::getMethodParameters() method also needed to be fixed.

Since PHP 5.6, default values in function declarations can contain constant expressions.
These expressions can contain a T_BITWISE_AND, but the File::getMethodParameters() method did not account for this and would misidentify the T_BITWISE_AND as indicating that the variable was being passed by reference.

Includes a new unit test for the File::getMethodParameters() method to specifically safeguard against this issue.

@jrfnl jrfnl force-pushed the feature/issue-1604-squiz-operator-spacing-references branch 2 times, most recently from 87b672e to 18ef222 Compare August 13, 2017 06:07
A number of these will currently fail.
…nce not being identified correctly

Since PHP 5.6, default values in function declarations can contain constant expressions.
These expressions can contain a `T_BITWISE_AND`, but the `File::getMethodParameters()` method did not account for this and would misidentify the `T_BITWISE_AND` as indicating that the variable was being passed by reference.

Includes a new unit test for the `File::getMethodParameters()` method to specifically safeguard against this issue.
The `File:isReference()` method misidentified a number of situations where the `T_BITWISE_AND` token was encountered, most notably:
* An array assignment of a calculated value with a bitwise and operator in it ,was being misidentified as a reference.
* A calculated default value for a function parameter with a bitwise and operator in it, was being misidentified as a reference.
* New by reference was not recognized as a reference.
* References to class properties with `self::`, `parent::`, `static::`, `namespace\Class::`, `classname::` were not recognized as references.

This commit fixes these cases.

Fixes 1604
@jrfnl jrfnl force-pushed the feature/issue-1604-squiz-operator-spacing-references branch from 18ef222 to badc598 Compare August 15, 2017 02:28
@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 15, 2017

Rebased for merge conflicts

@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 6, 2017

@gsherwood Anything I can do to help move this one forward ?

@gsherwood
Copy link
Member

Anything I can do to help move this one forward ?

Nope. Hopefully will get to it next week before release.

@gsherwood gsherwood added this to the 3.1.1 milestone Sep 19, 2017
@gsherwood gsherwood merged commit badc598 into squizlabs:master Sep 25, 2017
gsherwood added a commit that referenced this pull request Sep 25, 2017
@gsherwood
Copy link
Member

Thanks a lot for this. Sorry I didn't get around to it before the release.

@jrfnl jrfnl deleted the feature/issue-1604-squiz-operator-spacing-references branch September 25, 2017 03:44
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 25, 2017

Thanks for merging this. I'm just glad it's fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File::isReference has problems with some bitwise operators and class property references
2 participants