Skip to content

Commit

Permalink
Fix docs double import (#478)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TomasVotruba
TomasVotruba committed Jul 21, 2021
1 parent 242fa16 commit 24a32df
Show file tree
Hide file tree
Showing 16 changed files with 158 additions and 117 deletions.
5 changes: 3 additions & 2 deletions packages/NodeTypeResolver/PHPStan/Type/TypeFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ public function createMixedPassedOrUnionType(array $types): Type
}

/**
* @param Type[] $types
* @return Type[]
* @template TType as Type
* @param array<TType> $types
* @return array<TType>
*/
public function uniquateTypes(array $types, bool $keepConstant = false): array
{
Expand Down
69 changes: 50 additions & 19 deletions packages/NodeTypeResolver/PhpDocNodeVisitor/NameImportingPhpDocNodeVisitor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use Rector\CodingStyle\ClassNameImport\ClassNameImportSkipper;
use Rector\Core\Configuration\Option;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\Provider\CurrentFileProvider;
use Rector\Core\ValueObject\Application\File;
use Rector\PostRector\Collector\UseNodesToAddCollector;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType;
Expand All @@ -29,7 +31,8 @@ public function __construct(
private StaticTypeMapper $staticTypeMapper,
private ParameterProvider $parameterProvider,
private ClassNameImportSkipper $classNameImportSkipper,
private UseNodesToAddCollector $useNodesToAddCollector
private UseNodesToAddCollector $useNodesToAddCollector,
private CurrentFileProvider $currentFileProvider
) {
}

Expand Down Expand Up @@ -68,7 +71,12 @@ public function enterNode(Node $node): ?Node
return null;
}

return $this->processFqnNameImport($this->currentPhpParserNode, $node, $staticType);
$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return null;
}

return $this->processFqnNameImport($this->currentPhpParserNode, $node, $staticType, $file);
}

public function setCurrentNode(PhpParserNode $phpParserNode): void
Expand All @@ -79,13 +87,15 @@ public function setCurrentNode(PhpParserNode $phpParserNode): void
private function processFqnNameImport(
PhpParserNode $phpParserNode,
IdentifierTypeNode $identifierTypeNode,
FullyQualifiedObjectType $fullyQualifiedObjectType
FullyQualifiedObjectType $fullyQualifiedObjectType,
File $file
): ?IdentifierTypeNode {
if ($this->classNameImportSkipper->shouldSkipNameForFullyQualifiedObjectType(
$file,
$phpParserNode,
$fullyQualifiedObjectType
)) {
return $identifierTypeNode;
return null;
}

$parent = $identifierTypeNode->getAttribute(PhpDocAttributeKey::PARENT);
Expand All @@ -94,28 +104,33 @@ private function processFqnNameImport(
return null;
}

$newNode = new IdentifierTypeNode($fullyQualifiedObjectType->getShortName());

// should skip because its already used
if ($this->useNodesToAddCollector->isShortImported($phpParserNode, $fullyQualifiedObjectType)) {
if ($this->useNodesToAddCollector->isImportShortable($phpParserNode, $fullyQualifiedObjectType)) {
$newNode = new IdentifierTypeNode($fullyQualifiedObjectType->getShortName());
if ($newNode->name !== $identifierTypeNode->name) {
return $newNode;
}

return $identifierTypeNode;
if ($this->useNodesToAddCollector->isShortImported($file, $fullyQualifiedObjectType)) {
if (! $this->useNodesToAddCollector->isImportShortable($file, $fullyQualifiedObjectType)) {
return null;
}

return $identifierTypeNode;
}
if ($newNode->name !== $identifierTypeNode->name) {
$this->useNodesToAddCollector->addUseImport($fullyQualifiedObjectType);
return $newNode;
}

$this->useNodesToAddCollector->addUseImport($fullyQualifiedObjectType);
return null;
}

$newNode = new IdentifierTypeNode($fullyQualifiedObjectType->getShortName());
if ($newNode->name !== $identifierTypeNode->name) {
// do not import twice
if ($this->useNodesToAddCollector->isShortImported($file, $fullyQualifiedObjectType)) {
return null;
}

$this->useNodesToAddCollector->addUseImport($fullyQualifiedObjectType);
return $newNode;
}

return $identifierTypeNode;
return null;
}

private function shouldSkipShortClassName(FullyQualifiedObjectType $fullyQualifiedObjectType): bool
Expand Down Expand Up @@ -145,10 +160,16 @@ private function processDoctrineAnnotationTagValueNode(
$staticType = new FullyQualifiedObjectType($staticType->getClassName());
}

$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return;
}

$shortentedIdentifierTypeNode = $this->processFqnNameImport(
$this->currentPhpParserNode,
$identifierTypeNode,
$staticType
$staticType,
$file
);

if (! $shortentedIdentifierTypeNode instanceof IdentifierTypeNode) {
Expand Down Expand Up @@ -187,7 +208,17 @@ private function enterSpacelessPhpDocTagNode(
$staticType = new FullyQualifiedObjectType($staticType->getClassName());
}

$importedName = $this->processFqnNameImport($this->currentPhpParserNode, $identifierTypeNode, $staticType);
$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return null;
}

$importedName = $this->processFqnNameImport(
$this->currentPhpParserNode,
$identifierTypeNode,
$staticType,
$file
);
if ($importedName !== null) {
$spacelessPhpDocTagNode->name = '@' . $importedName->name;
return $spacelessPhpDocTagNode;
Expand Down
72 changes: 16 additions & 56 deletions packages/PostRector/Collector/UseNodesToAddCollector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@

final class UseNodesToAddCollector implements NodeCollectorInterface
{
/**
* @var string[][]
*/
private array $removedShortUsesInFilePath = [];

/**
* @var array<string, FullyQualifiedObjectType[]>
*/
Expand Down Expand Up @@ -57,30 +52,13 @@ public function addFunctionUseImport(Node $node, FullyQualifiedObjectType $fully
$this->functionUseImportTypesInFilePath[$smartFileInfo->getRealPath()][] = $fullyQualifiedObjectType;
}

public function removeShortUse(Node $node, string $shortUse): void
{
$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return;
}

$smartFileInfo = $file->getSmartFileInfo();

$this->removedShortUsesInFilePath[$smartFileInfo->getRealPath()][] = $shortUse;
}

public function clear(SmartFileInfo $smartFileInfo): void
{
// clear applied imports, so isActive() doesn't return any false positives
unset($this->useImportTypesInFilePath[$smartFileInfo->getRealPath()], $this->functionUseImportTypesInFilePath[$smartFileInfo->getRealPath()]);
}

/**
* @return AliasedObjectType[]|FullyQualifiedObjectType[]
*/
public function getUseImportTypesByNode(Node $node): array
public function getUseImportTypesByNode(File $file, Node $node): array
{
$filePath = $this->getRealPathFromNode();
$fileInfo = $file->getSmartFileInfo();
$filePath = $fileInfo->getRealPath();

$objectTypes = $this->useImportTypesInFilePath[$filePath] ?? [];

Expand All @@ -99,9 +77,9 @@ public function getUseImportTypesByNode(Node $node): array
return $objectTypes;
}

public function hasImport(Node $node, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
public function hasImport(File $file, Node $node, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
{
$useImports = $this->getUseImportTypesByNode($node);
$useImports = $this->getUseImportTypesByNode($file, $node);

foreach ($useImports as $useImport) {
if ($useImport->equals($fullyQualifiedObjectType)) {
Expand All @@ -112,16 +90,14 @@ public function hasImport(Node $node, FullyQualifiedObjectType $fullyQualifiedOb
return false;
}

public function isShortImported(Node $node, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
public function isShortImported(File $file, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
{
$filePath = $this->getRealPathFromNode();
if ($filePath === null) {
return false;
}
$fileInfo = $file->getSmartFileInfo();
$filePath = $fileInfo->getRealPath();

$shortName = $fullyQualifiedObjectType->getShortName();

if ($this->isShortClassImported($filePath, $shortName)) {
if ($this->isShortClassImported($file, $shortName)) {
return true;
}

Expand All @@ -135,9 +111,10 @@ public function isShortImported(Node $node, FullyQualifiedObjectType $fullyQuali
return false;
}

public function isImportShortable(Node $node, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
public function isImportShortable(File $file, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
{
$filePath = $this->getRealPathFromNode();
$fileInfo = $file->getSmartFileInfo();
$filePath = $fileInfo->getRealPath();

$fileUseImportTypes = $this->useImportTypesInFilePath[$filePath] ?? [];

Expand Down Expand Up @@ -173,29 +150,12 @@ public function getFunctionImportsByFileInfo(SmartFileInfo $smartFileInfo): arra
return $this->functionUseImportTypesInFilePath[$smartFileInfo->getRealPath()] ?? [];
}

/**
* @return string[]
*/
public function getShortUsesByFileInfo(SmartFileInfo $smartFileInfo): array
private function isShortClassImported(File $file, string $shortName): bool
{
return $this->removedShortUsesInFilePath[$smartFileInfo->getRealPath()] ?? [];
}
$fileInfo = $file->getSmartFileInfo();
$realPath = $fileInfo->getRealPath();

private function getRealPathFromNode(): ?string
{
$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return null;
}

$smartFileInfo = $file->getSmartFileInfo();

return $smartFileInfo->getRealPath();
}

private function isShortClassImported(string $filePath, string $shortName): bool
{
$fileUseImports = $this->useImportTypesInFilePath[$filePath] ?? [];
$fileUseImports = $this->useImportTypesInFilePath[$realPath] ?? [];

foreach ($fileUseImports as $fileUseImport) {
if ($fileUseImport->getShortName() === $shortName) {
Expand Down
24 changes: 16 additions & 8 deletions packages/PostRector/Rector/NameImportingPostRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,22 +102,30 @@ private function processNodeName(Name $name, File $file): ?Node
/** @var Use_[] $currentUses */
$currentUses = $this->betterNodeFinder->findInstanceOf($file->getNewStmts(), Use_::class);

if ($this->shouldImportName($name, $file, $currentUses)) {
return $this->nameImporter->importName($name, $file, $currentUses);
}

return null;
}

/**
* @param Use_[] $currentUses
*/
private function shouldImportName(Name $name, File $file, array $currentUses): bool
{
if (substr_count($name->toCodeString(), '\\') <= 1) {
return $this->nameImporter->importName($name, $currentUses);
return true;
}

if (! $this->classNameImportSkipper->isFoundInUse($name, $currentUses)) {
return $this->nameImporter->importName($name, $currentUses);
return true;
}

if ($this->classNameImportSkipper->isAlreadyImported($name, $currentUses)) {
return $this->nameImporter->importName($name, $currentUses);
}

if ($this->reflectionProvider->hasFunction(new Name($name->getLast()), null)) {
return $this->nameImporter->importName($name, $currentUses);
return true;
}

return null;
return $this->reflectionProvider->hasFunction(new Name($name->getLast()), null);
}
}
11 changes: 4 additions & 7 deletions packages/PostRector/Rector/UseAddingPostRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function __construct(
private UseImportsRemover $useImportsRemover,
private UseNodesToAddCollector $useNodesToAddCollector,
private CurrentFileProvider $currentFileProvider,
private RenamedClassesDataCollector $renamedClassesDataCollector
private RenamedClassesDataCollector $renamedClassesDataCollector,
) {
}

Expand All @@ -47,25 +47,22 @@ public function beforeTraverse(array $nodes): array

$useImportTypes = $this->useNodesToAddCollector->getObjectImportsByFileInfo($smartFileInfo);
$functionUseImportTypes = $this->useNodesToAddCollector->getFunctionImportsByFileInfo($smartFileInfo);
$removedShortUses = $this->useNodesToAddCollector->getShortUsesByFileInfo($smartFileInfo);

$oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();

// nothing to import or remove
if ($useImportTypes === [] && $functionUseImportTypes === [] && $removedShortUses === [] && $oldToNewClasses === []) {
if ($useImportTypes === [] && $functionUseImportTypes === [] && $oldToNewClasses === []) {
return $nodes;
}

/** @var FullyQualifiedObjectType[] $useImportTypes */
$useImportTypes = $this->typeFactory->uniquateTypes($useImportTypes);

$this->useNodesToAddCollector->clear($smartFileInfo);

// A. has namespace? add under it
$namespace = $this->betterNodeFinder->findFirstInstanceOf($nodes, Namespace_::class);
if ($namespace instanceof Namespace_) {
// first clean
$this->useImportsRemover->removeImportsFromNamespace($namespace, $removedShortUses);
//$this->useImportsRemover->removeImportsFromNamespace($namespace, $removedShortUses);
// then add, to prevent adding + removing false positive of same short use
$this->useImportsAdder->addImportsToNamespace($namespace, $useImportTypes, $functionUseImportTypes);

Expand All @@ -77,7 +74,7 @@ public function beforeTraverse(array $nodes): array
$nodes = $firstNode->stmts;
}

$removedShortUses = array_merge($removedShortUses, $this->renamedClassesDataCollector->getOldClasses());
$removedShortUses = $this->renamedClassesDataCollector->getOldClasses();

// B. no namespace? add in the top
// first clean
Expand Down
7 changes: 5 additions & 2 deletions packages/StaticTypeMapper/ValueObject/Type/FullyQualifiedObjectType.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ public function getUseNode(): Use_
public function getFunctionUseNode(): Use_
{
$name = new Name($this->getClassName());
$useUse = new UseUse($name, null, Use_::TYPE_FUNCTION);
$useUse = new UseUse($name, null);

$name->setAttribute(AttributeKey::PARENT_NODE, $useUse);

return new Use_([$useUse]);
$use = new Use_([$useUse]);
$use->type = Use_::TYPE_FUNCTION;

return $use;
}

public function getShortNameLowered(): string
Expand Down
8 changes: 3 additions & 5 deletions phpstan.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,12 +551,10 @@ parameters:
- '#Cognitive complexity for "Rector\\Php80\\Rector\\Class_\\AnnotationToAttributeRector\:\:processDoctrineAnnotationClasses\(\)" is 11, keep it under 9#'
- '#Method Rector\\NodeNestingScope\\ParentFinder\:\:findByTypes\(\) should return T of PhpParser\\Node\|null but returns class\-string<T of PhpParser\\Node\>\|T of PhpParser\\Node#'

# -
# message: src/PhpParser/Node/BetterNodeFinder.php
# paths:
# -

-
message: '#Instead of "ReflectionFunction" class/interface use "PHPStan\\Reflection\\FunctionReflection"#'
paths:
- rules/DowngradePhp80/NodeAnalyzer/UnnamedArgumentResolver.php #45

# array_index on generic types
- '#Method Rector\\NodeTypeResolver\\PHPStan\\Type\\TypeFactory\:\:uniquateTypes\(\) should return array<TType of PHPStan\\Type\\Type\> but returns array<int, PHPStan\\Type\\Type\>#'
Loading

0 comments on commit 24a32df

Please sign in to comment.