Skip to content

Commit

Permalink
Comment syntax to ignore lines now only ignores the following line if…
Browse files Browse the repository at this point in the history 
… it is on a line by itself + unit tests + fixes for new comment syntax docblock comments
  • Loading branch information
@gsherwood
gsherwood committed Oct 30, 2017
1 parent 5325170 commit 389e52f
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 4 deletions.
4 changes: 4 additions & 0 deletions package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
--- phpcs:ignore has the token T_PHPCS_IGNORE
--- phpcs:set has the token T_PHPCS_SET

- @codingStandardsIgnoreLine comments now only ignore the following line if they are on a line by themselves
-- If they are at the end of an existing line, they will only ignore the line they are on
-- Stops some lines from accidently being ignored
-- Same rule applies for the new phpcs:ignore comment syntax
- Array properties specified in ruleset files now have their keys and values trimmed
-- This saves having to do this in individual sniffs, and stops errors introduced by whitespace in rulesets
-- Thanks to Juliette Reinders Folmer for the patch
Expand Down
1 change: 1 addition & 0 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ public function process()
&& ($token['code'] === T_COMMENT
|| $token['code'] === T_PHPCS_IGNORE_FILE
|| $token['code'] === T_PHPCS_SET
|| $token['code'] === T_DOC_COMMENT_STRING
|| $token['code'] === T_DOC_COMMENT_TAG
|| ($inTests === true && $token['code'] === T_INLINE_HTML))
) {
Expand Down
19 changes: 16 additions & 3 deletions src/Tokenizers/Tokenizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ private function createPositionMap()

if ($checkAnnotations === true
&& ($this->tokens[$i]['code'] === T_COMMENT
|| $this->tokens[$i]['code'] === T_DOC_COMMENT_STRING
|| $this->tokens[$i]['code'] === T_DOC_COMMENT_TAG
|| ($inTests === true && $this->tokens[$i]['code'] === T_INLINE_HTML))
) {
Expand All @@ -239,10 +240,22 @@ private function createPositionMap()
} else if ($ignoring === false
&& strpos($commentText, '@codingStandardsIgnoreLine') !== false
) {
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = true;
// Ignore this comment too.
// If this comment is the only thing on the line, it tells us
// to ignore the following line. If the line contains other content
// then we are just ignoring this one single line.
$this->ignoredLines[$this->tokens[$i]['line']] = true;
}
for ($prev = ($i - 1); $prev >= 0; $prev--) {
if ($this->tokens[$prev]['code'] === T_WHITESPACE) {
continue;
}

break;
}

if ($this->tokens[$prev]['line'] !== $this->tokens[$i]['line']) {
$this->ignoredLines[($this->tokens[$i]['line'] + 1)] = true;
}
}//end if
} else if (substr($commentTextLower, 0, 6) === 'phpcs:') {
if (substr($commentTextLower, 0, 9) === 'phpcs:set') {
// Ignore standards for lines that change sniff settings.
Expand Down
212 changes: 211 additions & 1 deletion tests/Core/ErrorSuppressionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,26 @@ public function testSuppressError()
$this->assertEquals(1, count($errors));

// Process with inline comment suppression.
$content = '<?php '.PHP_EOL.'// phpcs:disable'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'// phpcs:enable';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with inline comment suppression mixed case.
$content = '<?php '.PHP_EOL.'// PHPCS:Disable'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'// pHPcs:enabLE';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with inline comment suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreStart'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'// @codingStandardsIgnoreEnd';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -52,6 +72,16 @@ public function testSuppressError()
$this->assertEquals(0, count($errors));

// Process with block comment suppression.
$content = '<?php '.PHP_EOL.'/* phpcs:disable */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/* phpcs:disable */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with block comment suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/* @codingStandardsIgnoreStart */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/* @codingStandardsIgnoreEnd */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -62,6 +92,16 @@ public function testSuppressError()
$this->assertEquals(0, count($errors));

// Process with a docblock suppression.
$content = '<?php '.PHP_EOL.'/** phpcs:disable */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/** phpcs:enable */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with a docblock suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/** @codingStandardsIgnoreStart */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/** @codingStandardsIgnoreEnd */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand Down Expand Up @@ -98,6 +138,16 @@ public function testSuppressSomeErrors()
$this->assertEquals(2, count($errors));

// Process with suppression.
$content = '<?php '.PHP_EOL.'// phpcs:disable'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'// phpcs:enable'.PHP_EOL.'$var = TRUE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

// Process with suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreStart'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'// @codingStandardsIgnoreEnd'.PHP_EOL.'$var = TRUE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -108,6 +158,16 @@ public function testSuppressSomeErrors()
$this->assertEquals(1, count($errors));

// Process with a PHPDoc block suppression.
$content = '<?php '.PHP_EOL.'/** phpcs:disable */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/** phpcs:enable */'.PHP_EOL.'$var = TRUE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

// Process with a PHPDoc block suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/** @codingStandardsIgnoreStart */'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'/** @codingStandardsIgnoreEnd */'.PHP_EOL.'$var = TRUE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand Down Expand Up @@ -144,6 +204,16 @@ public function testSuppressWarning()
$this->assertEquals(1, count($warnings));

// Process with suppression.
$content = '<?php '.PHP_EOL.'// phpcs:disable'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// phpcs:enable';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreStart'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'// @codingStandardsIgnoreEnd';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -154,6 +224,16 @@ public function testSuppressWarning()
$this->assertEquals(0, count($warnings));

// Process with a docblock suppression.
$content = '<?php '.PHP_EOL.'/** phpcs:disable */'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'/** phpcs:enable */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with a docblock suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/** @codingStandardsIgnoreStart */'.PHP_EOL.'//TODO: write some code'.PHP_EOL.'/** @codingStandardsIgnoreEnd */';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand Down Expand Up @@ -189,7 +269,17 @@ public function testSuppressLine()
$this->assertEquals(2, $numErrors);
$this->assertEquals(2, count($errors));

// Process with suppression.
// Process with suppression on line before.
$content = '<?php '.PHP_EOL.'// phpcs:ignore'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = FALSE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

// Process with suppression on line before (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreLine'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = FALSE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -199,6 +289,26 @@ public function testSuppressLine()
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

// Process with suppression on same line.
$content = '<?php '.PHP_EOL.'$var = FALSE; // phpcs:ignore'.PHP_EOL.'$var = FALSE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

// Process with suppression on same line (deprecated syntax).
$content = '<?php '.PHP_EOL.'$var = FALSE; // @codingStandardsIgnoreLine'.PHP_EOL.'$var = FALSE;';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(1, $numErrors);
$this->assertEquals(1, count($errors));

}//end testSuppressLine()


Expand All @@ -216,6 +326,16 @@ public function testNestedSuppressLine()
$ruleset = new Ruleset($config);

// Process with codingStandardsIgnore[Start|End] suppression and no single line suppression.
$content = '<?php '.PHP_EOL.'// phpcs:disable'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = TRUE;'.PHP_EOL.'// phpcs:enable';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with codingStandardsIgnore[Start|End] suppression and no single line suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreStart'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = TRUE;'.PHP_EOL.'// @codingStandardsIgnoreEnd';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -227,6 +347,17 @@ public function testNestedSuppressLine()

// Process with codingStandardsIgnoreLine suppression
// nested within codingStandardsIgnore[Start|End] suppression.
$content = '<?php '.PHP_EOL.'// phpcs:disable'.PHP_EOL.'// phpcs:ignore'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = TRUE;'.PHP_EOL.'// phpcs:enable';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with codingStandardsIgnoreLine suppression
// nested within codingStandardsIgnore[Start|End] suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreStart'.PHP_EOL.'// @codingStandardsIgnoreLine'.PHP_EOL.'$var = FALSE;'.PHP_EOL.'$var = TRUE;'.PHP_EOL.'// @codingStandardsIgnoreEnd';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand Down Expand Up @@ -261,6 +392,16 @@ public function testSuppressScope()
$this->assertEquals(0, count($errors));

// Process with suppression.
$content = '<?php '.PHP_EOL.'class MyClass() {'.PHP_EOL.'//phpcs:disable'.PHP_EOL.'function myFunction() {'.PHP_EOL.'//phpcs:enable'.PHP_EOL.'$this->foo();'.PHP_EOL.'}'.PHP_EOL.'}';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'class MyClass() {'.PHP_EOL.'//@codingStandardsIgnoreStart'.PHP_EOL.'function myFunction() {'.PHP_EOL.'//@codingStandardsIgnoreEnd'.PHP_EOL.'$this->foo();'.PHP_EOL.'}'.PHP_EOL.'}';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -271,6 +412,15 @@ public function testSuppressScope()
$this->assertEquals(0, count($errors));

// Process with a docblock suppression.
$content = '<?php '.PHP_EOL.'class MyClass() {'.PHP_EOL.'/** phpcs:disable */'.PHP_EOL.'function myFunction() {'.PHP_EOL.'/** phpcs:enable */'.PHP_EOL.'$this->foo();'.PHP_EOL.'}'.PHP_EOL.'}';
$file = $phpcs->processFile('suppressionTest.php', $content);

$errors = $file->getErrors();
$numErrors = $file->getErrorCount();
$this->assertEquals(0, $numErrors);
$this->assertEquals(0, count($errors));

// Process with a docblock suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'class MyClass() {'.PHP_EOL.'/** @codingStandardsIgnoreStart */'.PHP_EOL.'function myFunction() {'.PHP_EOL.'/** @codingStandardsIgnoreEnd */'.PHP_EOL.'$this->foo();'.PHP_EOL.'}'.PHP_EOL.'}';
$file = $phpcs->processFile('suppressionTest.php', $content);

Expand Down Expand Up @@ -306,6 +456,16 @@ public function testSuppressFile()
$this->assertEquals(1, count($warnings));

// Process with suppression.
$content = '<?php '.PHP_EOL.'// phpcs:ignoreFile'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'// @codingStandardsIgnoreFile'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -315,7 +475,47 @@ public function testSuppressFile()
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process mixed case.
$content = '<?php '.PHP_EOL.'// PHPCS:Ignorefile'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process late comment.
$content = '<?php '.PHP_EOL.'class MyClass {}'.PHP_EOL.'$foo = new MyClass()'.PHP_EOL.'// phpcs:ignoreFile';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process late comment (deprecated syntax).
$content = '<?php '.PHP_EOL.'class MyClass {}'.PHP_EOL.'$foo = new MyClass()'.PHP_EOL.'// @codingStandardsIgnoreFile';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with a block comment suppression.
$content = '<?php '.PHP_EOL.'/* phpcs:ignoreFile */'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with a block comment suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/* @codingStandardsIgnoreFile */'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand All @@ -326,6 +526,16 @@ public function testSuppressFile()
$this->assertEquals(0, count($warnings));

// Process with docblock suppression.
$content = '<?php '.PHP_EOL.'/** phpcs:ignoreFile */'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();

$warnings = $file->getWarnings();
$numWarnings = $file->getWarningCount();
$this->assertEquals(0, $numWarnings);
$this->assertEquals(0, count($warnings));

// Process with docblock suppression (deprecated syntax).
$content = '<?php '.PHP_EOL.'/** @codingStandardsIgnoreFile */'.PHP_EOL.'//TODO: write some code';
$file = new DummyFile($content, $ruleset, $config);
$file->process();
Expand Down

0 comments on commit 389e52f

Please sign in to comment.