Skip to content

Commit

Permalink
Using PHPCS annotations to selectively re-enable sniffs is now more f…
Browse files Browse the repository at this point in the history 
…lexible (ref #1986)
  • Loading branch information
@gsherwood
gsherwood committed Apr 11, 2018
1 parent c3bf013 commit 8c8e2b1
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 17 deletions.
4 changes: 4 additions & 0 deletions package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
- You can now apply include-pattern rules to individual message codes in a ruleset like you can with exclude-pattern rules
-- Previously, include-pattern rules only applied to entire sniffs
-- If a message code has both include and exclude patterns, the exclude patterns will be ignored
- Using PHPCS annotations to selectively re-enable sniffs is now more flexible
-- Previously, you could only re-enable a sniff/category/standard using the exact same code that was disabled
-- Now, you can disable a standard and only re-enable a specific category or sniff
-- Or, you can disable a specific sniff and have it re-enable when you re-enable the category or standard
- The JSON report format now does escaping in error source codes as well as error messages
-- Thanks to Martin Vasel for the patch
- Invalid installed_paths values are now ignored instead of causing a fatal error
Expand Down
30 changes: 25 additions & 5 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ public function addFixableWarning(
protected function addMessage($error, $message, $line, $column, $code, $data, $severity, $fixable)
{
// Check if this line is ignoring all message codes.
if (isset($this->tokenizer->ignoredLines[$line]['all']) === true) {
if (isset($this->tokenizer->ignoredLines[$line]['.all']) === true) {
return false;
}

Expand Down Expand Up @@ -857,12 +857,32 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
];
}//end if

// Check if this line is ignoring this specific message.
foreach ($checkCodes as $checkCode) {
if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) {
if (isset($this->tokenizer->ignoredLines[$line]) === true) {
// Check if this line is ignoring this specific message.
$ignored = false;
foreach ($checkCodes as $checkCode) {
if (isset($this->tokenizer->ignoredLines[$line][$checkCode]) === true) {
$ignored = true;
break;
}
}

// If it is ignored, make sure it's not whitelisted.
if ($ignored === true
&& isset($this->tokenizer->ignoredLines[$line]['.except']) === true
) {
foreach ($checkCodes as $checkCode) {
if (isset($this->tokenizer->ignoredLines[$line]['.except'][$checkCode]) === true) {
$ignored = false;
break;
}
}
}

if ($ignored === true) {
return false;
}
}
}//end if

$includeAll = true;
if ($this->configCache['cache'] === false
Expand Down
31 changes: 19 additions & 12 deletions src/Tokenizers/Tokenizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ private function createPositionMap()
if ($ignoring === null
&& strpos($commentText, '@codingStandardsIgnoreStart') !== false
) {
$ignoring = ['all' => true];
$ignoring = ['.all' => true];
if ($ownLine === true) {
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
}
} else if ($ignoring !== null
&& strpos($commentText, '@codingStandardsIgnoreEnd') !== false
) {
if ($ownLine === true) {
$this->ignoredLines[$this->tokens[$i]['line']] = ['all' => true];
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
} else {
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
}
Expand All @@ -272,7 +272,7 @@ private function createPositionMap()
} else if ($ignoring === null
&& strpos($commentText, '@codingStandardsIgnoreLine') !== false
) {
$ignoring = ['all' => true];
$ignoring = ['.all' => true];
if ($ownLine === true) {
$this->ignoredLines[$this->tokens[$i]['line']] = $ignoring;
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoring;
Expand Down Expand Up @@ -367,7 +367,7 @@ private function createPositionMap()
if (substr($commentTextLower, 0, 9) === 'phpcs:set') {
// Ignore standards for complete lines that change sniff settings.
if ($lineHasOtherTokens === false) {
$this->ignoredLines[$this->tokens[$i]['line']] = ['all' => true];
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
}

$this->tokens[$i]['code'] = T_PHPCS_SET;
Expand All @@ -379,7 +379,7 @@ private function createPositionMap()
} else if (substr($commentTextLower, 0, 13) === 'phpcs:disable') {
if ($lineHasOtherContent === false) {
// Completely ignore the comment line.
$this->ignoredLines[$this->tokens[$i]['line']] = ['all' => true];
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
}

if ($ignoring === null) {
Expand All @@ -390,7 +390,7 @@ private function createPositionMap()

$additionalText = substr($commentText, 14);
if ($additionalText === false) {
$ignoring = ['all' => true];
$ignoring = ['.all' => true];
} else {
$parts = explode(',', substr($commentText, 13));
foreach ($parts as $sniffCode) {
Expand All @@ -415,19 +415,26 @@ private function createPositionMap()
foreach ($parts as $sniffCode) {
$sniffCode = trim($sniffCode);
$enabledSniffs[$sniffCode] = true;
if (isset($ignoring[$sniffCode]) === true) {
unset($ignoring[$sniffCode]);

foreach (array_keys($ignoring) as $ignoredSniffCode) {
if ($ignoredSniffCode === $sniffCode
|| strpos($ignoredSniffCode, $sniffCode.'.') === 0
) {
unset($ignoring[$ignoredSniffCode]);
}
}
}

if (empty($ignoring) === true) {
$ignoring = null;
} else {
$ignoring['.except'] = $enabledSniffs;
}
}
}//end if

if ($lineHasOtherContent === false) {
// Completely ignore the comment line.
$this->ignoredLines[$this->tokens[$i]['line']] = ['all' => true];
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
} else {
// The comment is on the same line as the code it is ignoring,
// so respect the new ignore rules.
Expand All @@ -444,7 +451,7 @@ private function createPositionMap()

$additionalText = substr($commentText, 13);
if ($additionalText === false) {
$ignoreRules = ['all' => true];
$ignoreRules = ['.all' => true];
} else {
$parts = explode(',', substr($commentText, 13));
foreach ($parts as $sniffCode) {
Expand All @@ -463,7 +470,7 @@ private function createPositionMap()
if ($lineHasOtherContent === false) {
// Completely ignore the comment line, and set the folllowing
// line to include the ignore rules we've set.
$this->ignoredLines[$this->tokens[$i]['line']] = ['all' => true];
$this->ignoredLines[$this->tokens[$i]['line']] = ['.all' => true];
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = $ignoreRules;
} else {
// The comment is on the same line as the code it is ignoring,
Expand Down
56 changes: 56 additions & 0 deletions tests/Core/ErrorSuppressionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,62 @@ public function testEnableSelected()
$this->assertEquals(1, $numWarnings);
$this->assertCount(1, $warnings);

// Suppress a whole standard and re-enable a category.
$content = '<?php '.PHP_EOL.'// phpcs:disable Generic'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// phpcs:enable Generic.Commenting'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numErrors);
$this->assertCount(0, $errors);
$this->assertEquals(1, $numWarnings);
$this->assertCount(1, $warnings);

// Suppress a category and re-enable a whole standard.
$content = '<?php '.PHP_EOL.'// phpcs:disable Generic.Commenting'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// phpcs:enable Generic'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(1, $numErrors);
$this->assertCount(1, $errors);
$this->assertEquals(1, $numWarnings);
$this->assertCount(1, $warnings);

// Suppress a sniff and re-enable a category.
$content = '<?php '.PHP_EOL.'// phpcs:disable Generic.Commenting.Todo'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// phpcs:enable Generic.Commenting'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(1, $numErrors);
$this->assertCount(1, $errors);
$this->assertEquals(1, $numWarnings);
$this->assertCount(1, $warnings);

// Suppress a whole standard and re-enable a sniff.
$content = '<?php '.PHP_EOL.'// phpcs:disable Generic'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// phpcs:enable Generic.Commenting.Todo'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numErrors);
$this->assertCount(0, $errors);
$this->assertEquals(1, $numWarnings);
$this->assertCount(1, $warnings);

}//end testEnableSelected()


Expand Down

0 comments on commit 8c8e2b1

Please sign in to comment.