Skip to content

Commit

Permalink
NewClassesSniff incorrectly reports imported classes as missing (#1695)
Browse files Browse the repository at this point in the history
NewClassesSniff incorrectly reports imported classes as missing

* Record `use`s in attribute and ignores them when checking for usage
  of PHP default classes.
  • Loading branch information
rdss-sknott committed Apr 8, 2024
1 parent 7cd518e commit a988e5b
Show file tree
Hide file tree
Showing 4 changed files with 477 additions and 0 deletions.
72 changes: 72 additions & 0 deletions PHPCompatibility/Sniffs/Classes/NewClassesSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use PHPCSUtils\Utils\ControlStructures;
use PHPCSUtils\Utils\FunctionDeclarations;
use PHPCSUtils\Utils\Scopes;
use PHPCSUtils\Utils\UseStatements;
use PHPCSUtils\Utils\Variables;

/**
Expand Down Expand Up @@ -1095,6 +1096,29 @@ class NewClassesSniff extends Sniff
],
];

/**
* Current file being scanned.
*
* @since 10.0.0
*
* @var string
*/
private $currentFile = '';

/**
* Stores information about imported, namespaced classes with names which are also in use by PHP.
*
* When those classes are used, they do not point to the PHP classes, but to the
* namespaced, imported class and those usages should be ignored by the sniff.
*
* The array is indexed by unqualified class 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 $importedClasses = [];

/**
* Returns an array of tokens this test wants to listen for.
Expand Down Expand Up @@ -1123,6 +1147,7 @@ public function register()
$this->newClasses = \array_merge($this->newClasses, $this->newExceptions);

$targets = [
\T_USE,
\T_NEW,
\T_CLASS,
\T_ANON_CLASS,
Expand Down Expand Up @@ -1150,9 +1175,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->importedClasses = [];
}

$tokens = $phpcsFile->getTokens();

switch ($tokens[$stackPtr]['code']) {
case \T_USE:
$this->processUseToken($phpcsFile, $stackPtr);
break;

case \T_VARIABLE:
$this->processVariableToken($phpcsFile, $stackPtr);
break;
Expand Down Expand Up @@ -1373,6 +1409,38 @@ private function processCatchToken(File $phpcsFile, $stackPtr)
}
}

/**
* Processes this test for when a use token is encountered.
*
* - Save imported classes 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 (isset($this->newClasses[$lowerFullyQualifiedName])) {
continue;
}

$this->importedClasses[strtolower($name)] = true;
}
}


/**
* Handle the retrieval of relevant information and - if necessary - throwing of an
Expand All @@ -1389,6 +1457,10 @@ private function processCatchToken(File $phpcsFile, $stackPtr)
*/
protected function handleFeature(File $phpcsFile, $stackPtr, array $itemInfo)
{
if (isset($this->importedClasses[$itemInfo['nameLc']])) {
return;
}

$itemArray = $this->newClasses[$itemInfo['nameLc']];
$versionInfo = $this->getVersionInfo($itemArray);

Expand Down
27 changes: 27 additions & 0 deletions PHPCompatibility/Tests/Classes/NewClassesUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,4 +334,31 @@ public function testNoViolationsInFileOnValidVersion()
$file = $this->sniffFile(__FILE__, '99.0'); // High version beyond newest addition.
$this->assertNoViolation($file);
}

/**
* If classes with same name are used in other namespaces, they should not be flagged.
*
* @return void
*/
public function testNoViolationsInFileIfOtherNamespace()
{
$file = $this->sniffFile(__DIR__ . '/NewClassesUsesUnitTest.inc', '4.4');
$sharedRuleSet = $file->ruleset;
$sharedConfig = $file->config;

$forgedLocalFile = new \PHP_CodeSniffer\Files\LocalFile(
realpath(__DIR__ . '/NewClassesUsesNoLeakUnitTest.inc'),
$sharedRuleSet,
$sharedConfig
);
$forgedLocalFile->parse();
$forgedLocalFile->process();

$this->assertNoViolation($file);
$this->assertError(
$forgedLocalFile,
3,
'The built-in class ArrayObject is not present in PHP version 4.4 or earlier'
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

new ArrayObject();
Loading

0 comments on commit a988e5b

Please sign in to comment.