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.Operators.IncrementDecrementUsage.Found no longer works with missing spaces #325

Closed
3 tasks done
cgocast opened this issue Feb 2, 2024 · 4 comments · Fixed by #329
Closed
3 tasks done

Squiz.Operators.IncrementDecrementUsage.Found no longer works with missing spaces #325

cgocast opened this issue Feb 2, 2024 · 4 comments · Fixed by #329

Comments

@cgocast
Copy link

cgocast commented Feb 2, 2024

Describe the bug

In phpcs 2.5.0, we correctly add a Squiz.Operators.IncrementDecrementUsage.Found error when writing $a+=1 without spaces. In phpcs 3.7.2, the error is missing.

Code sample

$a=0;
$a += 1; // We correctly get a Squiz.Operators.IncrementDecrementUsage.Found
$a+=1;   // We are missing a Squiz.Operators.IncrementDecrementUsage.Found
echo $a;

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
  <description>If you are using a custom ruleset, please enter the relevant part here.</description>
  <rule ref="Squiz.Operators.IncrementDecrementUsage"/>
</ruleset>

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See error message displayed
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 4 | ERROR | Increment operators should be used where possible; found
   |       | "$a += 1;" but expected "++$a"
   |       | (Squiz.Operators.IncrementDecrementUsage.Found)
----------------------------------------------------------------------

Expected behavior

We should have also have an error on line 5

----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 4 | ERROR | Increment operators should be used where possible; found
   |       | "$a += 1;" but expected "++$a"
   |       | (Squiz.Operators.IncrementDecrementUsage.Found)
 5 | ERROR | Increment operators should be used where possible; found
   |       | "$a+=1;" but expected "++$a"
   |       | (Squiz.Operators.IncrementDecrementUsage.Found)
----------------------------------------------------------------------

Versions (please complete the following information)

Operating System Windows 11
PHP version 8.2.13
PHP_CodeSniffer version master
Standard Squiz
Install type Composer (local)

Please confirm:

  • I have searched the issue list and am not opening a duplicate issue.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer.
@jrfnl
Copy link
Member

jrfnl commented Feb 2, 2024

In phpcs 2.5.0, we correctly add a Squiz.Operators.IncrementDecrementUsage.Found error when writing $a+=1 without spaces. In phpcs 3.7.2, the error is missing.

I agree with you that the sniff should disregard spaces and that the current behaviour is a bug, but it's a bug which has always existed, so I don't see how you could get different results on PHPCS 2.5.0 - I mean, I've tested the code sample against PHPCS 2.5.0 and the results are exactly the same as with PHPCS 3.7.2 or even master (aside from a minor change in the message text).

@jrfnl
Copy link
Member

jrfnl commented Feb 7, 2024

PR #329 should fix this. Testing appreciated.

@jrfnl jrfnl added this to the 3.9.0 milestone Feb 7, 2024
@cgocast
Copy link
Author

cgocast commented Feb 7, 2024

Thank you very much @jrfnl !

I tested branch feature/325-squiz-incrementdecrementusage-bugfix and everything works as expected on my side.

Concerning versoin 2.5.0, I noticed I did not use a standard version. Some former employee at my office forked it and fixed the bug in the process. Your bugfix is much better than what he did.

@jrfnl
Copy link
Member

jrfnl commented Feb 7, 2024

Thanks for testing @cgocast ! And good to hear the explanation about what happened with your 2.5.0 version. I honestly couldn't fathom how it could have worked in 2.5.0. Maybe next time a colleague is tempted to make a fix, they could submit a PR instead ;-)

@jrfnl jrfnl closed this as completed in #329 Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants