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: add fully_qualified_strict_types fixer import_relative_symbols option #7721

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 29 additions & 1 deletion doc/rules/import/fully_qualified_strict_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,20 @@ is imported or belongs to the current namespace.
Configuration
-------------

``import_relative_symbols``
~~~~~~~~~~~~~~~~~~~~~~~~~~~

Whether non-FQCNs containing backslash should be automatically imported.
Applicable only with ``import_symbols`` enabled.

Allowed types: ``bool``

Default value: ``false``

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

Whether FQCNs found during analysis should be automatically imported.
Whether FQCNs should be automatically imported.

Allowed types: ``bool``

Expand Down Expand Up @@ -179,6 +189,24 @@ With configuration: ``['import_symbols' => true]``.
}
}

Example #5
~~~~~~~~~~

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

.. code-block:: diff

--- Original
+++ New
<?php

use Foo\Bar;
+use Foo\Bar\Baz;

$bar = new Bar();
-$baz = new Bar\Baz();
+$baz = new Baz();

Rule sets
---------

Expand Down
56 changes: 41 additions & 15 deletions src/Fixer/Import/FullyQualifiedStrictTypesFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,16 @@ public function foo(): \Other\FunctionReturnType
',
['import_symbols' => true]
),
new CodeSample(
'<?php

use Foo\Bar;

$bar = new Bar();
$baz = new Bar\Baz();
',
['import_symbols' => true, 'import_relative_symbols' => true]
),
]
);
}
Expand Down Expand Up @@ -211,7 +221,14 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
->getOption(),
(new FixerOptionBuilder(
'import_symbols',
'Whether FQCNs found during analysis should be automatically imported.'
'Whether FQCNs should be automatically imported.'
))
->setAllowedTypes(['bool'])
->setDefault(false)
->getOption(),
(new FixerOptionBuilder(
'import_relative_symbols',
'Whether non-FQCNs containing backslash should be automatically imported. Applicable only with `import_symbols` enabled.'
))
->setAllowedTypes(['bool'])
->setDefault(false)
Expand Down Expand Up @@ -390,7 +407,7 @@ private function shortenSymbol(string $fqcn, array $uses, string $namespaceName)
$tmpRes = substr($fqcn, \strlen($namespaceName) + 1);
if (!isset($this->cacheUseNameByShortNameLower[strtolower(explode('\\', $tmpRes, 2)[0])])) {
$res = $tmpRes;
$iMin = substr_count($namespaceName, '\\') - 1;
$iMin = substr_count($namespaceName, '\\') + 1;
}
}

Expand Down Expand Up @@ -448,7 +465,10 @@ private function setupUsesFromDiscoveredSymbols(array &$uses, string $namespaceN
foreach ($discoveredSymbols as $symbol) {
$shortEndNameLower = strtolower(str_contains($symbol, '\\') ? substr($symbol, strrpos($symbol, '\\') + 1) : $symbol);
if (!isset($discoveredFqcnByShortNameLower[$shortEndNameLower])) {
if ('' !== $namespaceName && !str_starts_with($symbol, '\\') && str_contains($symbol, '\\')) { // @TODO add option to force all classes to be imported
if ('' !== $namespaceName
&& !str_starts_with($symbol, '\\') && false === $this->configuration['import_relative_symbols']
&& str_contains($symbol, '\\')
) {
continue;
}

Expand Down Expand Up @@ -519,20 +539,26 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name
return $matches[0];
}

// @TODO parse the complex type using TypeExpression and fix all names inside (like `int|string` or `list<int|string>`)
if (!Preg::match('/^[a-zA-Z0-9_\\\\]+(\|null)?$/', $matches[4])) {
return $matches[0];
}
/** @TODO parse the complex type using TypeExpression and fix all names inside (like `list<\Foo\Bar|'a|b|c'|string>` or `\Foo\Bar[]`) */
$unsupported = false;

$shortTokens = $this->determineShortType($matches[4], $uses, $namespaceName);
if (null === $shortTokens) {
return $matches[0];
}
return $matches[1].$matches[2].$matches[3].implode('|', array_map(function ($v) use ($uses, $namespaceName, &$unsupported) {
if ($unsupported || !Preg::match('/^'.self::REGEX_CLASS.'$/', $v)) {
$unsupported = true;

return $v;
}

$shortTokens = $this->determineShortType($v, $uses, $namespaceName);
if (null === $shortTokens) {
return $v;
}

return $matches[1].$matches[2].$matches[3].implode('', array_map(
static fn (Token $token) => $token->getContent(),
$shortTokens
));
return implode('', array_map(
static fn (Token $token) => $token->getContent(),
$shortTokens
));
}, explode('|', $matches[4])));
}, $phpDocContent);

if ($phpDocContentNew !== $phpDocContent) {
Expand Down
235 changes: 235 additions & 0 deletions tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,198 @@ class Cl {}
new R\T();
EOD,
];

yield 'shortening - namespace with shorter import' => [
<<<'EOD'
<?php

namespace U\V\W;

use U\V;

new \U();
new V();
new V\W();
new X();
new X\Y();
new X\Y\Z();
EOD,
];

yield 'shortening - namespace with same import' => [
<<<'EOD'
<?php

namespace U\V\W;

use U\V\W;

new \U();
new \U\V();
new W();
new X();
new X\Y();
new X\Y\Z();
EOD,
];

yield 'shortening - namespace with useless import' => [
<<<'EOD'
<?php

namespace U\V\W;

use U\V\W\X;

new \U();
new \U\V();
new \U\V\W();
new X();
new X\Y();
new X\Y\Z();
EOD,
];

yield 'shortening - namespace with longer import' => [
<<<'EOD'
<?php

namespace U\V\W;

use U\V\W\X\Y;

new \U();
new \U\V();
new \U\V\W();
new X();
new Y();
new Y\Z();
new Y\Z\e();
new Y\Z\e\f();
EOD,
];

yield 'import even if party already imported - global' => [
<<<'EOD'
<?php

use Ns\Foo;
use Ns\Foo\M;
use Ns\Foo\M\N;

new M();
new N();
new Exception();
new Exception();
EOD,
<<<'EOD'
<?php

use Ns\Foo;

new Foo\M();
new Foo\M\N();
new \Exception();
new Exception();
EOD,
['import_symbols' => true, 'import_relative_symbols' => true],
];

yield 'import even if party already imported - namespaced' => [
<<<'EOD'
<?php

namespace Ns;

use Ns2\Foo;
use Ns2\Foo\M;
use Ns2\Foo\M\N;

new M();
new N();
new \Exception();
new Exception();
EOD,
<<<'EOD'
<?php

namespace Ns;

use Ns2\Foo;

new Foo\M();
new Foo\M\N();
new \Exception();
new Exception();
EOD,
['import_symbols' => true, 'import_relative_symbols' => true],
];

yield 'import even if partly importable using namespace' => [
<<<'EOD'
<?php

namespace Ns;
use Ns\A\B;
use Ns\A\B\C;

new A();
new B();
new C();
EOD,
<<<'EOD'
<?php

namespace Ns;

new \Ns\A();
new \Ns\A\B();
new \Ns\A\B\C();
EOD,
['import_symbols' => true],
];

yield 'import even if partly already imported using namespace - without import_relative_symbols' => [
<<<'EOD'
<?php

namespace Ns;

new A();
new A\B();
new A\B\C();
EOD,
null,
['import_symbols' => true],
];

yield 'import even if partly already imported using namespace - with import_relative_symbols' => [
<<<'EOD'
<?php

namespace Ns;

use Ns2\Foo;
use Ns\A\B;
use Ns\A\B\C;

new A();
new B();
new C();
EOD,
<<<'EOD'
<?php

namespace Ns;

use Ns2\Foo;

new A();
new A\B();
new A\B\C();
EOD,
['import_symbols' => true, 'import_relative_symbols' => true],
];
}

/**
Expand Down Expand Up @@ -1520,6 +1712,49 @@ class SomeClass {}',
function foo($v) {}',
];

yield 'Test PHPDoc union with imports' => [
'<?php

namespace Ns;
use Other\Foo;
use Other\FooŇ;

/**
* @param \Exception|\Exception2|int|Foo|FooŇ|null $v
*/
function foo($v) {}',
'<?php

namespace Ns;

/**
* @param \Exception|\Exception2|int|\Other\Foo|\Other\FooŇ|null $v
*/
function foo($v) {}',
['import_symbols' => true],
];

yield 'Test PHPDoc string must be kept as is' => [
'<?php

namespace Ns;
use Other\Foo;

/**
* @param Foo|\'\Other\Bar|\Other\Bar2|\Other\Bar3\'|\Other\Foo2 $v
*/
function foo($v) {}',
'<?php

namespace Ns;

/**
* @param \Other\Foo|\'\Other\Bar|\Other\Bar2|\Other\Bar3\'|\Other\Foo2 $v
*/
function foo($v) {}',
['import_symbols' => true],
];

yield 'Test PHPDoc in interface' => [
'<?php

Expand Down
2 changes: 1 addition & 1 deletion tests/Test/AbstractFixerWithAliasedOptionsTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
abstract class AbstractFixerWithAliasedOptionsTestCase extends AbstractFixerTestCase
{
/**
* @var null|\PhpCsFixer\Fixer\ConfigurableFixerInterface
* @var null|ConfigurableFixerInterface
*/
private $fixerWithAliasedConfig;

Expand Down