Merge pull request #627 from wimg/feature/newkeywords-bugfix

NewKeywords: fix too aggressive accounting for PHPCS cross-compat
PHPCompatibility · Mar 23, 2018 · 129afa0 · 129afa0
2 parents e47e63f + ce8f737
commit 129afa0
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 2 deletions.
diff --git a/PHPCompatibility/Sniffs/PHP/NewKeywordsSniff.php b/PHPCompatibility/Sniffs/PHP/NewKeywordsSniff.php
@@ -204,6 +204,10 @@ public function process(\PHP_CodeSniffer_File $phpcsFile, $stackPtr)
         // Translate T_STRING token if necessary.
         if ($tokens[$stackPtr]['type'] === 'T_STRING') {
             $content = $tokens[$stackPtr]['content'];
+            if (strpos($content, '__') !== 0) {
+                $content = strtolower($tokens[$stackPtr]['content']);
+            }
+
             if (isset($this->translateContentToToken[$content]) === false) {
                 // Not one of the tokens we're looking for.
                 return;
@@ -248,6 +252,14 @@ public function process(\PHP_CodeSniffer_File $phpcsFile, $stackPtr)
         $nextToken = $phpcsFile->findNext(\PHP_CodeSniffer_Tokens::$emptyTokens, ($end + 1), null, true);
         $prevToken = $phpcsFile->findPrevious(\PHP_CodeSniffer_Tokens::$emptyTokens, ($stackPtr - 1), null, true);
 
+        if ($prevToken !== false
+            && ($tokens[$prevToken]['code'] === T_DOUBLE_COLON
+            || $tokens[$prevToken]['code'] === T_OBJECT_OPERATOR)
+        ) {
+            // Class property of the same name as one of the keywords. Ignore.
+            return;
+        }
+
         // Skip attempts to use keywords as functions or class names - the former
         // will be reported by ForbiddenNamesAsInvokedFunctionsSniff, whilst the
         // latter will be (partially) reported by the ForbiddenNames sniff.

diff --git a/PHPCompatibility/Tests/Sniffs/PHP/NewKeywordsSniffTest.php b/PHPCompatibility/Tests/Sniffs/PHP/NewKeywordsSniffTest.php
@@ -69,6 +69,17 @@ public function testNamespaceKeyword()
         $this->assertNoViolation($file, 20);
     }
 
+    /**
+     * Test against false positives for the namespace keyword.
+     *
+     * @return void
+     */
+    public function testNamespaceNoFalsePositives()
+    {
+        $file = $this->sniffFile(self::TEST_FILE, '5.2');
+        $this->assertNoViolation($file, 117);
+    }
+
     /**
      * testNamespaceConstant
      *
@@ -92,9 +103,11 @@ public function testTraitKeyword()
     {
         $file = $this->sniffFile(self::TEST_FILE, '5.3');
         $this->assertError($file, 24, '"trait" keyword is not present in PHP version 5.3 or earlier');
+        $this->assertError($file, 105, '"trait" keyword is not present in PHP version 5.3 or earlier');
 
         $file = $this->sniffFile(self::TEST_FILE, '5.4');
         $this->assertNoViolation($file, 24);
+        $this->assertNoViolation($file, 105);
     }
 
     /**
@@ -158,6 +171,17 @@ public function dataYield()
         );
     }
 
+    /**
+     * Test against false positives for the yield keyword.
+     *
+     * @return void
+     */
+    public function testYieldNoFalsePositives()
+    {
+        $file = $this->sniffFile(self::TEST_FILE, '5.4');
+        $this->assertNoViolation($file, 120);
+    }
+
     /**
      * Test yield from
      *
@@ -200,9 +224,11 @@ public function testFinally()
     {
         $file = $this->sniffFile(self::TEST_FILE, '5.4');
         $this->assertError($file, 9, '"finally" keyword (in exception handling) is not present in PHP version 5.4 or earlier');
+        $this->assertError($file, 108, '"finally" keyword (in exception handling) is not present in PHP version 5.4 or earlier');
 
         $file = $this->sniffFile(self::TEST_FILE, '5.5');
         $this->assertNoViolation($file, 9);
+        $this->assertNoViolation($file, 108);
     }
 
     /**
@@ -355,7 +381,7 @@ public function testHaltCompiler()
         if (PHP_MAJOR_VERSION === 5 && PHP_MINOR_VERSION === 3) {
             // PHP 5.3 actually shows the warning.
             $file = $this->sniffFile(self::TEST_FILE, '5.0');
-            $this->assertError($file, 102, '"__halt_compiler" keyword is not present in PHP version 5.0 or earlier');
+            $this->assertError($file, 122, '"__halt_compiler" keyword is not present in PHP version 5.0 or earlier');
         } else {
             /*
              * Usage of `__halt_compiler()` cannot be tested on its own token as the compiler
@@ -364,7 +390,7 @@ public function testHaltCompiler()
              * not be reported.
              */
             $file = $this->sniffFile(self::TEST_FILE, '5.2');
-            $this->assertNoViolation($file, 105);
+            $this->assertNoViolation($file, 125);
         }
     }
 

diff --git a/PHPCompatibility/Tests/sniff-examples/new_keywords.php b/PHPCompatibility/Tests/sniff-examples/new_keywords.php
@@ -99,6 +99,26 @@ function testYieldFrom() {
 using nowdoc syntax.
 LABEL;
 
+/*
+ * Test case-insensitive matching of the PHPCS cross-version compat layer.
+ */
+TRAIT MyFoobarTrait {}
+
+try {
+} FINALLY {
+}
+
+/*
+ * Check against false positives/correct PHPCS cross-version compat layer.
+ * The fact that these keywords are reserved, is not our concern. That is handled by the forbiddenNames sniff.
+ */
+
+// "namespace" is a property (allowed use, though confusing), not the keyword.
+$response->header( 'Location', rest_url( sprintf( '%s/%s/%d', $this->namespace, $this->rest_base, $id ) ) );
+
+// yield used as a class constant.
+echo MyClass::yield;
+
 __halt_compiler();
 
 bla();