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

✨ New Universal.CodeAnalysis.NoDoubleNegative sniff #277

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 21, 2023

... to detect double negatives (!!) and advise to use a boolean cast instead.

The sniff will correctly handle a situation where even more consecutive not operators are found.

In the case, the number of operators is uneven, it will auto-fix to a single not operator.

And when a double not operator is found before an expression involving instanceof, the error will still be thrown, but not auto-fix as the instanceof operator is nested right between the ! operator and a (bool) cast operator precedence wise, so auto-fixing without also adding parentheses would change the behaviour of the code.

Includes fixer.
Includes unit tests.
Includes documentation.

Ref: https://www.php.net/manual/en/language.operators.precedence.php

... to detect double negatives (`!!`) and advise to use a boolean cast instead.

The sniff will correctly handle a situation where even more consecutive not operators are found.

In the case, the number of operators is uneven, it will auto-fix to a single not operator.

And when a double not operator is found before an expression involving `instanceof`, the error will still be thrown, but not auto-fix as the `instanceof` operator is nested right between the `!` operator and a `(bool)` cast operator precedence wise, so auto-fixing without also adding parentheses would change the behaviour of the code.

Includes fixer.
Includes unit tests.
Includes documentation.

Ref: https://www.php.net/manual/en/language.operators.precedence.php
Copy link

@diedexx diedexx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to test to confirm it, but this might be a case that isn't detecting the double !.

echo !@! $undefined_var;

Apparently you can silence the undefined variable warning in this way. Not sure why anyone would though. 🤷

Comment on lines +52 to +58
// Collect all the operators only once.
$this->operatorsWithLowerPrecedence = Tokens::$assignmentTokens;
$this->operatorsWithLowerPrecedence += Tokens::$booleanOperators;
$this->operatorsWithLowerPrecedence += Tokens::$comparisonTokens;
$this->operatorsWithLowerPrecedence += Tokens::$operators;
$this->operatorsWithLowerPrecedence[\T_INLINE_THEN] = \T_INLINE_THEN;
$this->operatorsWithLowerPrecedence[\T_INLINE_ELSE] = \T_INLINE_ELSE;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: why not use the constructor for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rare for a sniff to have a constructor and the register() method is normally only called once anyway, so as good a place as any to do this in.

@jrfnl
Copy link
Member Author

jrfnl commented Sep 25, 2023

I wasn't able to test to confirm it, but this might be a case that isn't detecting the double !.

echo !@! $undefined_var;

Apparently you can silence the undefined variable warning in this way. Not sure why anyone would though. 🤷

Good point and no, the sniff would not flag that, but I'm not sure the sniff should catch that...

@jrfnl jrfnl merged commit 52ab69a into develop Nov 12, 2023
39 checks passed
@jrfnl jrfnl deleted the universal/new-nodoublenegative-sniff branch November 12, 2023 20:28
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.

None yet

2 participants