Generic/JumbledIncrementer: fix non-problematic bug and improve test coverage

[rodrigoprimo]

Description

This PR adds a few commits related to the Generic.CodeAnalysis.JumbledIncrementer sniff:

• Fix a non-problematic bug that would happen when handling an inner for loop without a closing parenthesis. In those cases, $end would be set to null (src/Standards/Generic/Sniffs/CodeAnalysis/JumbledIncrementerSniff.php#L116) instead of set to the index of the closing parenthesis token. This problem did not cause any issues as $next <= $end when $end is null is always false, and the for loop that looks for incrementers ends early.
• Improve test coverage
• Improve the name of a variable

A few things that I noticed while working on this PR that I would like to confirm if they are indeed issues (probably in separate PRs):

• The sniff documentation and its name only mention incrementers, but the sniff also works for decrementers. Should we update the documentation? I guess the name is a bit more complex to change, so I'm inclined to think we should not change it.
• The sniff assumes that any variable in the third part of the for loop condition is an incrementer or decrementer which is not true. The code below triggers the sniff and I'm not sure if it should:

$label = 'label';

for ($i = 1; $i <= 10; $i++, print $label) {
    for ($j = 0; $j < 5; $j++, print $label);
}

Suggested changelog entry

Generic.CodeAnalysis.JumbledIncrementer: fixed a non-problematic bug when the inner for loop is missing the closing parenthesis

Related issues/external references

Part of #146

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

@rodrigoprimo Thank you for this PR and my apologies for taking a while before reviewing it.

Generally speaking, all looking good! Thank you for making these changes.

There's a few small nitpicks, but nothing major.

While reviewing this PR, I noticed a few other things the sniff does not correctly take into account. I will open separate issues for those.

[rodrigoprimo]

Thanks for the review, @jrfnl!

I believe I addressed the points that you raised. I had to force push commits to the branch as I dropped the commit that renamed a variable in the sniff code and added a new commit in between commits to rename variables for the pre-existing tests.

This PR is ready for a final check.

[jrfnl]

As per the comment I left in my previous review, I've now opened two issues for potential follow-up improvements for the sniff: #440 and #441

I also realized that I hadn't answered the questions you included in the description yet.

The sniff documentation and its name only mention incrementers, but the sniff also works for decrementers. Should we update the documentation? I guess the name is a bit more complex to change, so I'm inclined to think we should not change it.

Good catch and yes, please feel invited to improve the documentation (docblock + XML ?).

And as you already surmised, changing the sniff name would be a breaking change, so is not on the table.

The sniff assumes that any variable in the third part of the for loop condition is an incrementer or decrementer which is not true. The code below triggers the sniff and I'm not sure if it should:

I fully agree with you that this is a bug. Issue #441 contains a range of additional code samples running into the same issue. Please add your code sample in a comment to that ticket so it won't be overlooked by whomever starts working on that ticket.

[jrfnl]

@rodrigoprimo Thank you for working on this and making the requested updates.

I'll go ahead and reorganize the commits and rebase the PR before merging it.

I will fix up that comment change which got undone while doing that.