Squiz/SwitchDeclaration: bug fix - fixer conflict for single line code #1315
+210
−37
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
While working on something else, I realized that the
Squiz.ControlStructures.SwitchDeclarationsniff did not take into account that the "code under scan" could potentially be written to have various "parts" of a switch case/default structure on a single line.This meant that, in that case, the sniff would have a fixer conflict with itself.
It would basically keep adding indentation whitespace for case/default/breaking statements not having the right indentation, but as it never added a new line before the indentation, the indentation would still be incorrect in the next loop.
Fixed now by adding a number of extra checks to the sniff:
'ContentBefore' .$type- which checks if there is anything but indentation whitespace, before acaseordefaultkeyword.'ContentBeforeBreak'- which checks if there is anything but indentation whitespace, before abreak/return(etc) keyword.'ContentAfter' .$type- which checks if there is anything but a trailing comment, after acaseordefaultstatement.The sniff also contains a couple of checks which verify the number of blank lines above/below certain statements. Those also didn't take into account that the count of the "lines between" may be zero, i.e. the code "after" being on the same line as the starting point for the check.
Those situations should now largely be fixed with the new lines being added via the
Content[Before|After]*error codes.For all other cases, the fixer code for the "number of blank lines" checks has been updated to handle trailing comments better, not leave trailing whitespace behind and not get into a fixer conflict with the new checks.
Notes:
While addressing those, the current changes may be made more efficient too.
Includes tests.
Suggested changelog entry
Fixed: Squiz.ControlStructures.SwitchDeclaration: a number of the fixers would get into fixer conflicts with each other if the code under scan contained multiple statements on a line within a
switch.- To fix this, the sniff will now forbid - and auto-fix - multiple statements on one line for
case/defaultand "case breaking" statements.Types of changes