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

Squiz/NonExecutableCode: slew of bug fixes, mostly related to modern PHP #3777

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Mar 15, 2023

Squiz/NonExecutableCode: remove redundant line

$prev is already defined in exactly the same way on line 66. No need to re-define it.

Squiz/NonExecutableCode: only ignore inline statements when PHP allows them

Aside from T_EXIT, PHP (prior to PHP 8.0) does not allow for the inline use of the other tokens this sniff registers.
While using those tokens inline would result in a parse error, that is not the concern of this sniff.

By ignoring these, the sniff causes false negatives.

This commit makes the sniff more selective and only ignores inline statements for tokens which are allowed in inline statements.

Includes unit test.

Squiz/NonExecutableCode: bug fix - allow for all logic operators

The sniff, as it was, only allowed for or and || logical operators before a termination expression, not for and, && or xor, while PHP allows these too.

See: https://3v4l.org/OWS4n#veol

Fixed now.

Includes unit tests.

Squiz/NonExecutableCode: bug fix - expressions in ternary

The T_EXIT token can be used in ternaries without affecting code after the ternary expression.

See: https://3v4l.org/oMWuA#veol

Fixed now.

Includes unit test.

Fixes #2857

Squiz/NonExecutableCode: bug fix - expressions after PHP 7.0/7.4 null coalesce (equals)

PHP 7.0 introduced the null coalesce operator.
PHP 7.4 introduced the null coalesce equals operator.

The T_EXIT token can be used in combination with those without affecting the code directly following it.

See: https://3v4l.org/C218d#veol

Fixed now.

Includes unit test.

Related to #2857

Squiz/NonExecutableCode: bug fix - expressions in PHP 7.4 arrow functions

PHP 7.4 introduced arrow functions. The T_EXIT token can be used in those without affecting the code following it.

See: https://3v4l.org/gSrt4#veol

Fixed now.

Includes unit test.

Squiz/NonExecutableCode: bug fix - PHP 8.0 inline throw expressions

PHP 8.0 introduced the ability to use throw as an expression instead of as a statement.
Ref: https://wiki.php.net/rfc/throw_expression

When used as an expression, the throw does not necessarily affect the code after it.

See: https://3v4l.org/AmMf2

Fixed now.

Includes unit tests.

Fixes #3592

`$prev` is already defined in exactly the same way on line 66. No need to re-define it.
…s them

Aside from `T_EXIT`, PHP (prior to PHP 8.0) does not allow for the inline use of the other tokens this sniff registers.
While using those tokens inline would result in a parse error, that is not the concern of this sniff.

By ignoring these, the sniff causes false negatives.

This commit makes the sniff more selective and only ignores inline statements for tokens which are allowed in inline statements.

Includes unit test.
The sniff, as it was, only allowed for `or` and `||` logical operators before a termination expression, not for `and`, `&&` or `xor`, while PHP allows these too.

See: https://3v4l.org/OWS4n#veol

Fixed now.

Includes unit tests.
The `T_EXIT` token can be used in ternaries without affecting code after the ternary expression.

See: https://3v4l.org/oMWuA#veol

Fixed now.

Includes unit test.

Fixes 2857
… coalesce (equals)

PHP 7.0 introduced the null coalesce operator.
PHP 7.4 introduced the null coalesce equals operator.

The `T_EXIT` token can be used in combination with those without affecting the code directly following it.

See: https://3v4l.org/C218d#veol

Fixed now.

Includes unit test.

Related to 2857
…ions

PHP 7.4 introduced arrow functions. The `T_EXIT` token can be used in those without affecting the code following it.

See: https://3v4l.org/gSrt4#veol

Fixed now.

Includes unit test.
PHP 8.0 introduced the ability to use `throw` as an expression instead of as a statement.
Ref: https://wiki.php.net/rfc/throw_expression

When used as an expression, the `throw` does not necessarily affect the code after it.

See: https://3v4l.org/AmMf2

Fixed now.

Includes unit tests.

Fixes 3592
@jrfnl
Copy link
Contributor Author

jrfnl commented Mar 15, 2023

@gsherwood Could this PR please be earmarked for PHPCS 3.8.0 ?

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation May 4, 2023
@gsherwood gsherwood added this to the 3.8.0 milestone May 4, 2023
@gsherwood gsherwood merged commit 1807f76 into squizlabs:master May 4, 2023
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release May 4, 2023
@jrfnl jrfnl deleted the feature/2857-3592-squiz-nonexecutable-code-allow-for-php8-throw-expressions branch May 4, 2023 07:41
gsherwood added a commit that referenced this pull request May 4, 2023
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 8, 2023

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
Projects
PHPCS v3 Development
Ready for Release
2 participants