From 97ef9ff17fb2a92141621baccae2f52d9659d0e6 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 16 Jul 2022 14:26:07 +0200 Subject: [PATCH] Improve namespace names validation (#2670) * inline mixed type * improve namespace function names * add validation for namespace, fix namespaced function regex --- .../Fixture/fixture4.php.inc | 23 ----------- .../Fixture/skip_underscore_namespace.php.inc | 11 +++++ .../config/configured_rule.php | 2 +- .../NodeAnalyzer/ChangedArgumentsDetector.php | 5 +-- rules/Naming/NamespaceMatcher.php | 6 ++- .../ConstFetch/RenameConstantRector.php | 6 +++ .../ValueObject/RenameClassAndConstFetch.php | 3 ++ .../ValueObject/RenameClassConstFetch.php | 2 + .../Renaming/ValueObject/RenamedNamespace.php | 5 +++ .../RemoveSkippedRectorsCompilerPass.php | 5 +-- src/PhpParser/Node/NodeFactory.php | 13 ++---- src/PhpParser/Node/Value/ValueResolver.php | 20 +--------- src/Validation/RectorAssert.php | 32 ++++++++++++++- src/functions/node_helper.php | 5 +-- tests/Validation/RectorAssertTest.php | 40 ++++++++++++++++++- 15 files changed, 109 insertions(+), 69 deletions(-) delete mode 100644 rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc create mode 100644 rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc diff --git a/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc b/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc deleted file mode 100644 index fb469a941bf..00000000000 --- a/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc +++ /dev/null @@ -1,23 +0,0 @@ - ------ - diff --git a/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc b/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc new file mode 100644 index 00000000000..729a3277a84 --- /dev/null +++ b/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc @@ -0,0 +1,11 @@ + 'NewNamespace', 'OldNamespaceWith\OldSplitNamespace' => 'NewNamespaceWith\NewSplitNamespace', 'Old\Long\AnyNamespace' => 'Short\AnyNamespace', - 'PHPUnit_Framework_' => 'PHPUnit\Framework\\', + 'PHPUnit_Framework_' => 'PHPUnit\Framework', 'Foo\Bar' => 'Foo\Tmp', 'App\Repositories' => 'App\Repositories\Example', ]); diff --git a/rules/Arguments/NodeAnalyzer/ChangedArgumentsDetector.php b/rules/Arguments/NodeAnalyzer/ChangedArgumentsDetector.php index 07dfae2c233..2dfea157185 100644 --- a/rules/Arguments/NodeAnalyzer/ChangedArgumentsDetector.php +++ b/rules/Arguments/NodeAnalyzer/ChangedArgumentsDetector.php @@ -19,10 +19,7 @@ public function __construct( ) { } - /** - * @param mixed $value - */ - public function isDefaultValueChanged(Param $param, $value): bool + public function isDefaultValueChanged(Param $param, mixed $value): bool { if ($param->default === null) { return false; diff --git a/rules/Naming/NamespaceMatcher.php b/rules/Naming/NamespaceMatcher.php index cd9562e0a45..a24b7abb95e 100644 --- a/rules/Naming/NamespaceMatcher.php +++ b/rules/Naming/NamespaceMatcher.php @@ -17,7 +17,11 @@ public function matchRenamedNamespace(string $name, array $oldToNewNamespace): ? /** @var string $oldNamespace */ foreach ($oldToNewNamespace as $oldNamespace => $newNamespace) { - if (str_starts_with($name, $oldNamespace)) { + if ($name === $oldNamespace) { + return new RenamedNamespace($name, $oldNamespace, $newNamespace); + } + + if (str_starts_with($name, $oldNamespace . '\\')) { return new RenamedNamespace($name, $oldNamespace, $newNamespace); } } diff --git a/rules/Renaming/Rector/ConstFetch/RenameConstantRector.php b/rules/Renaming/Rector/ConstFetch/RenameConstantRector.php index f484ab4bb97..b216925085e 100644 --- a/rules/Renaming/Rector/ConstFetch/RenameConstantRector.php +++ b/rules/Renaming/Rector/ConstFetch/RenameConstantRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Name; use Rector\Core\Contract\Rector\ConfigurableRectorInterface; use Rector\Core\Rector\AbstractRector; +use Rector\Core\Validation\RectorAssert; use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; use Webmozart\Assert\Assert; @@ -88,6 +89,11 @@ public function configure(array $configuration): void Assert::allString(array_keys($configuration)); Assert::allString($configuration); + foreach ($configuration as $oldConstant => $newConstant) { + RectorAssert::constantName($oldConstant); + RectorAssert::constantName($newConstant); + } + /** @var array $configuration */ $this->oldToNewConstants = $configuration; } diff --git a/rules/Renaming/ValueObject/RenameClassAndConstFetch.php b/rules/Renaming/ValueObject/RenameClassAndConstFetch.php index 66955012024..a4753ea12c1 100644 --- a/rules/Renaming/ValueObject/RenameClassAndConstFetch.php +++ b/rules/Renaming/ValueObject/RenameClassAndConstFetch.php @@ -17,7 +17,10 @@ public function __construct( private readonly string $newConstant ) { RectorAssert::className($oldClass); + RectorAssert::constantName($oldConstant); + RectorAssert::className($newClass); + RectorAssert::constantName($newConstant); } public function getOldObjectType(): ObjectType diff --git a/rules/Renaming/ValueObject/RenameClassConstFetch.php b/rules/Renaming/ValueObject/RenameClassConstFetch.php index 65abd35dc84..b6ce43f349d 100644 --- a/rules/Renaming/ValueObject/RenameClassConstFetch.php +++ b/rules/Renaming/ValueObject/RenameClassConstFetch.php @@ -16,6 +16,8 @@ public function __construct( private readonly string $newConstant ) { RectorAssert::className($oldClass); + RectorAssert::constantName($oldConstant); + RectorAssert::constantName($newConstant); } public function getOldObjectType(): ObjectType diff --git a/rules/Renaming/ValueObject/RenamedNamespace.php b/rules/Renaming/ValueObject/RenamedNamespace.php index 5f6c4249498..01f59fa4bde 100644 --- a/rules/Renaming/ValueObject/RenamedNamespace.php +++ b/rules/Renaming/ValueObject/RenamedNamespace.php @@ -4,6 +4,8 @@ namespace Rector\Renaming\ValueObject; +use Rector\Core\Validation\RectorAssert; + final class RenamedNamespace { public function __construct( @@ -11,6 +13,9 @@ public function __construct( private readonly string $oldNamespace, private readonly string $newNamespace ) { + RectorAssert::namespaceName($currentName); + RectorAssert::namespaceName($oldNamespace); + RectorAssert::namespaceName($newNamespace); } public function getNameInNewNamespace(): string diff --git a/src/DependencyInjection/CompilerPass/RemoveSkippedRectorsCompilerPass.php b/src/DependencyInjection/CompilerPass/RemoveSkippedRectorsCompilerPass.php index efae72c703c..a8bb5ec47b0 100644 --- a/src/DependencyInjection/CompilerPass/RemoveSkippedRectorsCompilerPass.php +++ b/src/DependencyInjection/CompilerPass/RemoveSkippedRectorsCompilerPass.php @@ -42,10 +42,7 @@ private function resolveSkippedRectorClasses(ContainerBuilder $containerBuilder) return array_filter($skipParameters, fn ($element): bool => $this->isRectorClass($element)); } - /** - * @param mixed $element - */ - private function isRectorClass($element): bool + private function isRectorClass(mixed $element): bool { if (! is_string($element)) { return false; diff --git a/src/PhpParser/Node/NodeFactory.php b/src/PhpParser/Node/NodeFactory.php index bd1a8aeb169..101de59f98c 100644 --- a/src/PhpParser/Node/NodeFactory.php +++ b/src/PhpParser/Node/NodeFactory.php @@ -163,10 +163,7 @@ public function createPropertyAssignmentWithExpr(string $propertyName, Expr $exp return new Assign($propertyFetch, $expr); } - /** - * @param mixed $argument - */ - public function createArg($argument): Arg + public function createArg(mixed $argument): Arg { return new Arg(BuilderHelpers::normalizeValue($argument)); } @@ -511,10 +508,7 @@ public function createClassConstant(string $name, Expr $expr, int $modifier): Cl return $classConst; } - /** - * @param mixed $item - */ - private function createArrayItem($item, string | int | null $key = null): ArrayItem + private function createArrayItem(mixed $item, string | int | null $key = null): ArrayItem { $arrayItem = null; @@ -560,10 +554,9 @@ private function createArrayItem($item, string | int | null $key = null): ArrayI } /** - * @param mixed $value * @return mixed|Error|Variable */ - private function normalizeArgValue($value) + private function normalizeArgValue(mixed $value) { if ($value instanceof Param) { return $value->var; diff --git a/src/PhpParser/Node/Value/ValueResolver.php b/src/PhpParser/Node/Value/ValueResolver.php index 6d96b5964c2..7dc8d9de916 100644 --- a/src/PhpParser/Node/Value/ValueResolver.php +++ b/src/PhpParser/Node/Value/ValueResolver.php @@ -43,28 +43,12 @@ public function __construct( ) { } - /** - * @param mixed $value - */ - public function isValue(Expr $expr, $value): bool + public function isValue(Expr $expr, mixed $value): bool { return $this->getValue($expr) === $value; } - public function getStringValue(Expr $expr): string - { - $resolvedValue = $this->getValue($expr); - if (! is_string($resolvedValue)) { - throw new ShouldNotHappenException(); - } - - return $resolvedValue; - } - - /** - * @return mixed|null - */ - public function getValue(Expr $expr, bool $resolvedClassReference = false) + public function getValue(Expr $expr, bool $resolvedClassReference = false): mixed { if ($expr instanceof Concat) { return $this->processConcat($expr, $resolvedClassReference); diff --git a/src/Validation/RectorAssert.php b/src/Validation/RectorAssert.php index 93d99bfd27f..2032802bae0 100644 --- a/src/Validation/RectorAssert.php +++ b/src/Validation/RectorAssert.php @@ -19,6 +19,19 @@ final class RectorAssert */ private const CLASS_NAME_REGEX = '#^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*$#'; + /** + * @var string + */ + private const NAMESPACE_REGEX = '#^' . self::NAKED_NAMESPACE_REGEX . '$#'; + + /** + * @see https://stackoverflow.com/a/60470526/1348344 + * @see https://regex101.com/r/37aUWA/1 + * + * @var string + */ + private const NAKED_NAMESPACE_REGEX = '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\\\\]*[a-zA-Z0-9_\x7f-\xff]'; + /** * @see https://www.php.net/manual/en/language.variables.basics.php * @see https://regex101.com/r/hFw17T/1 @@ -41,11 +54,26 @@ final class RectorAssert * * @var string */ - private const FUNCTION_NAME_REGEX = '#([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]))?([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)$#'; + private const FUNCTION_NAME_REGEX = '#^(' . self::NAKED_NAMESPACE_REGEX . '\\\\)?([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)$#'; /** - * Assert value is valid class name + * @see https://www.php.net/manual/en/language.constants.php + * @see https://regex101.com/r/Fu6WHQ/1 + * + * @var string */ + private const CONSTANT_REGEX = '#^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$#'; + + public static function namespaceName(string $name): void + { + self::elementName($name, self::NAMESPACE_REGEX, 'namespace'); + } + + public static function constantName(string $name): void + { + self::elementName($name, self::CONSTANT_REGEX, 'constant'); + } + public static function className(string $name): void { self::elementName($name, self::CLASS_NAME_REGEX, 'class'); diff --git a/src/functions/node_helper.php b/src/functions/node_helper.php index 176ff63e216..ad883754d6c 100644 --- a/src/functions/node_helper.php +++ b/src/functions/node_helper.php @@ -7,10 +7,7 @@ use Tracy\Dumper; if (! function_exists('dump_with_depth')) { - /** - * @param mixed $value - */ - function dump_with_depth($value, int $depth = 2): void + function dump_with_depth(mixed $value, int $depth = 2): void { Dumper::dump($value, [ Dumper::DEPTH => $depth, diff --git a/tests/Validation/RectorAssertTest.php b/tests/Validation/RectorAssertTest.php index 3ee3a22d391..a16a2bb72a7 100644 --- a/tests/Validation/RectorAssertTest.php +++ b/tests/Validation/RectorAssertTest.php @@ -60,7 +60,7 @@ public function testValidFunctionName(string $functionName): void } /** - * @return \Iterator + * @return Iterator */ public function provideDataValidFunctionNames(): Iterator { @@ -79,7 +79,7 @@ public function testValidMethodName(string $methodName): void } /** - * @return \Iterator + * @return Iterator */ public function provideDataValidMehtodNames(): Iterator { @@ -87,4 +87,40 @@ public function provideDataValidMehtodNames(): Iterator yield ['__method_magic']; yield ['__M3th0d']; } + + /** + * @dataProvider provideDataInvalidFunctionNames() + */ + public function testInvalidFunctionName(string $functionName): void + { + $this->expectException(InvalidArgumentException::class); + RectorAssert::functionName($functionName); + } + + public function provideDataInvalidFunctionNames(): Iterator + { + yield ['35']; + yield ['/function']; + yield ['$function']; + yield ['-key_name']; + } + + /** + * @dataProvider provideDataInvalidNamespaceNames() + */ + public function testNamespaceName(string $namespaceName): void + { + $this->expectException(InvalidArgumentException::class); + RectorAssert::namespaceName($namespaceName); + } + + /** + * @return Iterator + */ + public function provideDataInvalidNamespaceNames(): Iterator + { + yield ['321Namespace']; + yield ['$__Namespace']; + yield ['Name*space']; + } }