From ec0010d39952836c750ec380085829e62b03c85c Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Tue, 3 Nov 2026 10:23:15 +0000 Subject: [PATCH] Write class names added by fixes relative to namespace and uses NewClassNamesVisitor runs after every fix and writes the fully qualified class names a fix added, in PHP code and in PHPDocs, relative to the current namespace and existing use statements. With fixer.importNames (on by default) it adds a use statement for a name from another namespace unless its short name already means something in the file. fixer.preferSelf writes the current class as self. The migrate:neon2attributes command now builds fully qualified names and lets the visitor import them, replacing ImportScope. Amp-Thread-ID: https://ampcode.com/threads/T-01a179f2-995f-7745-8d8c-4c174ba5c0ce --- conf/config.neon | 3 + conf/parametersSchema.neon | 4 + src/Analyser/RuleErrorTransformer.php | 14 + src/Command/Neon2Attributes/ImportScope.php | 120 ----- .../PhpClassAttributesWriter.php | 84 ++-- src/Fixable/NewClassNamesVisitor.php | 476 ++++++++++++++++++ .../data/skip-tests-requires-php.php.fixed | 2 +- .../AddNamesFullyQualifiedRuleTest.php | 39 ++ tests/PHPStan/Fixable/AddNamesRule.php | 80 +++ tests/PHPStan/Fixable/AddNamesRuleTest.php | 40 ++ .../Fixable/data/add-names-first-use.php | 17 + .../data/add-names-first-use.php.fixed | 20 + .../data/add-names-fully-qualified.php.fixed | 32 ++ ...add-names-global-fully-qualified.php.fixed | 15 + .../PHPStan/Fixable/data/add-names-global.php | 15 + .../Fixable/data/add-names-global.php.fixed | 17 + .../Fixable/data/add-names-sorted-uses.php | 26 + .../data/add-names-sorted-uses.php.fixed | 29 ++ tests/PHPStan/Fixable/data/add-names.php | 32 ++ .../PHPStan/Fixable/data/add-names.php.fixed | 33 ++ .../Fixable/fully-qualified-names.neon | 4 + .../Methods/OverridingMethodRuleTest.php | 14 + .../data/fix-override-attribute-global.php | 21 + .../fix-override-attribute-global.php.fixed | 22 + .../data/fix-override-attribute-imported.php | 35 ++ .../fix-override-attribute-imported.php.fixed | 37 ++ 26 files changed, 1056 insertions(+), 175 deletions(-) delete mode 100644 src/Command/Neon2Attributes/ImportScope.php create mode 100644 src/Fixable/NewClassNamesVisitor.php create mode 100644 tests/PHPStan/Fixable/AddNamesFullyQualifiedRuleTest.php create mode 100644 tests/PHPStan/Fixable/AddNamesRule.php create mode 100644 tests/PHPStan/Fixable/AddNamesRuleTest.php create mode 100644 tests/PHPStan/Fixable/data/add-names-first-use.php create mode 100644 tests/PHPStan/Fixable/data/add-names-first-use.php.fixed create mode 100644 tests/PHPStan/Fixable/data/add-names-fully-qualified.php.fixed create mode 100644 tests/PHPStan/Fixable/data/add-names-global-fully-qualified.php.fixed create mode 100644 tests/PHPStan/Fixable/data/add-names-global.php create mode 100644 tests/PHPStan/Fixable/data/add-names-global.php.fixed create mode 100644 tests/PHPStan/Fixable/data/add-names-sorted-uses.php create mode 100644 tests/PHPStan/Fixable/data/add-names-sorted-uses.php.fixed create mode 100644 tests/PHPStan/Fixable/data/add-names.php create mode 100644 tests/PHPStan/Fixable/data/add-names.php.fixed create mode 100644 tests/PHPStan/Fixable/fully-qualified-names.neon create mode 100644 tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php create mode 100644 tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php.fixed create mode 100644 tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php create mode 100644 tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php.fixed diff --git a/conf/config.neon b/conf/config.neon index e62bd4dc7b5..5174d60f881 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -248,6 +248,9 @@ parameters: errorFormat: null sysGetTempDir: ::sys_get_temp_dir() sourceLocatorPlaygroundMode: false + fixer: + importNames: true + preferSelf: false pro: dnsServers: - '1.1.1.2' diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index f7a62b602a6..fbf06496f89 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -189,6 +189,10 @@ parametersSchema: editorUrl: schema(string(), nullable()) editorUrlTitle: schema(string(), nullable()) errorFormat: schema(string(), nullable()) + fixer: structure([ + importNames: bool(), + preferSelf: bool() + ]) pro: structure([ dnsServers: schema(listOf(string()), min(1)), tmpDir: string() diff --git a/src/Analyser/RuleErrorTransformer.php b/src/Analyser/RuleErrorTransformer.php index 9bc3bb82609..ef730cac9f4 100644 --- a/src/Analyser/RuleErrorTransformer.php +++ b/src/Analyser/RuleErrorTransformer.php @@ -2,16 +2,19 @@ namespace PHPStan\Analyser; +use PhpParser\ErrorHandler\Collecting; use PhpParser\Internal\TokenStream; use PhpParser\Node; use PhpParser\Node\Stmt; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\CloningVisitor; +use PhpParser\NodeVisitor\NameResolver; use PhpParser\Parser; use PhpParser\Token; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\File\FileReader; +use PHPStan\Fixable\NewClassNamesVisitor; use PHPStan\Fixable\PhpPrinter; use PHPStan\Fixable\PhpPrinterIndentationDetectorVisitor; use PHPStan\Fixable\ReplacingNodeVisitor; @@ -68,6 +71,10 @@ public function __construct( private Parser $parser, #[AutowiredParameter(ref: '@defaultAnalysisParser')] private AnalysisParser $analysisParser, + #[AutowiredParameter(ref: '%fixer.importNames%')] + private bool $importNames, + #[AutowiredParameter(ref: '%fixer.preferSelf%')] + private bool $preferSelf, ) { $this->diffOutputBuilder = new UnifiedDiffOutputBuilder('', addLineNumbers: true); @@ -239,6 +246,13 @@ private function fix(string $fixingFile, array $fileNodes, array $fixes): ?Fixed return null; } + $nameResolver = new NameResolver(new Collecting(), ['replaceNodes' => false]); + /** @var Stmt[] $newStmts */ + $newStmts = (new NodeTraverser( + $nameResolver, + new NewClassNamesVisitor($nameResolver->getNameContext(), $this->importNames, $this->preferSelf), + ))->traverse($newStmts); + $newCode = $this->getPrinter($indent)->printFormatPreserving($newStmts, $fileNodes, $parsedFile['tokens']); if ($parsedFile['code'] === $newCode) { return null; diff --git a/src/Command/Neon2Attributes/ImportScope.php b/src/Command/Neon2Attributes/ImportScope.php deleted file mode 100644 index ae67ebbfc89..00000000000 --- a/src/Command/Neon2Attributes/ImportScope.php +++ /dev/null @@ -1,120 +0,0 @@ - imported class names by lowercase alias */ - public array $uses = []; - - /** @var array lowercase first parts of names that resolve against the namespace, and of classes declared in it */ - public array $unqualifiedNames = []; - - /** - * @param list $stmts - */ - public function __construct(public string $namespace, public array $stmts) - { - foreach ($stmts as $stmt) { - if ($stmt instanceof Node\Stmt\Use_ && $stmt->type === Node\Stmt\Use_::TYPE_NORMAL) { - foreach ($stmt->uses as $use) { - $this->uses[strtolower($use->getAlias()->toString())] = $use->name->toString(); - } - } elseif ($stmt instanceof Node\Stmt\GroupUse) { - foreach ($stmt->uses as $use) { - if ($stmt->type !== Node\Stmt\Use_::TYPE_NORMAL && $use->type !== Node\Stmt\Use_::TYPE_NORMAL) { - continue; - } - $this->uses[strtolower($use->getAlias()->toString())] = $stmt->prefix->toString() . '\\' . $use->name->toString(); - } - } elseif ($stmt instanceof Node\Stmt\ClassLike && $stmt->name !== null) { - $this->unqualifiedNames[strtolower($stmt->name->toString())] = true; - } - } - - $nodeFinder = new NodeFinder(); - $functionAndConstantNames = []; - foreach ($nodeFinder->find($stmts, static fn (Node $node): bool => $node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\ConstFetch) as $node) { - if (!$node instanceof Node\Expr\FuncCall && !$node instanceof Node\Expr\ConstFetch) { - continue; - } - $functionAndConstantNames[spl_object_id($node->name)] = true; - } - $nonUseStmts = array_filter($stmts, static fn (Node\Stmt $stmt): bool => !$stmt instanceof Node\Stmt\Use_ && !$stmt instanceof Node\Stmt\GroupUse); - foreach ($nodeFinder->findInstanceOf($nonUseStmts, Node\Name::class) as $name) { - if (!$name->isUnqualified() && !$name->isQualified()) { - continue; - } - if (array_key_exists(spl_object_id($name), $functionAndConstantNames)) { - continue; - } - - $this->unqualifiedNames[strtolower($name->getFirst())] = true; - } - } - - /** - * Inserts the use statement in alphabetical order among the existing ones. - * - * @param non-empty-string $className - */ - public function addUse(string $className, string $lowerAlias): void - { - $this->uses[$lowerAlias] = $className; - $use = new Node\Stmt\Use_([new Node\UseItem(new Node\Name($className))]); - - $position = null; - $lastClassUse = null; - $firstOtherUse = null; - $lastDeclare = null; - foreach ($this->stmts as $i => $stmt) { - if ($stmt instanceof Node\Stmt\Declare_) { - $lastDeclare = $i; - continue; - } - if (!$stmt instanceof Node\Stmt\Use_) { - continue; - } - if ($stmt->type !== Node\Stmt\Use_::TYPE_NORMAL) { - $firstOtherUse ??= $i; - continue; - } - - $lastClassUse = $i; - if ($position !== null || count($stmt->uses) !== 1 || strcasecmp($stmt->uses[0]->name->toString(), $className) <= 0) { - continue; - } - - $position = $i; - } - - if ($position === null) { - if ($lastClassUse !== null) { - $position = $lastClassUse + 1; - } elseif ($firstOtherUse !== null) { - $position = $firstOtherUse; - } else { - // the first use statement, separated from the code below it by an empty line - array_splice($this->stmts, $lastDeclare !== null ? $lastDeclare + 1 : 0, 0, [$use, new Node\Stmt\Nop()]); - return; - } - } - - array_splice($this->stmts, $position, 0, [$use]); - } - -} diff --git a/src/Command/Neon2Attributes/PhpClassAttributesWriter.php b/src/Command/Neon2Attributes/PhpClassAttributesWriter.php index 235ed7d55e5..bbe2977ee90 100644 --- a/src/Command/Neon2Attributes/PhpClassAttributesWriter.php +++ b/src/Command/Neon2Attributes/PhpClassAttributesWriter.php @@ -2,18 +2,20 @@ namespace PHPStan\Command\Neon2Attributes; +use PhpParser\ErrorHandler\Collecting; use PhpParser\Internal\TokenStream; use PhpParser\Node; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\CloningVisitor; +use PhpParser\NodeVisitor\NameResolver; use PhpParser\ParserFactory; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\File\FileReader; +use PHPStan\Fixable\NewClassNamesVisitor; use PHPStan\Fixable\PhpPrinter; use PHPStan\Fixable\PhpPrinterIndentationDetectorVisitor; use PHPStan\ShouldNotHappenException; use function array_is_list; -use function array_key_exists; use function count; use function is_array; use function is_bool; @@ -24,9 +26,6 @@ use function str_contains; use function str_repeat; use function strcasecmp; -use function strrpos; -use function strtolower; -use function substr; /** * Adds attributes to classes and their constructor parameters, keeping the rest of the file as it was. @@ -62,20 +61,27 @@ public function write(string $file, array $edits): string $this->addAttributes($file, $newStmts, $edit); } + $nameResolver = new NameResolver(new Collecting(), ['replaceNodes' => false]); + /** @var list $newStmts */ + $newStmts = (new NodeTraverser( + $nameResolver, + new NewClassNamesVisitor($nameResolver->getNameContext(), importNames: true, preferSelf: false), + ))->traverse($newStmts); + return (new AttributesPrinter(['indent' => $indent]))->printFormatPreserving($newStmts, $oldStmts, $tokens); } /** * @param list $stmts */ - private function addAttributes(string $file, array &$stmts, ClassAttributes $edit): void + private function addAttributes(string $file, array $stmts, ClassAttributes $edit): void { $className = $edit->reflection->getName(); $namespace = $edit->reflection->getNamespaceName(); $shortName = $edit->reflection->getShortName(); if ($namespace === '') { - $this->addAttributesInScope($stmts, '', $shortName, $edit); + $this->addAttributesInScope($stmts, $shortName, $edit); return; } @@ -83,7 +89,7 @@ private function addAttributes(string $file, array &$stmts, ClassAttributes $edi if (!$stmt instanceof Node\Stmt\Namespace_ || $stmt->name === null || strcasecmp($stmt->name->toString(), $namespace) !== 0) { continue; } - if ($this->addAttributesInScope($stmt->stmts, $namespace, $shortName, $edit)) { + if ($this->addAttributesInScope($stmt->stmts, $shortName, $edit)) { return; } } @@ -94,7 +100,7 @@ private function addAttributes(string $file, array &$stmts, ClassAttributes $edi /** * @param list $stmts statements of a namespace, or of a file without one */ - private function addAttributesInScope(array &$stmts, string $namespace, string $shortName, ClassAttributes $edit): bool + private function addAttributesInScope(array $stmts, string $shortName, ClassAttributes $edit): bool { $class = null; foreach ($stmts as $stmt) { @@ -109,8 +115,7 @@ private function addAttributesInScope(array &$stmts, string $namespace, string $ return false; } - $scope = new ImportScope($namespace, $stmts); - $class->attrGroups[] = new Node\AttributeGroup([$this->createAttribute($scope, $edit->attribute)]); + $class->attrGroups[] = new Node\AttributeGroup([$this->createAttribute($edit->attribute)]); $constructor = $class->getMethod('__construct'); $isOneLine = $constructor !== null @@ -125,41 +130,44 @@ private function addAttributesInScope(array &$stmts, string $namespace, string $ continue; } - $param->attrGroups[] = new Node\AttributeGroup([$this->createAttribute($scope, $attribute)]); + $param->attrGroups[] = new Node\AttributeGroup([$this->createAttribute($attribute)]); $param->setAttribute(AttributesPrinter::INLINE_ATTRIBUTES, $isOneLine); } } - $stmts = $scope->stmts; - return true; } - private function createAttribute(ImportScope $scope, AttributeSpec $attribute): Node\Attribute + private function createAttribute(AttributeSpec $attribute): Node\Attribute { - return new Node\Attribute($this->createName($scope, self::DI_NAMESPACE . $attribute->name), $this->createArgs($scope, $attribute)); + return new Node\Attribute(new Node\Name\FullyQualified(self::DI_NAMESPACE . $attribute->name), $this->createArgs($attribute)); } /** * @return list */ - private function createArgs(ImportScope $scope, AttributeSpec $attribute): array + private function createArgs(AttributeSpec $attribute): array { $args = []; foreach ($attribute->arguments as $name => $value) { - $args[] = new Node\Arg($this->createExpr($scope, $value), name: new Node\Identifier($name)); + $args[] = new Node\Arg($this->createExpr($value), name: new Node\Identifier($name)); } return $args; } - private function createExpr(ImportScope $scope, mixed $value): Node\Expr + private function createExpr(mixed $value): Node\Expr { if ($value instanceof AttributeSpec) { - return new Node\Expr\New_($this->createName($scope, self::DI_NAMESPACE . $value->name), $this->createArgs($scope, $value)); + return new Node\Expr\New_(new Node\Name\FullyQualified(self::DI_NAMESPACE . $value->name), $this->createArgs($value)); } if ($value instanceof ClassReference) { - return new Node\Expr\ClassConstFetch($this->createName($scope, $value->className), 'class'); + $className = ltrim($value->className, '\\'); + if ($className === '') { + throw new ShouldNotHappenException('Empty class name.'); + } + + return new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified($className), 'class'); } if (is_string($value)) { return new Node\Scalar\String_($value); @@ -175,8 +183,8 @@ private function createExpr(ImportScope $scope, mixed $value): Node\Expr $isList = array_is_list($value); foreach ($value as $key => $item) { $items[] = new Node\ArrayItem( - $this->createExpr($scope, $item), - $isList ? null : $this->createExpr($scope, $key), + $this->createExpr($item), + $isList ? null : $this->createExpr($key), ); } @@ -186,36 +194,4 @@ private function createExpr(ImportScope $scope, mixed $value): Node\Expr throw new ShouldNotHappenException(); } - /** - * Uses the short name and adds a use statement unless the short name already means another class in the file. - */ - private function createName(ImportScope $scope, string $className): Node\Name - { - $className = ltrim($className, '\\'); - $position = strrpos($className, '\\'); - $shortName = $position === false ? $className : substr($className, $position + 1); - if ($className === '' || $shortName === '') { - throw new ShouldNotHappenException(sprintf('Invalid class name %s.', $className)); - } - $lowerShortName = strtolower($shortName); - - if (array_key_exists($lowerShortName, $scope->uses)) { - return strcasecmp($scope->uses[$lowerShortName], $className) === 0 - ? new Node\Name($shortName) - : new Node\Name\FullyQualified($className); - } - - $localName = $scope->namespace === '' ? $shortName : $scope->namespace . '\\' . $shortName; - if (strcasecmp($localName, $className) === 0) { - return new Node\Name($shortName); - } - if (array_key_exists($lowerShortName, $scope->unqualifiedNames)) { - return new Node\Name\FullyQualified($className); - } - - $scope->addUse($className, $lowerShortName); - - return new Node\Name($shortName); - } - } diff --git a/src/Fixable/NewClassNamesVisitor.php b/src/Fixable/NewClassNamesVisitor.php new file mode 100644 index 00000000000..e114f107c59 --- /dev/null +++ b/src/Fixable/NewClassNamesVisitor.php @@ -0,0 +1,476 @@ + */ + private array $classNames = []; + + /** @var array lowercase first parts of names in the current namespace that already mean something */ + private array $takenNames = []; + + /** @var list */ + private array $addedUses = []; + + private Lexer $phpDocLexer; + + private PhpDocParser $phpDocParser; + + public function __construct( + private NameContext $nameContext, + private bool $importNames, + private bool $preferSelf, + ) + { + // the format-preserving printer needs token indexes + $config = new ParserConfig(['lines' => true, 'indexes' => true]); + $constExprParser = new ConstExprParser($config); + $this->phpDocLexer = new Lexer($config); + $this->phpDocParser = new PhpDocParser($config, new TypeParser($config, $constExprParser), $constExprParser); + } + + #[Override] + public function beforeTraverse(array $nodes): ?array + { + $this->classNames = []; + $this->addedUses = []; + $this->takenNames = $this->findTakenNames($nodes); + + return null; + } + + #[Override] + public function enterNode(Node $node): ?Node + { + if ($node instanceof Stmt\Namespace_) { + $this->addedUses = []; + $this->takenNames = $this->findTakenNames($node->stmts); + } + + if ($node instanceof Stmt\ClassLike) { + $this->classNames[] = $node instanceof Stmt\Trait_ || $node->namespacedName === null ? null : $node->namespacedName->toString(); + } + + $this->markNames($node); + $this->writeNewPhpDocNames($node); + + if ( + !$node instanceof FullyQualified + || $node->getAttribute('origNode') !== null + || $node->getAttribute(self::SKIP_ATTRIBUTE) === true + ) { + return null; + } + + return $this->resolve($node->toString(), $node->getAttribute(self::SELF_NOT_ALLOWED_ATTRIBUTE) !== true); + } + + #[Override] + public function leaveNode(Node $node): ?Node + { + if ($node instanceof Stmt\ClassLike) { + array_pop($this->classNames); + } + + if ($node instanceof Stmt\Namespace_) { + $node->stmts = $this->addUses($node->stmts); + $this->addedUses = []; + } + + return null; + } + + #[Override] + public function afterTraverse(array $nodes): ?array + { + if ($this->addedUses === []) { + return null; + } + + /** @var list $nodes */ + return $this->addUses($nodes); + } + + /** + * @param non-empty-string $className + * @return Name|null null to keep it fully qualified + */ + private function resolve(string $className, bool $selfAllowed): ?Name + { + $currentClassName = $this->classNames === [] ? null : $this->classNames[count($this->classNames) - 1]; + if ($this->preferSelf && $selfAllowed && $currentClassName !== null && strcasecmp($currentClassName, $className) === 0) { + return new Name('self'); + } + + $shortName = $this->nameContext->getShortName($className, Stmt\Use_::TYPE_NORMAL); + if ( + !$shortName->isFullyQualified() + // in the global namespace, Foo\Bar is not shorter than \Foo\Bar + && ($shortName->isUnqualified() || strcasecmp($shortName->toString(), $className) !== 0) + ) { + return $shortName; + } + + if (!$this->importNames) { + return null; + } + + $name = new Name($className); + $lastName = $name->getLast(); + if (!$name->isQualified() || isset($this->takenNames[strtolower($lastName)])) { + return null; + } + + $this->takenNames[strtolower($lastName)] = true; + $this->nameContext->addAlias($name, $lastName, Stmt\Use_::TYPE_NORMAL); + $this->addedUses[] = $className; + + return new Name([$lastName]); + } + + /** + * Names that the file already uses without a leading backslash, and classes declared in it. + * Importing a class with one of these short names would change what the existing code means. + * + * @param Node[] $nodes + * @return array + */ + private function findTakenNames(array $nodes): array + { + $takenNames = []; + $nodeFinder = new NodeFinder(); + // names that are not class names used in the code, or that only declare names + $skippedNames = []; + foreach ($nodeFinder->find($nodes, static fn (Node $node): bool => $node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\ConstFetch || $node instanceof Node\UseItem || $node instanceof Stmt\GroupUse || $node instanceof Stmt\Namespace_) as $node) { + if ($node instanceof Stmt\GroupUse) { + $skippedNames[] = $node->prefix; + } elseif ($node instanceof Node\UseItem || $node instanceof Stmt\Namespace_ || $node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\ConstFetch) { + $skippedNames[] = $node->name; + } + } + + $phpDocNameCollector = new class extends AbstractNodeVisitor { + + /** @var array */ + public array $names = []; + + #[Override] + public function enterNode(PhpDocNodeInterface $node): ?PhpDocNodeInterface + { + if ($node instanceof IdentifierTypeNode && !str_starts_with($node->name, '\\')) { + $this->names[strtolower(explode('\\', $node->name)[0])] = true; + } elseif ($node instanceof PhpDocTagNode && str_contains($node->name, '\\')) { + // Doctrine-style annotations like @ORM\Column + $this->names[strtolower(explode('\\', ltrim($node->name, '@'))[0])] = true; + } + + return null; + } + + }; + $phpDocTraverser = new PhpDocNodeTraverser([$phpDocNameCollector]); + + foreach ($nodeFinder->find($nodes, static fn (Node $node): bool => true) as $node) { + if ($node instanceof Stmt\Use_ || $node instanceof Stmt\GroupUse) { + foreach ($node->uses as $useItem) { + if (($useItem->type === Stmt\Use_::TYPE_UNKNOWN ? $node->type : $useItem->type) !== Stmt\Use_::TYPE_NORMAL) { + continue; + } + + $takenNames[strtolower($useItem->getAlias()->toString())] = true; + } + continue; + } + + if ($node instanceof Stmt\ClassLike && $node->name !== null) { + $takenNames[strtolower($node->name->toString())] = true; + } + + if ($node instanceof Name && !$node->isFullyQualified() && !$node->isSpecialClassName()) { + $isSkipped = false; + foreach ($skippedNames as $skippedName) { + if ($skippedName === $node) { + $isSkipped = true; + break; + } + } + if (!$isSkipped) { + $takenNames[strtolower($node->getFirst())] = true; + } + } + + $doc = $node->getDocComment(); + if ($doc === null) { + continue; + } + + $phpDocTraverser->traverse([$this->parsePhpDoc($doc)]); + } + + return $takenNames + $phpDocNameCollector->names; + } + + /** + * Writes the fully qualified class names in the PHPDoc tags that the fix added, + * like the added class names in PHP code. + */ + private function writeNewPhpDocNames(Node $node): void + { + $doc = $node->getDocComment(); + if ($doc === null) { + return; + } + + $originalNode = $node->getAttribute('origNode'); + $originalDoc = $originalNode instanceof Node ? $originalNode->getDocComment() : null; + if ($originalDoc !== null && $originalDoc->getText() === $doc->getText()) { + return; + } + + // tags that were already there are kept as written + $originalTags = []; + if ($originalDoc !== null) { + foreach ($this->parsePhpDoc($originalDoc)->getTags() as $tag) { + $originalTags[] = (string) $tag; + } + } + + $tokens = new TokenIterator($this->phpDocLexer->tokenize($doc->getText())); + $phpDocNode = $this->phpDocParser->parse($tokens); + + /** @var PhpDocNode $newPhpDocNode */ + [$newPhpDocNode] = (new PhpDocNodeTraverser([new PhpDocCloningVisitor()]))->traverse([$phpDocNode]); + + $namesTraverser = new PhpDocNodeTraverser([new class (fn (string $className, bool $selfAllowed): ?Name => $this->resolve($className, $selfAllowed)) extends AbstractNodeVisitor { + + /** @var callable(non-empty-string, bool): ?Name */ + private $resolve; + + /** @param callable(non-empty-string, bool): ?Name $resolve */ + public function __construct(callable $resolve) + { + $this->resolve = $resolve; + } + + #[Override] + public function enterNode(PhpDocNodeInterface $node): ?PhpDocNodeInterface + { + // keywords and template types are never written with a leading backslash + if (!$node instanceof IdentifierTypeNode || !str_starts_with($node->name, '\\')) { + return null; + } + + $className = substr($node->name, 1); + if ($className === '') { + return null; + } + + $name = ($this->resolve)($className, true); + if ($name === null) { + return null; + } + + return $name->isSpecialClassName() ? new IdentifierTypeNode($name->toString()) : new RelativeNameNode($name->toString()); + } + + }]); + + $changed = false; + foreach ($newPhpDocNode->children as $i => $child) { + if (!$child instanceof PhpDocTagNode) { + continue; + } + + $key = array_search((string) $child, $originalTags, true); + if ($key !== false) { + unset($originalTags[$key]); + continue; + } + + [$newChild] = $namesTraverser->traverse([$child]); + if (!$newChild instanceof PhpDocTagNode) { + continue; + } + + $newPhpDocNode->children[$i] = $newChild; + $changed = true; + } + + if (!$changed) { + return; + } + + $node->setDocComment(new Doc((new Printer())->printFormatPreserving($newPhpDocNode, $phpDocNode, $tokens))); + } + + private function parsePhpDoc(Doc $doc): PhpDocNode + { + return $this->phpDocParser->parse(new TokenIterator($this->phpDocLexer->tokenize($doc->getText()))); + } + + /** + * Function and constant names are not class names, + * and `self` is not valid PHP everywhere a class name is. + */ + private function markNames(Node $node): void + { + if ($node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\ConstFetch) { + $node->name->setAttribute(self::SKIP_ATTRIBUTE, true); + return; + } + + $names = []; + if ($node instanceof Node\Attribute) { + $names = [$node->name]; + } elseif ($node instanceof Stmt\Class_) { + $names = [$node->extends, ...$node->implements]; + } elseif ($node instanceof Stmt\Interface_) { + $names = $node->extends; + } elseif ($node instanceof Stmt\Enum_) { + $names = $node->implements; + } elseif ($node instanceof Stmt\TraitUse) { + $names = $node->traits; + } + + foreach ($names as $name) { + if ($name === null) { + continue; + } + + $name->setAttribute(self::SELF_NOT_ALLOWED_ATTRIBUTE, true); + } + } + + /** + * Inserts the added `use` statements in alphabetical order among the existing ones. + * + * @param list $stmts statements of a namespace, or of a file without one + * @return list + */ + private function addUses(array $stmts): array + { + foreach ($this->addedUses as $className) { + $stmts = $this->addUse($stmts, $className); + } + + return $stmts; + } + + /** + * @param list $stmts + * @param non-empty-string $className + * @return list + */ + private function addUse(array $stmts, string $className): array + { + $use = new Stmt\Use_([new Node\UseItem(new Name($className))]); + + $position = null; + $lastClassUse = null; + $firstOtherUse = null; + $lastDeclare = null; + foreach ($stmts as $i => $stmt) { + if ($stmt instanceof Stmt\Declare_) { + $lastDeclare = $i; + continue; + } + if (!$stmt instanceof Stmt\Use_) { + continue; + } + if ($stmt->type !== Stmt\Use_::TYPE_NORMAL) { + $firstOtherUse ??= $i; + continue; + } + + $lastClassUse = $i; + if ($position !== null || count($stmt->uses) !== 1 || strcasecmp($stmt->uses[0]->name->toString(), $className) <= 0) { + continue; + } + + $position = $i; + } + + if ($position === null) { + if ($lastClassUse !== null) { + $position = $lastClassUse + 1; + } elseif ($firstOtherUse !== null) { + $position = $firstOtherUse; + } else { + // the first use statement, separated from the code below it by an empty line + array_splice($stmts, $lastDeclare !== null ? $lastDeclare + 1 : 0, 0, [$use, new Stmt\Nop()]); + return $stmts; + } + } + + array_splice($stmts, $position, 0, [$use]); + + return $stmts; + } + +} diff --git a/tests/PHPStan/Build/data/skip-tests-requires-php.php.fixed b/tests/PHPStan/Build/data/skip-tests-requires-php.php.fixed index 11df5d6549a..315283d66ae 100644 --- a/tests/PHPStan/Build/data/skip-tests-requires-php.php.fixed +++ b/tests/PHPStan/Build/data/skip-tests-requires-php.php.fixed @@ -9,7 +9,7 @@ use const PHP_VERSION_ID; class FooTest extends TestCase { - #[\PHPUnit\Framework\Attributes\RequiresPhp('>= 8.0.0')] + #[RequiresPhp('>= 8.0.0')] public function testVersionCheck(): void { } diff --git a/tests/PHPStan/Fixable/AddNamesFullyQualifiedRuleTest.php b/tests/PHPStan/Fixable/AddNamesFullyQualifiedRuleTest.php new file mode 100644 index 00000000000..d40ed90d90f --- /dev/null +++ b/tests/PHPStan/Fixable/AddNamesFullyQualifiedRuleTest.php @@ -0,0 +1,39 @@ + + */ +class AddNamesFullyQualifiedRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new AddNamesRule(self::getContainer()->getByType(PhpDocEditor::class)); + } + + public function testFix(): void + { + $this->fix(__DIR__ . '/data/add-names.php', __DIR__ . '/data/add-names-fully-qualified.php.fixed'); + } + + public function testFixGlobalNamespace(): void + { + $this->fix(__DIR__ . '/data/add-names-global.php', __DIR__ . '/data/add-names-global-fully-qualified.php.fixed'); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/fully-qualified-names.neon', + ]; + } + +} diff --git a/tests/PHPStan/Fixable/AddNamesRule.php b/tests/PHPStan/Fixable/AddNamesRule.php new file mode 100644 index 00000000000..2f73b4571dc --- /dev/null +++ b/tests/PHPStan/Fixable/AddNamesRule.php @@ -0,0 +1,80 @@ + + */ +final class AddNamesRule implements Rule +{ + + public function __construct(private PhpDocEditor $phpDocEditor) + { + } + + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $docComment = $node->getDocComment(); + if ($docComment === null || preg_match_all('/@add-name\s+\\\\?(\S+)/', $docComment->getText(), $matches) === 0) { + return []; + } + + $names = $matches[1]; + + return [ + RuleErrorBuilder::message(sprintf('Adding %d names.', count($names))) + ->identifier('tests.addNames') + ->fixNode($node, function (Node\Stmt\ClassMethod $method) use ($names) { + $i = 0; + $this->phpDocEditor->edit($method, static function (\PHPStan\PhpDocParser\Ast\Node $phpDocNode) use (&$i) { + if (!$phpDocNode instanceof PhpDocTagNode || $phpDocNode->name !== '@add-name') { + return null; + } + + $i++; + + return new PhpDocTagNode('@param', new ParamTagValueNode( + new FullyQualifiedNameNode((string) $phpDocNode->value), + false, + '$p' . $i, + '', + false, + )); + }); + + foreach ($names as $j => $name) { + $method->params[] = new Node\Param( + new Node\Expr\Variable('p' . ($j + 1)), + type: new FullyQualified($name), + ); + } + + return $method; + }) + ->build(), + ]; + } + +} diff --git a/tests/PHPStan/Fixable/AddNamesRuleTest.php b/tests/PHPStan/Fixable/AddNamesRuleTest.php new file mode 100644 index 00000000000..2b40879f90c --- /dev/null +++ b/tests/PHPStan/Fixable/AddNamesRuleTest.php @@ -0,0 +1,40 @@ + + */ +class AddNamesRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new AddNamesRule(self::getContainer()->getByType(PhpDocEditor::class)); + } + + public function testFix(): void + { + $this->fix(__DIR__ . '/data/add-names.php', __DIR__ . '/data/add-names.php.fixed'); + } + + public function testFixGlobalNamespace(): void + { + $this->fix(__DIR__ . '/data/add-names-global.php', __DIR__ . '/data/add-names-global.php.fixed'); + } + + public function testFixFirstUse(): void + { + $this->fix(__DIR__ . '/data/add-names-first-use.php', __DIR__ . '/data/add-names-first-use.php.fixed'); + } + + public function testFixSortedUses(): void + { + $this->fix(__DIR__ . '/data/add-names-sorted-uses.php', __DIR__ . '/data/add-names-sorted-uses.php.fixed'); + } + +} diff --git a/tests/PHPStan/Fixable/data/add-names-first-use.php b/tests/PHPStan/Fixable/data/add-names-first-use.php new file mode 100644 index 00000000000..19514cb65b3 --- /dev/null +++ b/tests/PHPStan/Fixable/data/add-names-first-use.php @@ -0,0 +1,17 @@ +fix(__DIR__ . '/data/fix-override-attribute.php', __DIR__ . '/data/fix-override-attribute.php.fixed'); } + public function testFixOverrideImported(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->checkMissingOverrideMethodAttribute = true; + $this->fix(__DIR__ . '/data/fix-override-attribute-imported.php', __DIR__ . '/data/fix-override-attribute-imported.php.fixed'); + } + + public function testFixOverrideGlobalNamespace(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->checkMissingOverrideMethodAttribute = true; + $this->fix(__DIR__ . '/data/fix-override-attribute-global.php', __DIR__ . '/data/fix-override-attribute-global.php.fixed'); + } + public function testFixWithTabs(): void { $this->phpVersionId = PHP_VERSION_ID; diff --git a/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php new file mode 100644 index 00000000000..5d456eb24f7 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php @@ -0,0 +1,21 @@ += 8.3 + +class FixOverrideAttributeGlobalFoo +{ + + public function doFoo(): void + { + + } + +} + +class FixOverrideAttributeGlobalBar extends FixOverrideAttributeGlobalFoo +{ + + public function doFoo(): void + { + + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php.fixed b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php.fixed new file mode 100644 index 00000000000..048240dd7fa --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-global.php.fixed @@ -0,0 +1,22 @@ += 8.3 + +class FixOverrideAttributeGlobalFoo +{ + + public function doFoo(): void + { + + } + +} + +class FixOverrideAttributeGlobalBar extends FixOverrideAttributeGlobalFoo +{ + + #[Override] + public function doFoo(): void + { + + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php new file mode 100644 index 00000000000..86971046343 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php @@ -0,0 +1,35 @@ += 8.3 + +namespace FixOverrideAttributeImported; + +use Override; + +class Foo +{ + + public function doFoo(): void + { + + } + + public function doBar(): void + { + + } + +} + +class Bar extends Foo +{ + + public function doFoo(): void + { + + } + + public function doBar(): void + { + + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php.fixed b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php.fixed new file mode 100644 index 00000000000..a2092ccdeb0 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/fix-override-attribute-imported.php.fixed @@ -0,0 +1,37 @@ += 8.3 + +namespace FixOverrideAttributeImported; + +use Override; + +class Foo +{ + + public function doFoo(): void + { + + } + + public function doBar(): void + { + + } + +} + +class Bar extends Foo +{ + + #[Override] + public function doFoo(): void + { + + } + + #[Override] + public function doBar(): void + { + + } + +}