Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Ability to import FQCNs found during analysis #7597

Merged
merged 10 commits into from
Dec 20, 2023
15 changes: 0 additions & 15 deletions dev-tools/phpstan/baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -221,21 +221,6 @@
'count' => 1,
'path' => __DIR__ . '/../../src/Fixer/Import/GlobalNamespaceImportFixer.php',
];
$ignoreErrors[] = [
'message' => '#^Method PhpCsFixer\\\\Fixer\\\\Import\\\\GlobalNamespaceImportFixer\\:\\:filterUseDeclarations\\(\\) return type has no value type specified in iterable type array\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Fixer/Import/GlobalNamespaceImportFixer.php',
];
$ignoreErrors[] = [
'message' => '#^Method PhpCsFixer\\\\Fixer\\\\Import\\\\GlobalNamespaceImportFixer\\:\\:insertImports\\(\\) has parameter \\$imports with no value type specified in iterable type array\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Fixer/Import/GlobalNamespaceImportFixer.php',
];
$ignoreErrors[] = [
'message' => '#^Method PhpCsFixer\\\\Fixer\\\\Import\\\\GlobalNamespaceImportFixer\\:\\:prepareImports\\(\\) has parameter \\$global with no value type specified in iterable type array\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Fixer/Import/GlobalNamespaceImportFixer.php',
];
$ignoreErrors[] = [
'message' => '#^Method PhpCsFixer\\\\Fixer\\\\Import\\\\OrderedImportsFixer\\:\\:getNewOrder\\(\\) return type has no value type specified in iterable type array\\.$#',
'count' => 1,
Expand Down
49 changes: 49 additions & 0 deletions doc/rules/import/fully_qualified_strict_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ interfaces.
Configuration
-------------

``import_symbols``
~~~~~~~~~~~~~~~~~~

Whether FQCNs found during analysis should be automatically imported.

Allowed types: ``bool``

Default value: ``false``

``leading_backslash_in_global_namespace``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -122,6 +131,46 @@ With configuration: ``['leading_backslash_in_global_namespace' => true]``.
}
}

Example #4
~~~~~~~~~~

With configuration: ``['import_symbols' => true]``.

.. code-block:: diff

--- Original
+++ New
<?php

namespace Foo\Test;
+use Other\BaseClass;
+use Other\CaughtThrowable;
+use Other\FunctionArgument;
+use Other\FunctionReturnType;
+use Other\Interface1;
+use Other\Interface2;
+use Other\PropertyPhpDoc;
+use Other\StaticFunctionCall;

-class Foo extends \Other\BaseClass implements \Other\Interface1, \Other\Interface2
+class Foo extends BaseClass implements Interface1, Interface2
{
- /** @var \Other\PropertyPhpDoc */
+ /** @var PropertyPhpDoc */
private $array;
- public function __construct(\Other\FunctionArgument $arg) {}
- public function foo(): \Other\FunctionReturnType
+ public function __construct(FunctionArgument $arg) {}
+ public function foo(): FunctionReturnType
{
try {
- \Other\StaticFunctionCall::bar();
- } catch (\Other\CaughtThrowable $e) {}
+ StaticFunctionCall::bar();
+ } catch (CaughtThrowable $e) {}
}
}

Rule sets
---------

Expand Down
127 changes: 123 additions & 4 deletions src/Fixer/Import/FullyQualifiedStrictTypesFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\Fixer\ConfigurableFixerInterface;
use PhpCsFixer\Fixer\WhitespacesAwareFixerInterface;
use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
use PhpCsFixer\FixerConfiguration\FixerConfigurationResolverInterface;
use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
Expand All @@ -27,6 +28,7 @@
use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer;
use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Processor\ImportProcessor;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;

Expand All @@ -36,8 +38,17 @@
* @author Greg Korba <greg@codito.dev>
* @author SpacePossum <possumfromspace@gmail.com>
*/
final class FullyQualifiedStrictTypesFixer extends AbstractFixer implements ConfigurableFixerInterface
final class FullyQualifiedStrictTypesFixer extends AbstractFixer implements ConfigurableFixerInterface, WhitespacesAwareFixerInterface
{
/**
* @var array{
* const?: array<string, class-string>,
* class?: array<string, class-string>,
* function?: array<string, class-string>
* }
*/
private array $symbolsForImport = [];

public function getDefinition(): FixerDefinitionInterface
{
return new FixerDefinition(
Expand Down Expand Up @@ -113,14 +124,34 @@ class SomeClass implements \Foo\Bar\Baz
',
['leading_backslash_in_global_namespace' => true]
),
new CodeSample(
'<?php

namespace Foo\Test;

class Foo extends \Other\BaseClass implements \Other\Interface1, \Other\Interface2
{
/** @var \Other\PropertyPhpDoc */
private $array;
public function __construct(\Other\FunctionArgument $arg) {}
public function foo(): \Other\FunctionReturnType
{
try {
\Other\StaticFunctionCall::bar();
} catch (\Other\CaughtThrowable $e) {}
}
}
',
['import_symbols' => true]
),
]
);
}

/**
* {@inheritdoc}
*
* Must run before NoSuperfluousPhpdocTagsFixer.
* Must run before NoSuperfluousPhpdocTagsFixer, OrderedImportsFixer, StatementIndentationFixer.
* Must run after PhpdocToReturnTypeFixer.
*/
public function getPriority(): int
Expand Down Expand Up @@ -150,6 +181,13 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
->setAllowedTypes(['bool'])
->setDefault(false)
->getOption(),
(new FixerOptionBuilder(
'import_symbols',
'Whether FQCNs found during analysis should be automatically imported.'
))
->setAllowedTypes(['bool'])
->setDefault(false)
->getOption(),
]);
}

Expand All @@ -161,9 +199,11 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
foreach ($tokens->getNamespaceDeclarations() as $namespace) {
$namespaceName = strtolower($namespace->getFullName());
$uses = [];
$lastUse = null;

foreach ($namespaceUsesAnalyzer->getDeclarationsInNamespace($tokens, $namespace) as $use) {
$uses[strtolower(ltrim($use->getFullName(), '\\'))] = $use->getShortName();
$uses[$this->normaliseSymbolName($use->getFullName())] = $use->getShortName();
$lastUse = $use;
}

for ($index = $namespace->getScopeStartIndex(); $index < $namespace->getScopeEndIndex(); ++$index) {
Expand All @@ -181,6 +221,15 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
$this->fixPhpDoc($tokens, $index, $uses, $namespaceName);
}
}

if (true === $this->configuration['import_symbols'] && [] !== $this->symbolsForImport) {
$atIndex = (null !== $lastUse) ? $lastUse->getEndIndex() + 1 : $namespace->getEndIndex() + 1;

// Insert all registered FQCNs
$this->createImportProcessor()->insertImports($tokens, $this->symbolsForImport, $atIndex);

$this->symbolsForImport = [];
}
}
}

Expand Down Expand Up @@ -221,6 +270,10 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name
continue;
}

if (true === $this->configuration['import_symbols'] && isset($matches[2][0])) {
$this->registerSymbolForImport('class', $matches[2][0], $uses, $namespaceName);
}

$shortTokens = $this->determineShortType($typeName, $uses, $namespaceName);

if (null !== $shortTokens) {
Expand All @@ -245,14 +298,16 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name
*/
private function fixExtendsImplements(Tokens $tokens, int $index, array $uses, string $namespaceName): void
{
// We handle `extends` and `implements` with similar logic, but we need to exit the loop under different conditions.
$isExtends = $tokens[$index]->equals([T_EXTENDS]);
$index = $tokens->getNextMeaningfulToken($index);
$extend = ['content' => '', 'tokens' => []];

while (true) {
if ($tokens[$index]->equalsAny([',', '{', [T_IMPLEMENTS]])) {
$this->shortenClassIfPossible($tokens, $extend, $uses, $namespaceName);

if ($tokens[$index]->equals('{')) {
if ($tokens[$index]->equalsAny($isExtends ? [[T_IMPLEMENTS], '{'] : ['{'])) {
Comment on lines +301 to +310
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: basically it is a fix for redundant analysis that is currently executed when extends and implements are both used in a class. This method is invoked 2 times in applyFix() because both T_EXTENDS and T_IMPLEMENTS trigger it, but currently both are exiting on {, which means implements part is processed 2 times.

In the context of this particular PR, it resulted in symbols being imported twice, because e.g. implements \Other\Foo was shortened to implements Foo in the first run (for extends), and when processing implements it was registering Foo for importing. I've fixed the issue of importing non-FQNs (symbol must start with \ to be registered for importing), but this scenario allowed me to find the redundant loop.

break;
}

Expand Down Expand Up @@ -328,6 +383,10 @@ private function shortenClassIfPossible(Tokens $tokens, array $class, array $use
{
$longTypeContent = $class['content'];

if (true === $this->configuration['import_symbols']) {
$this->registerSymbolForImport('class', $longTypeContent, $uses, $namespaceName);
}

if (str_starts_with($longTypeContent, '\\')) {
$typeName = substr($longTypeContent, 1);
$typeNameLower = strtolower($typeName);
Expand Down Expand Up @@ -382,6 +441,10 @@ private function replaceByShortType(Tokens $tokens, TypeAnalysis $type, array $u
return;
}

if (true === $this->configuration['import_symbols']) {
$this->registerSymbolForImport('class', $typeName, $uses, $namespaceName);
}

$shortType = $this->determineShortType($typeName, $uses, $namespaceName);

if (null !== $shortType) {
Expand Down Expand Up @@ -552,4 +615,60 @@ private function namespacedStringToTokens(string $input, bool $withLeadingBacksl

return $tokens;
}

/**
* We need to create import processor dynamically (not in costructor), because actual whitespace configuration
* is set later, not when fixer's instance is created.
*/
private function createImportProcessor(): ImportProcessor
{
return new ImportProcessor($this->whitespacesConfig);
}

/**
* @param "class"|"const"|"function" $kind
* @param class-string $symbol
* @param array<string, string> $uses
*/
private function registerSymbolForImport(string $kind, string $symbol, array &$uses, string $namespaceName): void
{
$normalisedName = $this->normaliseSymbolName($symbol);

// Do NOT register symbol for importing if:
if (
// we already have the symbol in existing imports
isset($uses[$normalisedName])
// or if the symbol is not a FQCN
|| !str_starts_with($symbol, '\\')
// or if it's a global symbol
|| strpos($symbol, '\\') === strrpos($symbol, '\\')
) {
return;
}

$shortSymbol = substr($symbol, strrpos($symbol, '\\') + 1);
$importedShortNames = array_map(
static fn (string $name): string => strtolower($name),
array_values($uses)
);

// If symbol
if (\in_array(strtolower($shortSymbol), $importedShortNames, true)) {
return;
}

$this->symbolsForImport[$kind][$normalisedName] = ltrim($symbol, '\\');
ksort($this->symbolsForImport[$kind], SORT_NATURAL);

// We must fake that the symbol is imported, so that it can be shortened.
$uses[$normalisedName] = $shortSymbol;
}

/**
* @param class-string $name
*/
private function normaliseSymbolName(string $name): string
{
return strtolower(ltrim($name, '\\'));
}
}
Loading