Skip to content

RemovedPHP4StyleConstructors: fix bug with nested constructs#751

Merged
wimg merged 1 commit intomasterfrom
feature/bugfix-php4-style-constructor
Dec 13, 2018
Merged

RemovedPHP4StyleConstructors: fix bug with nested constructs#751
wimg merged 1 commit intomasterfrom
feature/bugfix-php4-style-constructor

Conversation

@jrfnl
Copy link
Copy Markdown
Member

@jrfnl jrfnl commented Oct 6, 2018

Inspired by upstream issue squizlabs/PHP_CodeSniffer#2178, I checked that the same bug didn't exist in the related PHPCompatibility sniff and found that it did.

Fixed by simply skipping to the function closer after examining each function declaration as the content of the function is not our concern.

This should also make the sniff faster.

Notes:

  • In PHPCS 2.3.x, anonymous classes do not yet have their own token, so the class name determination would be incorrect as the upstream method searches for the first T_STRING and in this case, that would be the name of the first function in the anonymous class.
    Added a work-around for this.
  • Interfaces can declare deprecated constructors as well and should trigger the error.

Includes unit test for both interfaces, classes with abstract methods as well as for the original case: functions in a nested anonymous class.

Also added some additional unit tests to raise code coverage.

@jrfnl jrfnl added Type: bug PR: quick merge PR only contains relatively simple changes PR: ready for review labels Oct 6, 2018
@jrfnl jrfnl added this to the 9.x Next milestone Oct 6, 2018
@jrfnl jrfnl force-pushed the feature/bugfix-php4-style-constructor branch from 7f4df89 to 041e187 Compare October 6, 2018 14:51
@jrfnl jrfnl force-pushed the feature/bugfix-php4-style-constructor branch from 041e187 to 3ddefab Compare October 15, 2018 00:19
Inspired by upstream issue 2178, I checked that the same bug didn't exist in the related PHPCompatibility sniff and found that it did.

Fixed by simply skipping to the function closer after examining each function declaration as the content of the function is not our concern.

This should also make the sniff faster.

Notes:
* In PHPCS 2.3.x, anonymous classes do not yet have their own token, so the class name determination would be incorrect as the upstream method searches for the first `T_STRING` and in this case, that would be the name of the first function in the class, nested or otherwise.
    Added a work-around for this.
* Interfaces can declare deprecated constructors as well and should trigger the error.

Includes unit test for both interfaces, classes with abstract methods as well as for the original case: functions in a nested anonymous class.

Also added some additional unit tests to raise code coverage.
@jrfnl jrfnl force-pushed the feature/bugfix-php4-style-constructor branch from 3ddefab to eec125b Compare October 28, 2018 15:04
@jrfnl jrfnl requested a review from wimg December 9, 2018 11:04
@wimg wimg merged commit 189e546 into master Dec 13, 2018
@wimg wimg deleted the feature/bugfix-php4-style-constructor branch December 13, 2018 19:27
@jrfnl jrfnl removed the PR: quick merge PR only contains relatively simple changes label Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants