Tests/Tokenizer: fix conditions checking range in T_DEFAULT test

[rodrigoprimo]

Description

This PR addresses an issue that I missed when working on #850. Among other things, the RecurseScopeMapDefaultKeywordConditionsTest ::testSwitchDefault() method checks if tokens within the scope of a T_DEFAULT token have T_DEFAULT set as one of its conditions array.

The test incorrectly checked token conditions due to an off-by-one error in the loop range when the T_DEFAULT uses curly braces.

For a T_DEFAULT with curly braces, the tokenizer adds T_DEFAULT to the conditions array of all the tokens within its scope up to the T_BREAK|T_RETURN|T_CONTINUE, but the test was checking only until the token before the T_BREAK|T_RETURN|T_CONTINUE. While for a T_DEFAULT without curly braces, it adds to all the tokens within the scope until the token before the scope closer.

This commit updates the test to ensure that all tokens within a default case scope that should have the T_DEFAULT token in their conditions array are properly checked. To achieve that, the loop was modified:

• The end point ($end) is now set to $closer - 1 when curly braces are not used.
• The loop condition uses inclusive comparison (<=) to ensure that the $end is also checked.

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 To me, this change is difficult to grasp. As the bug is supposed to be with the curly braces (which use the $conditionStop variable), why are you changing the $end for non-curly braced expressions ?

It would seem more logical to me to change line 162 to $end = ($token + $conditionStop + 1);.

[rodrigoprimo]

I considered changing line 162 to $end = ($token + $conditionStop + 1);, but decided in favor of the approach in this PR because I think it is easier to understand the behavior of the code when reading it as it matches more accurately what the tokenizer does:

• For non-curly braced expressions, the last token with T_DEFAULT set in the conditions array is the one just before the scope closer ($closer - 1).
• For curly braced expressions, I find it better if $conditionsStop matches the exact offset in relation to the testMarker after which T_DEFAULT stops being set in the conditions array than setting $conditionStop to a value and then having to add 1 to actually get the value that is needed.

That being said, I think both options are ok, and I'm happy to change to what you are suggesting if you prefer.

[jrfnl]

@rodrigoprimo Maybe there shouldn't even be any change to the logic, just a change in how the $conditionStop is set and making sure that the definition of what token that parameter is supposed to hold is clearly defined in the method docblock... ?

[rodrigoprimo]

@jrfnl, if you prefer this approach, that works for me, and I can update the PR.

[jrfnl]

@rodrigoprimo I have no real preference, other than that the commit message matches what the PR does and that's not the case with the current fix.

[rodrigoprimo]

@jrfnl, I went with your first suggestion and changed line 162 to $end = ($token + $conditionStop + 1);. I also updated the commit message. Since this is a small change, I amended the original commit and force pushed. I hope that is ok.