Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add T_USE to the list of parenthesis openers #2593

Closed
gsherwood opened this issue Aug 26, 2019 · 2 comments
Closed

Add T_USE to the list of parenthesis openers #2593

gsherwood opened this issue Aug 26, 2019 · 2 comments

Comments

@gsherwood
Copy link
Member

T_USE can open parenthesis, or can be a scope opener. It currently doesn't get parenthesis opener/closer indexes when used as a USE group in a closure. Adding support isn't that hard, but may break backwards compatibility for some sniffs.

A diff of changes that could be required in the core:

diff --git a/src/Sniffs/AbstractVariableSniff.php b/src/Sniffs/AbstractVariableSniff.php
index 5dc8ba556..f2139b15a 100644
--- a/src/Sniffs/AbstractVariableSniff.php
+++ b/src/Sniffs/AbstractVariableSniff.php
@@ -118,19 +118,13 @@ abstract class AbstractVariableSniff extends AbstractScopeSniff
         if ($inFunction === false && isset($tokens[$stackPtr]['nested_parenthesis']) === true) {
             foreach ($tokens[$stackPtr]['nested_parenthesis'] as $opener => $closer) {
                 if (isset($tokens[$opener]['parenthesis_owner']) === false) {
-                    // Check if this is a USE statement for a closure.
-                    $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), null, true);
-                    if ($tokens[$prev]['code'] === T_USE) {
-                        $inFunction = true;
-                        break;
-                    }
-
                     continue;
                 }

                 $owner = $tokens[$opener]['parenthesis_owner'];
                 if ($tokens[$owner]['code'] === T_FUNCTION
                     || $tokens[$owner]['code'] === T_CLOSURE
+                    || $tokens[$owner]['code'] === T_USE
                 ) {
                     $inFunction = true;
                     break;
diff --git a/src/Tokenizers/Tokenizer.php b/src/Tokenizers/Tokenizer.php
index bc904586c..677448e24 100644
--- a/src/Tokenizers/Tokenizer.php
+++ b/src/Tokenizers/Tokenizer.php
@@ -691,13 +691,26 @@ abstract class Tokenizer
                 $this->tokens[$i]['parenthesis_closer'] = null;
                 $this->tokens[$i]['parenthesis_owner']  = $i;
                 $openOwner = $i;
+
+                if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                    echo str_repeat("\t", (count($openers) + 1));
+                    echo "=> Found parenthesis owner at $i".PHP_EOL;
+                }
             } else if ($this->tokens[$i]['code'] === T_OPEN_PARENTHESIS) {
                 $openers[] = $i;
                 $this->tokens[$i]['parenthesis_opener'] = $i;
                 if ($openOwner !== null) {
+                    if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                        echo str_repeat("\t", count($openers));
+                        echo "=> Found parenthesis opener at $i for $openOwner".PHP_EOL;
+                    }
+
                     $this->tokens[$openOwner]['parenthesis_opener'] = $i;
                     $this->tokens[$i]['parenthesis_owner']          = $openOwner;
                     $openOwner = null;
+                } else if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                    echo str_repeat("\t", count($openers));
+                    echo "=> Found unowned parenthesis opener at $i".PHP_EOL;
                 }
             } else if ($this->tokens[$i]['code'] === T_CLOSE_PARENTHESIS) {
                 // Did we set an owner for this set of parenthesis?
@@ -709,6 +722,14 @@ abstract class Tokenizer

                         $this->tokens[$owner]['parenthesis_closer'] = $i;
                         $this->tokens[$i]['parenthesis_owner']      = $owner;
+
+                        if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                            echo str_repeat("\t", (count($openers) + 1));
+                            echo "=> Found parenthesis closer at $i for $owner".PHP_EOL;
+                        }
+                    } else if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                        echo str_repeat("\t", (count($openers) + 1));
+                        echo "=> Found unowned parenthesis closer at $i for $opener".PHP_EOL;
                     }

                     $this->tokens[$i]['parenthesis_opener']      = $opener;
@@ -730,6 +751,10 @@ abstract class Tokenizer
                     echo str_repeat("\t", count($curlyOpeners));
                     echo "=> Found square bracket opener at $i".PHP_EOL;
                 }
+
+                if ($openOwner !== null) {
+                    $openOwner = null;
+                }
                 break;
             case T_OPEN_CURLY_BRACKET:
                 if (isset($this->tokens[$i]['scope_closer']) === false) {
@@ -741,6 +766,10 @@ abstract class Tokenizer
                         echo "=> Found curly bracket opener at $i".PHP_EOL;
                     }
                 }
+
+                if ($openOwner !== null) {
+                    $openOwner = null;
+                }
                 break;
             case T_CLOSE_SQUARE_BRACKET:
                 if (empty($squareOpeners) === false) {
diff --git a/src/Util/Tokens.php b/src/Util/Tokens.php
index bf3b5f9f3..4819d69d4 100644
--- a/src/Util/Tokens.php
+++ b/src/Util/Tokens.php
@@ -348,6 +348,7 @@ final class Tokens
         T_LIST     => T_LIST,
         T_FUNCTION => T_FUNCTION,
         T_CLOSURE  => T_CLOSURE,
+        T_USE      => T_USE,
         T_WHILE    => T_WHILE,
         T_FOR      => T_FOR,
         T_FOREACH  => T_FOREACH,
@gsherwood gsherwood added this to the 4.0.0 milestone Aug 26, 2019
@gsherwood gsherwood added this to Backlog in PHPCS v4 Development via automation Aug 26, 2019
@gsherwood gsherwood moved this from Backlog to Selected for Development in PHPCS v4 Development Jan 31, 2020
gsherwood added a commit that referenced this issue Jul 23, 2020
@gsherwood
Copy link
Member Author

This change has now been made in the 4.0 branch

PHPCS v4 Development automation moved this from Selected for Development to Ready for Release Jul 23, 2020
@jrfnl
Copy link
Contributor

jrfnl commented Sep 5, 2020

I've been looking into the impact of this change and am finding quite some usability problems and also some bugs.

Example:

use function Vendor\Package\name;

name($param);

With the above code, the parenthesis for the name() function call now have the use token from the import use statement as the parenthesis owner.

I've submitted PR #3104 in an attempt to fix this and other inconsistencies caused by the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PHPCS v4 Development
  
Ready for Release
Development

No branches or pull requests

2 participants