Improve namespace names validation (#2670)

* inline mixed type * improve namespace function names * add validation for namespace, fix namespaced function regex
rectorphp · Jul 16, 2022 · 97ef9ff · 97ef9ff
1 parent e302870
commit 97ef9ff
Show file tree

Hide file tree

Showing 15 changed files with 109 additions and 69 deletions.
diff --git a/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc b/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/Fixture/fixture4.php.inc
diff --git a/...enaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc b/...enaming/Rector/Namespace_/RenameNamespaceRector/Fixture/skip_underscore_namespace.php.inc
@@ -0,0 +1,11 @@
+<?php
+
+namespace Rector\Tests\Renaming\Rector\Namespace_\RenameNamespaceRector\Fixture;
+
+final class SkipUnderscoreNamespace
+{
+    public function someClass()
+    {
+        return new \PHPUnit_Framework_Something;
+    }
+}
diff --git a/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/config/configured_rule.php b/rules-tests/Renaming/Rector/Namespace_/RenameNamespaceRector/config/configured_rule.php
@@ -11,7 +11,7 @@
             'OldNamespace' => '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
@@ -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
@@ -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
@@ -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<string, string> $configuration */
         $this->oldToNewConstants = $configuration;
     }

diff --git 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
@@ -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
@@ -4,13 +4,18 @@
 
 namespace Rector\Renaming\ValueObject;
 
+use Rector\Core\Validation\RectorAssert;
+
 final class RenamedNamespace
 {
     public function __construct(
         private readonly string $currentName,
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -60,7 +60,7 @@ public function testValidFunctionName(string $functionName): void
     }
 
     /**
-     * @return \Iterator<string[]>
+     * @return Iterator<string[]>
      */
     public function provideDataValidFunctionNames(): Iterator
     {
@@ -79,12 +79,48 @@ public function testValidMethodName(string $methodName): void
     }
 
     /**
-     * @return \Iterator<string[]>
+     * @return Iterator<string[]>
      */
     public function provideDataValidMehtodNames(): Iterator
     {
         yield ['some_method'];
         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<string[]>
+     */
+    public function provideDataInvalidNamespaceNames(): Iterator
+    {
+        yield ['321Namespace'];
+        yield ['$__Namespace'];
+        yield ['Name*space'];
+    }
 }