Skip to content

Squiz.ControlStructures.SwitchDeclaration: thorough review of the sniff code #1314

@jrfnl

Description

@jrfnl

While working on something else, I came across the Squiz.ControlStructures.SwitchDeclaration sniff, and saw the following issues:

  1. The sniff contains a number of whitespace/blank line related errors which are not currently auto-fixable, while I believe they could/should be auto-fixable.
  2. A number of the error messages do not use the $data parameter, but concatenate the error message together instead. This should be fixed. Also see: Review error message creation #1240
  3. Generally speaking, the sniff could do with a review with regards to the token walking being done.
  4. Fixer code should be reviewed to see if the fixer can handle all situations or could result in multiple loops. If the latter, the fixer code should be fixed.
    Example - "'CASE keyword must be followed by a single space'", SpacingAfterCase error code. The fixer doesn't appear to take into account that the whitespace after the "case" keyword could be multiple new lines.
    $error = 'CASE keyword must be followed by a single space';
    $fix = $phpcsFile->addFixableError($error, $nextCase, 'SpacingAfterCase');
    if ($fix === true) {
    if ($tokens[($nextCase + 1)]['type'] !== 'T_WHITESPACE') {
    $phpcsFile->fixer->addContent($nextCase, ' ');
    } else {
    $phpcsFile->fixer->replaceToken(($nextCase + 1), ' ');
    }
    }

There may be more issues, but this was just what I saw at first glance, so a thorough review would not go amiss.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions