Adapting namespace sensitivity from pr (#1695) to issue (#1649) (#1700)

* Adapting namespace sensitivity from pr (#1695) to issue (#1649) * Make InternalInterfacesSniff namespace sensitive * Make NewInterfaceSniff namespace sensitive * Add test cases for imports from internal namespace
PHPCompatibility · Apr 30, 2024 · 96072c3 · 96072c3
1 parent 782436f
commit 96072c3
Show file tree

Hide file tree

Showing 8 changed files with 276 additions and 4 deletions.
diff --git a/PHPCompatibility/Sniffs/Interfaces/InternalInterfacesSniff.php b/PHPCompatibility/Sniffs/Interfaces/InternalInterfacesSniff.php
@@ -15,6 +15,7 @@
 use PHPCSUtils\Tokens\Collections;
 use PHPCSUtils\Utils\MessageHelper;
 use PHPCSUtils\Utils\ObjectDeclarations;
+use PHPCSUtils\Utils\UseStatements;
 
 /**
  * Detect classes which implement PHP native interfaces intended only for PHP internal use.
@@ -60,6 +61,30 @@ class InternalInterfacesSniff extends Sniff
         'BackedEnum'        => true,
     ];
 
+    /**
+     * Current file being scanned.
+     *
+     * @since 10.0.0
+     *
+     * @var string
+     */
+    private $currentFile = '';
+
+    /**
+     * Stores information about imported, namespaced declarations with names which are also in use by PHP.
+     *
+     * When those declarations are used, they do not point to the PHP internal declarations, but to the
+     * namespaced, imported declarations and those usages should be ignored by the sniff.
+     *
+     * The array is indexed by unqualified declarations names in lower case. The value is always true.
+     * It is structured this way to utilize the isset() function for faster lookups.
+     *
+     * @since 10.0.0
+     *
+     * @var array<string,true>
+     */
+    private $importedDeclarations = [];
+
 
     /**
      * Returns an array of tokens this test wants to listen for.
@@ -74,7 +99,7 @@ public function register()
         $this->internalInterfaces = \array_change_key_case($this->internalInterfaces, \CASE_LOWER);
         $this->cannotBeExtended   = \array_change_key_case($this->cannotBeExtended, \CASE_LOWER);
 
-        return Collections::ooCanImplement() + [\T_INTERFACE => \T_INTERFACE];
+        return Collections::ooCanImplement() + [\T_USE => \T_USE, \T_INTERFACE => \T_INTERFACE];
     }
 
     /**
@@ -90,7 +115,20 @@ public function register()
      */
     public function process(File $phpcsFile, $stackPtr)
     {
+        $fileName = $phpcsFile->getFilename();
+        if ($this->currentFile !== $fileName) {
+            // Reset the properties for each new file.
+            $this->currentFile          = $fileName;
+            $this->importedDeclarations = [];
+        }
+
         $tokens = $phpcsFile->getTokens();
+
+        if ($tokens[$stackPtr]['code'] === \T_USE) {
+            $this->processUseToken($phpcsFile, $stackPtr);
+            return;
+        }
+
         if ($tokens[$stackPtr]['code'] === \T_INTERFACE) {
             $interfaces = ObjectDeclarations::findExtendedInterfaceNames($phpcsFile, $stackPtr);
             $targets    = $this->cannotBeExtended;
@@ -106,7 +144,7 @@ public function process(File $phpcsFile, $stackPtr)
         foreach ($interfaces as $interface) {
             $interface   = \ltrim($interface, '\\');
             $interfaceLc = \strtolower($interface);
-            if (isset($targets[$interfaceLc]) === true) {
+            if (isset($targets[$interfaceLc]) && ! isset($this->importedDeclarations[$interfaceLc])) {
                 $error     = 'The interface %s %s';
                 $errorCode = MessageHelper::stringToErrorCode($interfaceLc) . 'Found';
                 $data      = [
@@ -118,4 +156,38 @@ public function process(File $phpcsFile, $stackPtr)
             }
         }
     }
+
+
+    /**
+     * Processes this test for when a use token is encountered.
+     *
+     * - Save imported declarations for later use.
+     *
+     * @since 10.0.0
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @param int                         $stackPtr  The position of the current token in
+     *                                               the stack passed in $tokens.
+     *
+     * @return void
+     */
+    private function processUseToken(File $phpcsFile, $stackPtr)
+    {
+        if (!UseStatements::isImportUse($phpcsFile, $stackPtr)) {
+            return;
+        }
+
+        $splitUseStatement = UseStatements::splitImportUseStatement($phpcsFile, $stackPtr);
+
+        foreach ($splitUseStatement['name'] as $name => $fullyQualifiedName) {
+            $lowerFullyQualifiedName = strtolower($fullyQualifiedName);
+
+            // If the imported declaration is imported from the internal namespace it will not be excluded.
+            if (isset($this->internalInterfaces[$lowerFullyQualifiedName])) {
+                continue;
+            }
+
+            $this->importedDeclarations[strtolower($name)] = true;
+        }
+    }
 }
diff --git a/PHPCompatibility/Sniffs/Interfaces/NewInterfacesSniff.php b/PHPCompatibility/Sniffs/Interfaces/NewInterfacesSniff.php
@@ -21,6 +21,7 @@
 use PHPCSUtils\Utils\MessageHelper;
 use PHPCSUtils\Utils\ObjectDeclarations;
 use PHPCSUtils\Utils\Scopes;
+use PHPCSUtils\Utils\UseStatements;
 use PHPCSUtils\Utils\Variables;
 
 /**
@@ -173,6 +174,30 @@ class NewInterfacesSniff extends Sniff
         ],
     ];
 
+    /**
+     * Current file being scanned.
+     *
+     * @since 10.0.0
+     *
+     * @var string
+     */
+    private $currentFile = '';
+
+    /**
+     * Stores information about imported, namespaced declarations with names which are also in use by PHP.
+     *
+     * When those declarations are used, they do not point to the PHP internal declarations, but to the
+     * namespaced, imported declarations and those usages should be ignored by the sniff.
+     *
+     * The array is indexed by unqualified declarations names in lower case. The value is always true.
+     * It is structured this way to utilize the isset() function for faster lookups.
+     *
+     * @since 10.0.0
+     *
+     * @var array<string,true>
+     */
+    private $importedDeclaration = [];
+
     /**
      * Returns an array of tokens this test wants to listen for.
      *
@@ -187,6 +212,7 @@ public function register()
         $this->unsupportedMethods = \array_change_key_case($this->unsupportedMethods, \CASE_LOWER);
 
         $targets = [
+            \T_USE,
             \T_INTERFACE,
             \T_VARIABLE,
             \T_CATCH,
@@ -212,9 +238,20 @@ public function register()
      */
     public function process(File $phpcsFile, $stackPtr)
     {
+        $fileName = $phpcsFile->getFilename();
+        if ($this->currentFile !== $fileName) {
+            // Reset the properties for each new file.
+            $this->currentFile         = $fileName;
+            $this->importedDeclaration = [];
+        }
+
         $tokens = $phpcsFile->getTokens();
 
         switch ($tokens[$stackPtr]['code']) {
+            case \T_USE:
+                $this->processUseToken($phpcsFile, $stackPtr);
+                break;
+
             case \T_INTERFACE:
                 $this->processInterfaceToken($phpcsFile, $stackPtr);
                 break;
@@ -510,6 +547,38 @@ private function processCatchToken(File $phpcsFile, $stackPtr)
         }
     }
 
+    /**
+     * Processes this test for when a use token is encountered.
+     *
+     * - Save imported declarations for later use.
+     *
+     * @since 10.0.0
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @param int                         $stackPtr  The position of the current token in
+     *                                               the stack passed in $tokens.
+     *
+     * @return void
+     */
+    private function processUseToken(File $phpcsFile, $stackPtr)
+    {
+        if (!UseStatements::isImportUse($phpcsFile, $stackPtr)) {
+            return;
+        }
+
+        $splitUseStatement = UseStatements::splitImportUseStatement($phpcsFile, $stackPtr);
+
+        foreach ($splitUseStatement['name'] as $name => $fullyQualifiedName) {
+            $lowerFullyQualifiedName = strtolower($fullyQualifiedName);
+
+            // If the imported declaration is imported from the internal namespace it will not be excluded.
+            if (isset($this->newInterfaces[$lowerFullyQualifiedName])) {
+                continue;
+            }
+
+            $this->importedDeclaration[strtolower($name)] = true;
+        }
+    }
 
     /**
      * Handle the retrieval of relevant information and - if necessary - throwing of an
@@ -526,6 +595,10 @@ private function processCatchToken(File $phpcsFile, $stackPtr)
      */
     protected function handleFeature(File $phpcsFile, $stackPtr, array $itemInfo)
     {
+        if (isset($this->importedDeclaration[$itemInfo['nameLc']])) {
+            return;
+        }
+
         $itemArray   = $this->newInterfaces[$itemInfo['nameLc']];
         $versionInfo = $this->getVersionInfo($itemArray);
 

diff --git a/PHPCompatibility/Tests/Interfaces/InternalInterfacesUnitTest.inc b/PHPCompatibility/Tests/Interfaces/InternalInterfacesUnitTest.inc
@@ -1,5 +1,5 @@
 <?php
-
+use UnitEnum;
 class MyTraversable implements Traversable {}
 class MyDateTimeClass implements DateTimeInterface {}
 class MyThrowable implements Throwable {}

diff --git a/PHPCompatibility/Tests/Interfaces/InternalInterfacesUnitTest.php b/PHPCompatibility/Tests/Interfaces/InternalInterfacesUnitTest.php
@@ -163,4 +163,31 @@ public static function dataNoFalsePositives()
     /*
      * `testNoViolationsInFileOnValidVersion` test omitted as this sniff is version independent.
      */
+
+    /**
+     * If classes with same name are used in other namespaces, they should not be flagged.
+     *
+     * @return void
+     */
+    public function testNoViolationsInFileIfOtherNamespace()
+    {
+        $file          = $this->sniffFile(__DIR__ . '/InternalInterfacesUsesUnitTest.inc', '4.4');
+        $sharedRuleSet = $file->ruleset;
+        $sharedConfig  = $file->config;
+
+        $forgedLocalFile = new \PHP_CodeSniffer\Files\LocalFile(
+            realpath(__DIR__ . '/InternalInterfacesUnitTest.inc'),
+            $sharedRuleSet,
+            $sharedConfig
+        );
+        $forgedLocalFile->parse();
+        $forgedLocalFile->process();
+
+        $this->assertNoViolation($file);
+        $this->assertError(
+            $forgedLocalFile,
+            3,
+            'The interface Traversable shouldn\'t be implemented directly, implement the Iterator or IteratorAggregate interface instead.'
+        );
+    }
 }
diff --git a/PHPCompatibility/Tests/Interfaces/InternalInterfacesUsesUnitTest.inc b/PHPCompatibility/Tests/Interfaces/InternalInterfacesUsesUnitTest.inc
@@ -0,0 +1,12 @@
+<?php
+use Something\TRaVeRsaBlE;
+use Something\datetimeinterface;
+use Something\THROWABLE;
+use Something\UnitEnum;
+use Something\BackedEnum;
+
+class MyTraversable implements TRaVeRsaBlE {}
+class MyDateTimeClass implements datetimeinterface {}
+class MyThrowable implements THROWABLE {}
+class MyUnitEnum implements UnitEnum {}
+class MyBackedEnum implements BackedEnum {}
diff --git a/PHPCompatibility/Tests/Interfaces/NewInterfacesUnitTest.inc b/PHPCompatibility/Tests/Interfaces/NewInterfacesUnitTest.inc
@@ -1,5 +1,5 @@
 <?php
-
+use Countable;
 class MyCountable implements Countable {}
 class MyOuterIterator implements OuterIterator {}
 class MyRecursiveIterator implements RecursiveIterator {}

diff --git a/PHPCompatibility/Tests/Interfaces/NewInterfacesUnitTest.php b/PHPCompatibility/Tests/Interfaces/NewInterfacesUnitTest.php
@@ -227,4 +227,31 @@ public static function dataNoFalsePositives()
      * `testNoViolationsInFileOnValidVersion` test omitted as this sniff will throw an error
      * on invalid use of some magic methods for the Serializable Interface.
      */
+
+    /**
+     * If classes with same name are used in other namespaces, they should not be flagged.
+     *
+     * @return void
+     */
+    public function testNoViolationsInFileIfOtherNamespace()
+    {
+        $file          = $this->sniffFile(__DIR__ . '/NewInterfacesUsesUnitTest.inc', '4.4');
+        $sharedRuleSet = $file->ruleset;
+        $sharedConfig  = $file->config;
+
+        $forgedLocalFile = new \PHP_CodeSniffer\Files\LocalFile(
+            realpath(__DIR__ . '/NewInterfacesUnitTest.inc'),
+            $sharedRuleSet,
+            $sharedConfig
+        );
+        $forgedLocalFile->parse();
+        $forgedLocalFile->process();
+
+        $this->assertNoViolation($file);
+        $this->assertError(
+            $forgedLocalFile,
+            3,
+            'The built-in interface Countable is not present in PHP version 5.0 or earlier'
+        );
+    }
 }
diff --git a/PHPCompatibility/Tests/Interfaces/NewInterfacesUsesUnitTest.inc b/PHPCompatibility/Tests/Interfaces/NewInterfacesUsesUnitTest.inc
@@ -0,0 +1,61 @@
+<?php
+
+use NothingHere\Traversable;
+use NothingHere\Reflector;
+use NothingHere\Countable;
+use NothingHere\OUTERiterator;
+use NothingHere\RecursiveIterator;
+use NothingHere\SeekableIterator;
+use NothingHere\Serializable;
+use NothingHere\SplObserver;
+use NothingHere\SplSubject;
+use NothingHere\JsonSerializable;
+use NothingHere\SessionHandlerInterface;
+use NothingHere\DateTimeInterface;
+use NothingHere\SessionIdInterface;
+use NothingHere\Throwable;
+use NothingHere\SessionUpdateTimestampHandlerInterface;
+use NothingHere\Stringable;
+use NothingHere\DOMChildNode;
+use NothingHere\DOMParentNode;
+use NothingHere\UnitEnum;
+use NothingHere\BackedEnum;
+use NothingHere\Random\ {Engine, CryptoSafeEngine};
+
+
+class MyTraversable implements Traversable{}
+class MyReflector implements Reflector, Countable{}
+class MyOuterIterator implements OUTERiterator{}
+class MySerializable implements Serializable{}
+class MySplObserver implements SplObserver{}
+class MySplSubject implements SplSubject{}
+class MyJsonSerializable implements JsonSerializable{}
+class MySessionHandlerInterface implements SessionHandlerInterface{}
+class MyDateTimeInterface implements DateTimeInterface{}
+class MySessionIdInterface implements SessionIdInterface{}
+class MyThrowable implements Throwable{}
+class MySessionUpdateTimestampHandlerInterface implements SessionUpdateTimestampHandlerInterface{}
+class MyStringable implements Stringable{}
+class MyDOMChildNode implements DOMChildNode{}
+class MyDOMParentNode implements DOMParentNode{}
+class MyUnitEnum implements UnitEnum{}
+class MyRandomEngine implements Engine{}
+class MyRandomCryptoSafeEngine implements CryptoSafeEngine{}
+
+// Test anonymous class support.
+$a = new class implements BackedEnum {
+};
+
+class MyClass
+{
+    private JsonSerializable $asd;
+
+    // Interfaces as typehints.
+    function CountableTypeHint(RecursiveIterator $a): RecursiveIterator{}
+    // Interfaces as typehints.
+    function CountableTypeHint(SeekableIterator $a): ?SeekableIterator{}
+}
+
+try {
+} catch (Throwable $e) {
+}