[CodingStyle] Fix lowercase misses in importing, decouple ImportSkipper to collector with ClassNameImportSkipVoters (#4665)

* add import doc block test

* [CodingStyle] Decouple ImportSkipper to ClassNameImportSkipVoters

* drop same short namespace check, not used anywhere
This commit is contained in:
Tomas Votruba 2020-11-20 13:37:53 +00:00 committed by GitHub
parent 7de92da46f
commit 131357aac6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 345 additions and 231 deletions

View File

@ -4,20 +4,13 @@ declare(strict_types=1);
namespace Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer;
use Nette\Utils\Strings;
use PhpParser\Node;
use PhpParser\Node\Stmt\ClassLike;
use PHPStan\PhpDocParser\Ast\Node as PhpDocParserNode;
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\CodingStyle\Imports\ImportSkipper;
use Rector\CodingStyle\ClassNameImport\ClassNameImportSkipper;
use Rector\Core\Configuration\Option;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\ClassExistenceStaticHelper;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
use Rector\PHPStan\Type\ShortenedObjectType;
use Rector\PostRector\Collector\UseNodesToAddCollector;
use Rector\SimplePhpDocParser\PhpDocNodeTraverser;
use Rector\StaticTypeMapper\StaticTypeMapper;
@ -30,11 +23,6 @@ final class DocBlockNameImporter
*/
private $hasPhpDocChanged = false;
/**
* @var bool[][]
*/
private $usedShortNameByClasses = [];
/**
* @var PhpDocNodeTraverser
*/
@ -46,19 +34,9 @@ final class DocBlockNameImporter
private $staticTypeMapper;
/**
* @var NodeNameResolver
* @var ClassNameImportSkipper
*/
private $nodeNameResolver;
/**
* @var BetterStandardPrinter
*/
private $betterStandardPrinter;
/**
* @var ImportSkipper
*/
private $importSkipper;
private $classNameImportSkipper;
/**
* @var ParameterProvider
@ -71,9 +49,7 @@ final class DocBlockNameImporter
private $useNodesToAddCollector;
public function __construct(
BetterStandardPrinter $betterStandardPrinter,
ImportSkipper $importSkipper,
NodeNameResolver $nodeNameResolver,
ClassNameImportSkipper $classNameImportSkipper,
ParameterProvider $parameterProvider,
PhpDocNodeTraverser $phpDocNodeTraverser,
StaticTypeMapper $staticTypeMapper,
@ -81,9 +57,7 @@ final class DocBlockNameImporter
) {
$this->phpDocNodeTraverser = $phpDocNodeTraverser;
$this->staticTypeMapper = $staticTypeMapper;
$this->nodeNameResolver = $nodeNameResolver;
$this->betterStandardPrinter = $betterStandardPrinter;
$this->importSkipper = $importSkipper;
$this->classNameImportSkipper = $classNameImportSkipper;
$this->parameterProvider = $parameterProvider;
$this->useNodesToAddCollector = $useNodesToAddCollector;
}
@ -124,12 +98,10 @@ final class DocBlockNameImporter
IdentifierTypeNode $identifierTypeNode,
FullyQualifiedObjectType $fullyQualifiedObjectType
): PhpDocParserNode {
// nothing to be changed → skip
if ($this->hasTheSameShortClassInCurrentNamespace($node, $fullyQualifiedObjectType)) {
return $identifierTypeNode;
}
if ($this->importSkipper->shouldSkipNameForFullyQualifiedObjectType($node, $fullyQualifiedObjectType)) {
if ($this->classNameImportSkipper->shouldSkipNameForFullyQualifiedObjectType(
$node,
$fullyQualifiedObjectType
)) {
return $identifierTypeNode;
}
@ -150,70 +122,4 @@ final class DocBlockNameImporter
return $shortenedIdentifierTypeNode;
}
/**
* The class in the same namespace as different file can se used in this code, the short names would colide skip
*
* E.g. this namespace:
* App\Product
*
* And the FQN:
* App\SomeNesting\Product
*/
private function hasTheSameShortClassInCurrentNamespace(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
// the name is already in the same namespace implicitly
$namespaceName = $node->getAttribute(AttributeKey::NAMESPACE_NAME);
$currentNamespaceShortName = $namespaceName . '\\' . $fullyQualifiedObjectType->getShortName();
if (ClassExistenceStaticHelper::doesClassLikeExist($currentNamespaceShortName)) {
return false;
}
if ($currentNamespaceShortName === $fullyQualifiedObjectType->getClassName()) {
return false;
}
return $this->isCurrentNamespaceSameShortClassAlreadyUsed(
$node,
$currentNamespaceShortName,
$fullyQualifiedObjectType->getShortNameType()
);
}
private function isCurrentNamespaceSameShortClassAlreadyUsed(
Node $node,
string $fullyQualifiedName,
ShortenedObjectType $shortenedObjectType
): bool {
/** @var ClassLike|null $classLike */
$classLike = $node->getAttribute(AttributeKey::CLASS_NODE);
if ($classLike === null) {
// cannot say, so rather yes
return true;
}
$className = $this->nodeNameResolver->getName($classLike);
if (isset($this->usedShortNameByClasses[$className][$shortenedObjectType->getShortName()])) {
return $this->usedShortNameByClasses[$className][$shortenedObjectType->getShortName()];
}
$printedClass = $this->betterStandardPrinter->print($classLike->stmts);
// short with space " Type"| fqn
$shortNameOrFullyQualifiedNamePattern = sprintf(
'#(\s%s\b|\b%s\b)#',
preg_quote($shortenedObjectType->getShortName(), '#'),
preg_quote($fullyQualifiedName, '#')
);
$isShortClassUsed = (bool) Strings::match($printedClass, $shortNameOrFullyQualifiedNamePattern);
$this->usedShortNameByClasses[$className][$shortenedObjectType->getShortName()] = $isShortClassUsed;
return $isShortClassUsed;
}
}

View File

@ -11,7 +11,7 @@ use PhpParser\Node\Stmt\Namespace_;
use PhpParser\Node\Stmt\Nop;
use PhpParser\Node\Stmt\Use_;
use PHPStan\Type\ObjectType;
use Rector\CodingStyle\Imports\UsedImportsResolver;
use Rector\CodingStyle\ClassNameImport\UsedImportsResolver;
use Rector\PHPStan\Type\AliasedObjectType;
use Rector\PHPStan\Type\FullyQualifiedObjectType;

View File

@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Rector\CodingStyle\Imports;
namespace Rector\CodingStyle\ClassNameImport;
use PhpParser\Node;
use PhpParser\Node\Stmt\Namespace_;

View File

@ -0,0 +1,49 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\ClassNameImport\ClassNameImportSkipVoter;
use Nette\Utils\Strings;
use PhpParser\Node;
use Rector\CodingStyle\ClassNameImport\AliasUsesResolver;
use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
/**
* Prevents adding:
*
* use App\SomeClass;
*
* If there is already:
*
* use App\Something as SomeClass;
*/
final class AliasClassNameImportSkipVoter implements ClassNameImportSkipVoterInterface
{
/**
* @var AliasUsesResolver
*/
private $aliasUsesResolver;
public function __construct(AliasUsesResolver $aliasUsesResolver)
{
$this->aliasUsesResolver = $aliasUsesResolver;
}
public function shouldSkip(FullyQualifiedObjectType $fullyQualifiedObjectType, Node $node): bool
{
$aliasedUses = $this->aliasUsesResolver->resolveForNode($node);
foreach ($aliasedUses as $aliasedUse) {
$aliasedUseLowered = strtolower($aliasedUse);
// its aliased, we cannot just rename it
if (Strings::endsWith($aliasedUseLowered, '\\' . $fullyQualifiedObjectType->getShortNameLowered())) {
return true;
}
}
return false;
}
}

View File

@ -0,0 +1,45 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\ClassNameImport\ClassNameImportSkipVoter;
use PhpParser\Node;
use Rector\CodingStyle\ClassNameImport\ShortNameResolver;
use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
/**
* Prevents adding:
*
* use App\SomeClass;
*
* If there is already:
*
* class SomeClass {}
*/
final class ClassLikeNameClassNameImportSkipVoter implements ClassNameImportSkipVoterInterface
{
/**
* @var ShortNameResolver
*/
private $shortNameResolver;
public function __construct(ShortNameResolver $shortNameResolver)
{
$this->shortNameResolver = $shortNameResolver;
}
public function shouldSkip(FullyQualifiedObjectType $fullyQualifiedObjectType, Node $node): bool
{
$classLikeNames = $this->shortNameResolver->resolveShortClassLikeNamesForNode($node);
foreach ($classLikeNames as $classLikeName) {
if (strtolower($classLikeName) === $fullyQualifiedObjectType->getShortNameLowered()) {
return true;
}
}
return false;
}
}

View File

@ -0,0 +1,48 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\ClassNameImport\ClassNameImportSkipVoter;
use PhpParser\Node;
use Rector\CodingStyle\ClassNameImport\ShortNameResolver;
use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
/**
* Prevents adding:
*
* use App\SomeClass;
*
* If there is already:
*
* SomeClass::callThis();
*/
final class FullyQualifiedNameClassNameImportSkipVoter implements ClassNameImportSkipVoterInterface
{
/**
* @var ShortNameResolver
*/
private $shortNameResolver;
public function __construct(ShortNameResolver $shortNameResolver)
{
$this->shortNameResolver = $shortNameResolver;
}
public function shouldSkip(FullyQualifiedObjectType $fullyQualifiedObjectType, Node $node): bool
{
// "new X" or "X::static()"
$shortNames = $this->shortNameResolver->resolveForNode($node);
foreach ($shortNames as $shortName => $fullyQualifiedName) {
$shortNameLowered = strtolower($shortName);
if ($fullyQualifiedObjectType->getShortNameLowered() !== $shortNameLowered) {
continue;
}
return $fullyQualifiedObjectType->getClassNameLowered() !== strtolower($fullyQualifiedName);
}
return false;
}
}

View File

@ -2,13 +2,21 @@
declare(strict_types=1);
namespace Rector\PostRector\Validator;
namespace Rector\CodingStyle\ClassNameImport\ClassNameImportSkipVoter;
use PhpParser\Node;
use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
use Rector\PostRector\Collector\UseNodesToAddCollector;
final class CanImportBeAddedValidator
/**
* This prevents importing:
* - App\Some\Product
*
* if there is already:
* - use App\Another\Product
*/
final class UsesClassNameImportSkipVoter implements ClassNameImportSkipVoterInterface
{
/**
* @var UseNodesToAddCollector
@ -20,29 +28,23 @@ final class CanImportBeAddedValidator
$this->useNodesToAddCollector = $useNodesToAddCollector;
}
/**
* This prevents importing:
* - App\Some\Product
*
* if there is already:
* - use App\Another\Product
*/
public function canImportBeAdded(Node $node, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
public function shouldSkip(FullyQualifiedObjectType $fullyQualifiedObjectType, Node $node): bool
{
$useImportTypes = $this->useNodesToAddCollector->getUseImportTypesByNode($node);
foreach ($useImportTypes as $useImportType) {
if (! $useImportType->equals($fullyQualifiedObjectType) &&
$useImportType->areShortNamesEqual($fullyQualifiedObjectType)
if (! $useImportType->equals($fullyQualifiedObjectType) && $useImportType->areShortNamesEqual(
$fullyQualifiedObjectType
)
) {
return false;
return true;
}
if ($useImportType->equals($fullyQualifiedObjectType)) {
return true;
return false;
}
}
return true;
return false;
}
}

View File

@ -0,0 +1,38 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\ClassNameImport;
use PhpParser\Node;
use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
final class ClassNameImportSkipper
{
/**
* @var ClassNameImportSkipVoterInterface[]
*/
private $classNameImportSkipVoters = [];
/**
* @param ClassNameImportSkipVoterInterface[] $classNameImportSkipVoters
*/
public function __construct(array $classNameImportSkipVoters)
{
$this->classNameImportSkipVoters = $classNameImportSkipVoters;
}
public function shouldSkipNameForFullyQualifiedObjectType(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
foreach ($this->classNameImportSkipVoters as $classNameImportSkipVoter) {
if ($classNameImportSkipVoter->shouldSkip($fullyQualifiedObjectType, $node)) {
return true;
}
}
return false;
}
}

View File

@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Rector\CodingStyle\Imports;
namespace Rector\CodingStyle\ClassNameImport;
use Nette\Utils\Strings;
use PhpParser\Node;

View File

@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Rector\CodingStyle\Imports;
namespace Rector\CodingStyle\ClassNameImport;
use PhpParser\Node;
use PhpParser\Node\Stmt;

View File

@ -2,7 +2,7 @@
declare(strict_types=1);
namespace Rector\CodingStyle\Imports;
namespace Rector\CodingStyle\ClassNameImport;
use PhpParser\Node;
use PhpParser\Node\Stmt;

View File

@ -0,0 +1,13 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Contract\ClassNameImport;
use PhpParser\Node;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
interface ClassNameImportSkipVoterInterface
{
public function shouldSkip(FullyQualifiedObjectType $fullyQualifiedObjectType, Node $node): bool;
}

View File

@ -1,99 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Imports;
use Nette\Utils\Strings;
use PhpParser\Node;
use Rector\PHPStan\Type\FullyQualifiedObjectType;
use Rector\PostRector\Validator\CanImportBeAddedValidator;
final class ImportSkipper
{
/**
* @var AliasUsesResolver
*/
private $aliasUsesResolver;
/**
* @var ShortNameResolver
*/
private $shortNameResolver;
/**
* @var CanImportBeAddedValidator
*/
private $canImportBeAddedValidator;
public function __construct(
AliasUsesResolver $aliasUsesResolver,
CanImportBeAddedValidator $canImportBeAddedValidator,
ShortNameResolver $shortNameResolver
) {
$this->aliasUsesResolver = $aliasUsesResolver;
$this->shortNameResolver = $shortNameResolver;
$this->canImportBeAddedValidator = $canImportBeAddedValidator;
}
public function shouldSkipNameForFullyQualifiedObjectType(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
if ($this->isShortNameAlreadyUsedForDifferentFullyQualifiedName($node, $fullyQualifiedObjectType)) {
return true;
}
if ($this->isShortNameAlreadyUsedInImportAlias($node, $fullyQualifiedObjectType)) {
return true;
}
if ($this->isNameAlreadyUsedInClassLikeName($node, $fullyQualifiedObjectType)) {
return true;
}
return ! $this->canImportBeAddedValidator->canImportBeAdded($node, $fullyQualifiedObjectType);
}
private function isShortNameAlreadyUsedForDifferentFullyQualifiedName(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
// "new X" or "X::static()"
$shortNames = $this->shortNameResolver->resolveForNode($node);
foreach ($shortNames as $shortName => $fullyQualifiedName) {
if ($fullyQualifiedObjectType->getShortName() !== $shortName) {
continue;
}
return $fullyQualifiedObjectType->getClassName() !== $fullyQualifiedName;
}
return false;
}
private function isShortNameAlreadyUsedInImportAlias(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
$aliasedUses = $this->aliasUsesResolver->resolveForNode($node);
foreach ($aliasedUses as $aliasedUse) {
// its aliased, we cannot just rename it
if (Strings::endsWith($aliasedUse, '\\' . $fullyQualifiedObjectType->getShortName())) {
return true;
}
}
return false;
}
private function isNameAlreadyUsedInClassLikeName(
Node $node,
FullyQualifiedObjectType $fullyQualifiedObjectType
): bool {
$classLikeNames = $this->shortNameResolver->resolveShortClassLikeNamesForNode($node);
return in_array($fullyQualifiedObjectType->getShortName(), $classLikeNames, true);
}
}

View File

@ -0,0 +1,8 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle;
final class Node
{
}

View File

@ -10,8 +10,8 @@ use PhpParser\Node\Name;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\Node\Stmt\UseUse;
use Rector\CodingStyle\Imports\AliasUsesResolver;
use Rector\CodingStyle\Imports\ImportSkipper;
use Rector\CodingStyle\ClassNameImport\AliasUsesResolver;
use Rector\CodingStyle\ClassNameImport\ClassNameImportSkipper;
use Rector\Core\Configuration\Option;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\ClassExistenceStaticHelper;
@ -40,9 +40,9 @@ final class NameImporter
private $aliasUsesResolver;
/**
* @var ImportSkipper
* @var ClassNameImportSkipper
*/
private $importSkipper;
private $classNameImportSkipper;
/**
* @var NodeNameResolver
@ -66,7 +66,7 @@ final class NameImporter
public function __construct(
AliasUsesResolver $aliasUsesResolver,
ImportSkipper $importSkipper,
ClassNameImportSkipper $classNameImportSkipper,
NodeNameResolver $nodeNameResolver,
ParameterProvider $parameterProvider,
RenamedClassesCollector $renamedClassesCollector,
@ -75,7 +75,7 @@ final class NameImporter
) {
$this->staticTypeMapper = $staticTypeMapper;
$this->aliasUsesResolver = $aliasUsesResolver;
$this->importSkipper = $importSkipper;
$this->classNameImportSkipper = $classNameImportSkipper;
$this->nodeNameResolver = $nodeNameResolver;
$this->parameterProvider = $parameterProvider;
$this->useNodesToAddCollector = $useNodesToAddCollector;
@ -141,7 +141,10 @@ final class NameImporter
FullyQualifiedObjectType $fullyQualifiedObjectType
): ?Name {
// the same end is already imported → skip
if ($this->importSkipper->shouldSkipNameForFullyQualifiedObjectType($name, $fullyQualifiedObjectType)) {
if ($this->classNameImportSkipper->shouldSkipNameForFullyQualifiedObjectType(
$name,
$fullyQualifiedObjectType
)) {
return null;
}

View File

@ -0,0 +1,8 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Node;
final class Stmt
{
}

View File

@ -5,7 +5,7 @@ declare(strict_types=1);
namespace Rector\CodingStyle\Node;
use PhpParser\Node\Stmt\Use_;
use Rector\CodingStyle\Imports\ShortNameResolver;
use Rector\CodingStyle\ClassNameImport\ShortNameResolver;
use Rector\CodingStyle\Naming\ClassNaming;
final class UseNameAliasToNameResolver

View File

@ -0,0 +1,39 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector;
use Iterator;
use Rector\Core\Configuration\Option;
use Rector\Renaming\Rector\Name\RenameClassRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;
/**
* @see \Rector\PostRector\Rector\NameImportingPostRector
*/
final class DocBlockRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->setParameter(Option::AUTO_IMPORT_NAMES, true);
$this->setParameter(Option::IMPORT_DOC_BLOCKS, true);
$this->doTestFileInfo($fileInfo);
}
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/FixtureDocBlock');
}
protected function getRectorClass(): string
{
// here can be any Rector rule, as we're testing \Rector\PostRector\Rector\NameImportingPostRector in the full Retcor life cycle
return RenameClassRector::class;
}
}

View File

@ -0,0 +1,14 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\FixtureDocBlock;
class SkipAlreadyClassNameInInlinedVarDoc
{
public function run($anotherVariable)
{
/** @var \Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\SkipAlreadyClassNameInInlinedVarDoc $typedVariable */
$typedVariable = $anotherVariable;
}
}

View File

@ -0,0 +1,13 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\FixtureDocBlock;
class SkipalreadyclassnameinvarDoc
{
/**
* @var \Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\SkipAlreadyClassNameInVarDoc $result
*/
public $some;
}

View File

@ -21,7 +21,6 @@ final class ImportRootNamespaceClassesDisabledTest extends AbstractRectorTestCas
public function test(SmartFileInfo $fileInfo): void
{
$this->setParameter(Option::AUTO_IMPORT_NAMES, true);
$this->setParameter(Option::IMPORT_SHORT_CLASSES, false);
$this->doTestFileInfo($fileInfo);

View File

@ -0,0 +1,9 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source;
final class SkipAlreadyClassNameInInlinedVarDoc
{
}

View File

@ -0,0 +1,9 @@
<?php
declare(strict_types=1);
namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source;
final class SkipAlreadyClassNameInVarDoc
{
}

View File

@ -62,4 +62,14 @@ final class FullyQualifiedObjectType extends ObjectType
return new Use_([$useUse]);
}
public function getShortNameLowered(): string
{
return strtolower($this->getShortName());
}
public function getClassNameLowered(): string
{
return strtolower($this->getClassName());
}
}