Merge pull request #34 from PHPCSStandards/fredden/php82-readonly-key…

…word-as-function-call Tokenizer/PHP: fix mis-identification of 'readonly' keyword icw PHP 8.2 DNF types
PHPCSStandards · Nov 9, 2023 · 92a9697 · 92a9697
2 parents 7b103ed + 1858e46
commit 92a9697
Show file tree

Hide file tree

Showing 3 changed files with 202 additions and 14 deletions.
diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php
@@ -1313,32 +1313,113 @@ protected function tokenize($string)
                 "readonly" keyword for PHP < 8.1
             */
 
-            if (PHP_VERSION_ID < 80100
-                && $tokenIsArray === true
+            if ($tokenIsArray === true
                 && strtolower($token[1]) === 'readonly'
                 && isset($this->tstringContexts[$finalTokens[$lastNotEmptyToken]['code']]) === false
             ) {
                 // Get the next non-whitespace token.
                 for ($i = ($stackPtr + 1); $i < $numTokens; $i++) {
                     if (is_array($tokens[$i]) === false
-                        || $tokens[$i][0] !== T_WHITESPACE
+                        || isset(Util\Tokens::$emptyTokens[$tokens[$i][0]]) === false
                     ) {
                         break;
                     }
                 }
 
+                $isReadonlyKeyword = false;
+
                 if (isset($tokens[$i]) === false
                     || $tokens[$i] !== '('
                 ) {
+                    $isReadonlyKeyword = true;
+                } else if ($tokens[$i] === '(') {
+                    /*
+                     * Skip over tokens which can be used in type declarations.
+                     * At this point, the only token types which need to be taken into consideration
+                     * as potential type declarations are identifier names, T_ARRAY, T_CALLABLE and T_NS_SEPARATOR
+                     * and the union/intersection/dnf parentheses.
+                     */
+
+                    $foundDNFParens = 1;
+                    $foundDNFPipe   = 0;
+
+                    for (++$i; $i < $numTokens; $i++) {
+                        if (is_array($tokens[$i]) === true) {
+                            $tokenType = $tokens[$i][0];
+                        } else {
+                            $tokenType = $tokens[$i];
+                        }
+
+                        if (isset(Util\Tokens::$emptyTokens[$tokenType]) === true) {
+                            continue;
+                        }
+
+                        if ($tokenType === '|') {
+                            ++$foundDNFPipe;
+                            continue;
+                        }
+
+                        if ($tokenType === ')') {
+                            ++$foundDNFParens;
+                            continue;
+                        }
+
+                        if ($tokenType === '(') {
+                            ++$foundDNFParens;
+                            continue;
+                        }
+
+                        if ($tokenType === T_STRING
+                            || $tokenType === T_NAME_FULLY_QUALIFIED
+                            || $tokenType === T_NAME_RELATIVE
+                            || $tokenType === T_NAME_QUALIFIED
+                            || $tokenType === T_ARRAY
+                            || $tokenType === T_NAMESPACE
+                            || $tokenType === T_NS_SEPARATOR
+                            || $tokenType === T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG // PHP 8.0+.
+                            || $tokenType === '&' // PHP < 8.0.
+                        ) {
+                            continue;
+                        }
+
+                        // Reached the next token after.
+                        if (($foundDNFParens % 2) === 0
+                            && $foundDNFPipe >= 1
+                            && ($tokenType === T_VARIABLE
+                            || $tokenType === T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG)
+                        ) {
+                            $isReadonlyKeyword = true;
+                        }
+
+                        break;
+                    }//end for
+                }//end if
+
+                if ($isReadonlyKeyword === true) {
                     $finalTokens[$newStackPtr] = [
                         'code'    => T_READONLY,
                         'type'    => 'T_READONLY',
                         'content' => $token[1],
                     ];
                     $newStackPtr++;
 
-                    continue;
-                }
+                    if (PHP_CODESNIFFER_VERBOSITY > 1 && $type !== T_READONLY) {
+                        echo "\t\t* token $stackPtr changed from $type to T_READONLY".PHP_EOL;
+                    }
+                } else {
+                    $finalTokens[$newStackPtr] = [
+                        'code'    => T_STRING,
+                        'type'    => 'T_STRING',
+                        'content' => $token[1],
+                    ];
+                    $newStackPtr++;
+
+                    if (PHP_CODESNIFFER_VERBOSITY > 1 && $type !== T_STRING) {
+                        echo "\t\t* token $stackPtr changed from $type to T_STRING".PHP_EOL;
+                    }
+                }//end if
+
+                continue;
             }//end if
 
             /*

diff --git a/tests/Core/Tokenizer/BackfillReadonlyTest.inc b/tests/Core/Tokenizer/BackfillReadonlyTest.inc
@@ -49,9 +49,7 @@ class Foo
     public ReAdOnLy string $caseInsensitiveProperty;
 
     /* testReadonlyConstructorPropertyPromotion */
-    public function __construct(private readonly bool $constructorPropertyPromotion)
-    {
-    }
+    public function __construct(private readonly bool $constructorPropertyPromotion) {}
 
     /* testReadonlyConstructorPropertyPromotionWithReference */
     public function __construct(private ReadOnly bool &$constructorPropertyPromotion) {}
@@ -68,8 +66,6 @@ class ClassName {
 
     /* testReadonlyUsedAsMethodName */
     public function readonly() {
-        // Do something.
-
         /* testReadonlyUsedAsPropertyName */
         $this->readonly = 'foo';
 
@@ -79,22 +75,69 @@ class ClassName {
 }
 
 /* testReadonlyUsedAsFunctionName */
-function readonly()
-{
-}
+function readonly() {}
+
+/* testReadonlyUsedAsFunctionNameWithReturnByRef */
+function &readonly() {}
 
 /* testReadonlyUsedAsNamespaceName */
 namespace Readonly;
 /* testReadonlyUsedAsPartOfNamespaceName */
 namespace My\Readonly\Collection;
 /* testReadonlyAsFunctionCall */
 $var = readonly($a, $b);
+/* testReadonlyAsNamespacedFunctionCall */
+$var = My\NS\readonly($a, $b);
+/* testReadonlyAsNamespaceRelativeFunctionCall */
+$var = namespace\ReadOnly($a, $b);
+/* testReadonlyAsMethodCall */
+$var = $obj->readonly($a, $b);
+/* testReadonlyAsNullsafeMethodCall */
+$var = $obj?->readOnly($a, $b);
+/* testReadonlyAsStaticMethodCallWithSpace */
+$var = ClassName::readonly ($a, $b);
 /* testClassConstantFetchWithReadonlyAsConstantName */
 echo ClassName::READONLY;
 
 /* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */
 $var = readonly /* comment */ ();
 
+// These test cases are inspired by
+// https://github.com/php/php-src/commit/08b75395838b4b42a41e3c70684fa6c6b113eee0
+class ReadonlyWithDisjunctiveNormalForm
+{
+    /* testReadonlyPropertyDNFTypeUnqualified */
+    readonly (B&C)|A $h;
+
+    /* testReadonlyPropertyDNFTypeFullyQualified */
+    public readonly (\Fully\Qualified\B&\Full\C)|\Foo\Bar $j;
+
+    /* testReadonlyPropertyDNFTypePartiallyQualified */
+    protected readonly (Partially\Qualified&C)|A $l;
+
+    /* testReadonlyPropertyDNFTypeRelativeName */
+    private readonly (namespace\Relative&C)|A $n;
+
+    /* testReadonlyPropertyDNFTypeMultipleSets */
+    private readonly (A&C)|(B&C)|(C&D) $m;
+
+    /* testReadonlyPropertyDNFTypeWithArray */
+    private readonly (B & C)|array $o;
+
+    /* testReadonlyPropertyDNFTypeWithSpacesAndComments */
+    private readonly ( B & C /*something*/) | A $q;
+
+    public function __construct(
+        /* testReadonlyConstructorPropertyPromotionWithDNF */
+        private readonly (B&C)|A $b1,
+        /* testReadonlyConstructorPropertyPromotionWithDNFAndRefence */
+        readonly (B&C)|A &$b2,
+    ) {}
+
+    /* testReadonlyUsedAsMethodNameWithDNFParam */
+    public function readonly (A&B $param): void {}
+}
+
 /* testParseErrorLiveCoding */
 // This must be the last test in the file.
 readonly
diff --git a/tests/Core/Tokenizer/BackfillReadonlyTest.php b/tests/Core/Tokenizer/BackfillReadonlyTest.php
@@ -148,7 +148,39 @@ public function dataReadonly()
                 'readonly',
             ],
             [
-                '/* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */',
+                '/* testReadonlyPropertyDNFTypeUnqualified */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypeFullyQualified */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypePartiallyQualified */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypeRelativeName */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypeMultipleSets */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypeWithArray */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyPropertyDNFTypeWithSpacesAndComments */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyConstructorPropertyPromotionWithDNF */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyConstructorPropertyPromotionWithDNFAndRefence */',
                 'readonly',
             ],
             [
@@ -212,6 +244,10 @@ public function dataNotReadonly()
                 '/* testReadonlyUsedAsFunctionName */',
                 'readonly',
             ],
+            [
+                '/* testReadonlyUsedAsFunctionNameWithReturnByRef */',
+                'readonly',
+            ],
             [
                 '/* testReadonlyUsedAsNamespaceName */',
                 'Readonly',
@@ -224,10 +260,38 @@ public function dataNotReadonly()
                 '/* testReadonlyAsFunctionCall */',
                 'readonly',
             ],
+            [
+                '/* testReadonlyAsNamespacedFunctionCall */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyAsNamespaceRelativeFunctionCall */',
+                'ReadOnly',
+            ],
+            [
+                '/* testReadonlyAsMethodCall */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyAsNullsafeMethodCall */',
+                'readOnly',
+            ],
+            [
+                '/* testReadonlyAsStaticMethodCallWithSpace */',
+                'readonly',
+            ],
             [
                 '/* testClassConstantFetchWithReadonlyAsConstantName */',
                 'READONLY',
             ],
+            [
+                '/* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */',
+                'readonly',
+            ],
+            [
+                '/* testReadonlyUsedAsMethodNameWithDNFParam */',
+                'readonly',
+            ],
         ];
 
     }//end dataNotReadonly()