Merge pull request #119 from PHPCSStandards/feature/generic-lowercase…

…constants-performance-fix

Generic/LowerCaseConstant: improve performance

CHANGELOG.md

@@ -101,6 +101,7 @@ The file documents changes to the PHP_CodeSniffer project.
 - Runtime performance improvement for PHPCS CLI users. The improvement should be most noticeable for users on Windows.
     - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
 - The following sniffs have received performance related improvements:
+    - Generic.PHP.LowerCaseConstant
     - Generic.PHP.LowerCaseType
     - PSR12.Files.OpenTag
     - Thanks to Juliette Reinders Folmer (@jrfnl) for the patches

src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php

@@ -37,6 +37,29 @@ class LowerCaseConstantSniff implements Sniff
         T_NULL  => T_NULL,
     ];
 
+    /**
+     * Token types which can be encountered in a property type declaration.
+     *
+     * @var array<int|string, int|string>
+     */
+    private $propertyTypeTokens = [
+        T_CALLABLE             => T_CALLABLE,
+        T_SELF                 => T_SELF,
+        T_PARENT               => T_PARENT,
+        T_FALSE                => T_FALSE,
+        T_TRUE                 => T_TRUE,
+        T_NULL                 => T_NULL,
+        T_STRING               => T_STRING,
+        T_NAME_QUALIFIED       => T_NAME_QUALIFIED,
+        T_NAME_FULLY_QUALIFIED => T_NAME_FULLY_QUALIFIED,
+        T_NAME_RELATIVE        => T_NAME_RELATIVE,
+        T_NS_SEPARATOR         => T_NS_SEPARATOR,
+        T_NAMESPACE            => T_NAMESPACE,
+        T_TYPE_UNION           => T_TYPE_UNION,
+        T_TYPE_INTERSECTION    => T_TYPE_INTERSECTION,
+        T_NULLABLE             => T_NULLABLE,
+    ];
+
 
     /**
      * Returns an array of tokens this test wants to listen for.
@@ -47,7 +70,13 @@ public function register()
     {
         $targets = $this->targets;
 
-        // Register function keywords to filter out type declarations.
+        // Register scope modifiers to filter out property type declarations.
+        $targets  += Tokens::$scopeModifiers;
+        $targets[] = T_VAR;
+        $targets[] = T_STATIC;
+        $targets[] = T_READONLY;
+
+        // Register function keywords to filter out param/return type declarations.
         $targets[] = T_FUNCTION;
         $targets[] = T_CLOSURE;
         $targets[] = T_FN;
@@ -64,12 +93,43 @@ public function register()
      * @param int                         $stackPtr  The position of the current token in the
      *                                               stack passed in $tokens.
      *
-     * @return void|int
+     * @return void|int Optionally returns a stack pointer. The sniff will not be
+     *                  called again on the current file until the returned stack
+     *                  pointer is reached.
      */
     public function process(File $phpcsFile, $stackPtr)
     {
         $tokens = $phpcsFile->getTokens();
 
+        /*
+         * Skip over type declarations for properties.
+         *
+         * Note: for other uses of the visibility modifiers (functions, constants, trait use),
+         * nothing relevant will be skipped as the next non-empty token will be an "non-skippable"
+         * one.
+         * Functions are handled separately below (and then skip to their scope opener), so
+         * this should also not cause any confusion for constructor property promotion.
+         *
+         * For other uses of the "static" keyword, it also shouldn't be problematic as the only
+         * time the next non-empty token will be a "skippable" token will be in return type
+         * declarations, in which case, it is correct to skip over them.
+         */
+
+        if (isset(Tokens::$scopeModifiers[$tokens[$stackPtr]['code']]) === true
+            || $tokens[$stackPtr]['code'] === T_VAR
+            || $tokens[$stackPtr]['code'] === T_STATIC
+            || $tokens[$stackPtr]['code'] === T_READONLY
+        ) {
+            $skipOver = (Tokens::$emptyTokens + $this->propertyTypeTokens);
+            $skipTo   = $phpcsFile->findNext($skipOver, ($stackPtr + 1), null, true);
+            if ($skipTo !== false) {
+                return $skipTo;
+            }
+
+            // If we're at the end of the file, just return.
+            return;
+        }
+
         // Handle function declarations separately as they may contain the keywords in type declarations.
         if ($tokens[$stackPtr]['code'] === T_FUNCTION
             || $tokens[$stackPtr]['code'] === T_CLOSURE
@@ -79,9 +139,15 @@ public function process(File $phpcsFile, $stackPtr)
                 return;
             }
 
+            // Make sure to skip over return type declarations.
             $end = $tokens[$stackPtr]['parenthesis_closer'];
             if (isset($tokens[$stackPtr]['scope_opener']) === true) {
                 $end = $tokens[$stackPtr]['scope_opener'];
+            } else {
+                $skipTo = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], ($end + 1), null, false, null, true);
+                if ($skipTo !== false) {
+                    $end = $skipTo;
+                }
             }
 
             // Do a quick check if any of the targets exist in the declaration.
@@ -114,21 +180,6 @@ public function process(File $phpcsFile, $stackPtr)
             return $end;
         }//end if
 
-        // Handle property declarations separately as they may contain the keywords in type declarations.
-        if (isset($tokens[$stackPtr]['conditions']) === true) {
-            $conditions    = $tokens[$stackPtr]['conditions'];
-            $lastCondition = end($conditions);
-            if (isset(Tokens::$ooScopeTokens[$lastCondition]) === true) {
-                // This can only be an OO constant or property declaration as methods are handled above.
-                $equals = $phpcsFile->findPrevious(T_EQUAL, ($stackPtr - 1), null, false, null, true);
-                if ($equals !== false) {
-                    $this->processConstant($phpcsFile, $stackPtr);
-                }
-
-                return;
-            }
-        }
-
         // Handle everything else.
         $this->processConstant($phpcsFile, $stackPtr);
 

src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc

@@ -98,3 +98,52 @@ class TypedThings {
 }
 
 $cl = function (int|FALSE $param = NULL, Type|NULL $obj = new MyObj(FALSE)) : string|FALSE|NULL {};
+
+// Adding some extra tests to safeguard that function declarations which don't create scope are handled correctly.
+interface InterfaceMethodsWithReturnTypeNoScopeOpener {
+    private function typed($param = TRUE) : string|FALSE|NULL;
+}
+
+abstract class ClassMethodsWithReturnTypeNoScopeOpener {
+    abstract public function typed($param = FALSE) : TRUE;
+}
+
+// Additional tests to safeguard improved property type skip logic.
+readonly class Properties {
+    use SomeTrait {
+        sayHello as private myPrivateHello;
+    }
+
+    public Type|FALSE|NULL $propertyA = array(
+        'itemA' => TRUE,
+        'itemB' => FALSE,
+        'itemC' => NULL,
+    ), $propertyB = FALSE;
+
+    protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC;
+    var ?TRUE $propertyD;
+    static array|callable|FALSE|self|parent $propertyE = TRUE;
+    private
+        // phpcs:ignore Stnd.Cat.Sniff -- for reasons.
+        TRUE      /*comment*/
+        $propertyF = TRUE;
+
+    public function __construct(
+        public FALSE|NULL $promotedPropA,
+        readonly callable|TRUE $promotedPropB,
+    ) {
+        static $var;
+        echo static::class;
+        static::foo();
+        $var = $var instanceof static;
+        $obj = new static();
+    }
+
+    public static function foo(): static|self|FALSE {
+        $callable = static function() {};
+    }
+}
+
+// Last coding/parse error.
+// This has to be the last test in the file.
+function UnclosedCurly (): FALSE {

src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed

@@ -98,3 +98,52 @@ class TypedThings {
 }
 
 $cl = function (int|FALSE $param = null, Type|NULL $obj = new MyObj(false)) : string|FALSE|NULL {};
+
+// Adding some extra tests to safeguard that function declarations which don't create scope are handled correctly.
+interface InterfaceMethodsWithReturnTypeNoScopeOpener {
+    private function typed($param = true) : string|FALSE|NULL;
+}
+
+abstract class ClassMethodsWithReturnTypeNoScopeOpener {
+    abstract public function typed($param = false) : TRUE;
+}
+
+// Additional tests to safeguard improved property type skip logic.
+readonly class Properties {
+    use SomeTrait {
+        sayHello as private myPrivateHello;
+    }
+
+    public Type|FALSE|NULL $propertyA = array(
+        'itemA' => true,
+        'itemB' => false,
+        'itemC' => null,
+    ), $propertyB = false;
+
+    protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC;
+    var ?TRUE $propertyD;
+    static array|callable|FALSE|self|parent $propertyE = true;
+    private
+        // phpcs:ignore Stnd.Cat.Sniff -- for reasons.
+        TRUE      /*comment*/
+        $propertyF = true;
+
+    public function __construct(
+        public FALSE|NULL $promotedPropA,
+        readonly callable|TRUE $promotedPropB,
+    ) {
+        static $var;
+        echo static::class;
+        static::foo();
+        $var = $var instanceof static;
+        $obj = new static();
+    }
+
+    public static function foo(): static|self|FALSE {
+        $callable = static function() {};
+    }
+}
+
+// Last coding/parse error.
+// This has to be the last test in the file.
+function UnclosedCurly (): FALSE {

src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php

@@ -51,6 +51,14 @@ public function getErrorList($testFile='LowerCaseConstantUnitTest.inc')
                 94  => 2,
                 95  => 1,
                 100 => 2,
+                104 => 1,
+                108 => 1,
+                118 => 1,
+                119 => 1,
+                120 => 1,
+                121 => 1,
+                125 => 1,
+                129 => 1,
             ];
 
         case 'LowerCaseConstantUnitTest.js':