Merge pull request #6679 from morozov/remove-is-identifier-quoted

Disallow mixing qualified and unqualified names
doctrine · Dec 30, 2024 · 12ae09b · 12ae09b
2 parents eb79e6d + 58ab838
commit 12ae09b
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 85 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -8,6 +8,21 @@ awareness about deprecated code.
 
 # Upgrade to 5.0
 
+## BC BREAK: Removed `AbstractAsset::isIdentifierQuoted()`
+
+The `AbstractAsset::isIdentifierQuoted()` method has been removed.
+
+## BC BREAK: Disallowed mixing unqualified and qualified names in a schema without a default namespace
+
+If a schema lacks a default namespace configuration and has at least one object with an unqualified name, adding or
+referencing objects with qualified names is forbidden.
+
+If a schema lacks a default namespace configuration and has at least one object with a qualified name, adding or
+referencing objects with unqualified names is forbidden.
+
+Mixing unqualified and qualified names is permitted as long as the schema is configured to use a default namespace. In
+this case, the default namespace will be used to resolve unqualified names.
+
 ## BC BREAK: Removed `AbstractAsset::getQuotedName()`
 
 The `AbstractAsset::getQuotedName()` method has been removed.

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -62,18 +62,6 @@
                 -->
                 <referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::getNameParser" />
                 <referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::setName" />
-
-                <!--
-                    https://github.com/doctrine/dbal/pull/6674
-                    TODO: remove in 5.0.0
-                -->
-                <referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::getQuotedName" />
-
-                <!--
-                    https://github.com/doctrine/dbal/pull/6677
-                    TODO: remove in 5.0.0
-                -->
-                <referencedMethod name="Doctrine\DBAL\Schema\AbstractAsset::isIdentifierQuoted" />
             </errorLevel>
         </DeprecatedMethod>
         <DocblockTypeContradiction>

diff --git a/src/Schema/AbstractAsset.php b/src/Schema/AbstractAsset.php
@@ -11,7 +11,6 @@
 use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
 use Doctrine\DBAL\Schema\Name\Parser;
 use Doctrine\DBAL\Schema\Name\UnqualifiedName;
-use Doctrine\Deprecations\Deprecation;
 
 use function array_map;
 use function assert;
@@ -118,24 +117,6 @@ public function isQuoted(): bool
         return $this->_quoted;
     }
 
-    /**
-     * Checks if this identifier is quoted.
-     *
-     * @deprecated Parse the name and introspect its identifiers individually using {@see Identifier::isQuoted()}
-     *             instead.
-     */
-    protected function isIdentifierQuoted(string $identifier): bool
-    {
-        Deprecation::triggerIfCalledFromOutside(
-            'doctrine/dbal',
-            'https://github.com/doctrine/dbal/pull/6677',
-            '%s is deprecated and will be removed in 5.0.',
-            __METHOD__,
-        );
-
-        return isset($identifier[0]) && ($identifier[0] === '`' || $identifier[0] === '"' || $identifier[0] === '[');
-    }
-
     /**
      * Trim quotes from the identifier.
      */

diff --git a/src/Schema/Exception/ImproperlyQualifiedName.php b/src/Schema/Exception/ImproperlyQualifiedName.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Schema\Exception;
+
+use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
+use Doctrine\DBAL\Schema\SchemaException;
+use InvalidArgumentException;
+
+use function sprintf;
+
+/** @psalm-immutable */
+class ImproperlyQualifiedName extends InvalidArgumentException implements SchemaException
+{
+    public static function fromUnqualifiedName(OptionallyQualifiedName $name): self
+    {
+        return new self(sprintf('Schema uses qualified names, but %s is unqualified.', $name->toString()));
+    }
+
+    public static function fromQualifiedName(OptionallyQualifiedName $name): self
+    {
+        return new self(sprintf('Schema uses unqualified names, but %s is qualified.', $name->toString()));
+    }
+}
diff --git a/src/Schema/Schema.php b/src/Schema/Schema.php
@@ -6,6 +6,7 @@
 
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Schema\Exception\ImproperlyQualifiedName;
 use Doctrine\DBAL\Schema\Exception\InvalidName;
 use Doctrine\DBAL\Schema\Exception\NamespaceAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\SequenceAlreadyExists;
@@ -20,7 +21,6 @@
 use Doctrine\DBAL\Schema\Name\UnqualifiedName;
 use Doctrine\DBAL\SQL\Builder\CreateSchemaObjectsSQLBuilder;
 use Doctrine\DBAL\SQL\Builder\DropSchemaObjectsSQLBuilder;
-use Doctrine\Deprecations\Deprecation;
 
 use function array_values;
 use function count;
@@ -217,22 +217,12 @@ private function getKeyFromResolvedName(OptionallyQualifiedName $name): string
 
         if ($qualifier !== null) {
             if ($this->usesUnqualifiedNames) {
-                Deprecation::trigger(
-                    'doctrine/dbal',
-                    'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
-                    'Using qualified names to create or reference objects in a schema that uses unqualified '
-                        . 'names is deprecated.',
-                );
+                throw ImproperlyQualifiedName::fromQualifiedName($name);
             }
 
             $key = $qualifier->getValue() . '.' . $key;
         } elseif (count($this->namespaces) > 0) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
-                'Using unqualified names to create or reference objects in a schema that uses qualified '
-                    . 'names and lacks a default namespace configuration is deprecated.',
-            );
+            throw ImproperlyQualifiedName::fromUnqualifiedName($name);
         }
 
         return strtolower($key);
@@ -271,26 +261,14 @@ private function resolveName(OptionallyQualifiedName $name): OptionallyQualified
         return $name;
     }
 
-    /**
-     * Returns the unquoted representation of a given asset name.
-     */
-    private function getUnquotedAssetName(string $assetName): string
-    {
-        if ($this->isIdentifierQuoted($assetName)) {
-            return $this->trimQuotes($assetName);
-        }
-
-        return $assetName;
-    }
-
     /**
      * Does this schema have a namespace with the given name?
      */
     public function hasNamespace(string $name): bool
     {
-        $name = strtolower($this->getUnquotedAssetName($name));
+        $key = $this->getNamespaceKey($name);
 
-        return isset($this->namespaces[$name]);
+        return isset($this->namespaces[$key]);
     }
 
     /**
@@ -333,17 +311,33 @@ public function getSequences(): array
      */
     public function createNamespace(string $name): self
     {
-        $unquotedName = strtolower($this->getUnquotedAssetName($name));
+        $key = $this->getNamespaceKey($name);
 
-        if (isset($this->namespaces[$unquotedName])) {
-            throw NamespaceAlreadyExists::new($unquotedName);
+        if (isset($this->namespaces[$key])) {
+            throw NamespaceAlreadyExists::new($name);
         }
 
-        $this->namespaces[$unquotedName] = $name;
+        $this->namespaces[$key] = $name;
 
         return $this;
     }
 
+    /**
+     * Returns the key that will be used to store the given namespace name in the collection of namespaces.
+     */
+    private function getNamespaceKey(string $name): string
+    {
+        $parser = Parsers::getUnqualifiedNameParser();
+
+        try {
+            $parsedName = $parser->parse($name);
+        } catch (Parser\Exception $e) {
+            throw InvalidName::fromParserException($name, $e);
+        }
+
+        return strtolower($parsedName->getIdentifier()->getValue());
+    }
+
     /**
      * Creates a new table.
      */

diff --git a/tests/Schema/SchemaTest.php b/tests/Schema/SchemaTest.php
@@ -5,6 +5,7 @@
 namespace Doctrine\DBAL\Tests\Schema;
 
 use Doctrine\DBAL\Exception;
+use Doctrine\DBAL\Schema\Exception\ImproperlyQualifiedName;
 use Doctrine\DBAL\Schema\Exception\InvalidName;
 use Doctrine\DBAL\Schema\Name\Identifier;
 use Doctrine\DBAL\Schema\Schema;
@@ -13,16 +14,13 @@
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Types\Types;
-use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use PHPUnit\Framework\TestCase;
 
 use function array_shift;
 use function strlen;
 
 class SchemaTest extends TestCase
 {
-    use VerifyDeprecations;
-
     public function testAddTable(): void
     {
         $tableName = 'public.foo';
@@ -345,18 +343,14 @@ public function testCreatesNamespaceThroughAddingSequenceImplicitly(): void
 
     public function testAddObjectWithQualifiedNameAfterUnqualifiedName(): void
     {
-        $this->expectDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
-        );
+        $this->expectException(ImproperlyQualifiedName::class);
 
         new Schema([new Table('t'), new Table('public.t')]);
     }
 
     public function testAddObjectWithUnqualifiedNameAfterQualifiedName(): void
     {
-        $this->expectDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
-        );
+        $this->expectException(ImproperlyQualifiedName::class);
 
         new Schema([new Table('public.t'), new Table('t')]);
     }
@@ -365,9 +359,7 @@ public function testReferenceByQualifiedNameAmongUnqualifiedNames(): void
     {
         $schema = new Schema([new Table('t')]);
 
-        $this->expectDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
-        );
+        $this->expectException(ImproperlyQualifiedName::class);
 
         $schema->hasTable('public.t');
     }
@@ -376,9 +368,7 @@ public function testReferenceByUnqualifiedNameAmongQualifiedNames(): void
     {
         $schema = new Schema([new Table('public.t')]);
 
-        $this->expectDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
-        );
+        $this->expectException(ImproperlyQualifiedName::class);
 
         $schema->hasTable('t');
     }
@@ -388,23 +378,21 @@ public function testAddObjectWithQualifiedNameAfterUnqualifiedNameWithDefaultNam
         $schemaConfig = new SchemaConfig();
         $schemaConfig->setName('public');
 
-        $this->expectNoDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-qualified-names',
-        );
+        $schema = new Schema([new Table('t'), new Table('public.s')], [], $schemaConfig);
 
-        new Schema([new Table('t'), new Table('public.s')], [], $schemaConfig);
+        self::assertTrue($schema->hasTable('t'));
+        self::assertTrue($schema->hasTable('public.s'));
     }
 
     public function testAddObjectWithUnqualifiedNameAfterQualifiedNameWithDefaultNamespace(): void
     {
         $schemaConfig = new SchemaConfig();
         $schemaConfig->setName('public');
 
-        $this->expectNoDeprecationWithIdentifier(
-            'https://github.com/doctrine/dbal/pull/6677#user-content-unqualified-names',
-        );
+        $schema = new Schema([new Table('public.t'), new Table('s')], [], $schemaConfig);
 
-        new Schema([new Table('public.t'), new Table('s')], [], $schemaConfig);
+        self::assertTrue($schema->hasTable('public.t'));
+        self::assertTrue($schema->hasTable('s'));
     }
 
     public function testReferencingByUnqualifiedNameAmongQualifiedNamesWithDefaultNamespace(): void