Merge pull request #55 from wgevaert/improve-name-escape

Be less strict about escaping.

Author: wgevaert

src/Traits/ErrorTrait.php

@@ -35,7 +35,6 @@
 use function is_resource;
 use function is_string;
 use function key;
-use function preg_match;
 use function strpos;
 use TypeError;
 
@@ -110,11 +109,8 @@ private static function assertValidName(string $name): void
             throw new InvalidArgumentException("A name cannot be an empty string");
         }
 
-        if (!preg_match('/^\p{L}[\p{L}\d_]*$/u', $name)) {
-            throw new InvalidArgumentException('A name can only contain alphanumeric characters and underscores and must begin with an alphabetic character');
-        }
-
         if (strlen($name) >= 65535) {
+            // Remark: Actual limit depends on Neo4j version, but we just take the lower bound.
             throw new InvalidArgumentException('A name cannot be longer than 65534 characters');
         }
     }

src/Traits/EscapeTrait.php

@@ -21,7 +21,9 @@
 
 namespace WikibaseSolutions\CypherDSL\Traits;
 
-use InvalidArgumentException;
+use function preg_match;
+use function sprintf;
+use function str_replace;
 
 /**
  * Trait for encoding certain structures that are used in multiple clauses in a
@@ -30,25 +32,31 @@
 trait EscapeTrait
 {
     /**
-     * Escapes the given 'name'. A name is an unquoted literal in a Cypher query, such as variables,
-     * types or property names.
+     * Escapes a 'name' if it needs to be escaped.
+     * @see https://neo4j.com/docs/cypher-manual/4.4/syntax/naming
+     * A 'name' in cypher is any string that should be included directly in a cypher query,
+     * such as variable names, labels, property names and relation types
      *
      * @param string $name
      * @return string
      */
     public static function escape(string $name): string
     {
-        if ($name === "") {
-            return "";
-        }
-
-        if (ctype_alnum($name) && !ctype_digit($name)) {
+        if (preg_match('/^\p{L}[\p{L}\d_]*$/u', $name)) {
             return $name;
         }
 
-        if (strpos($name, '`') !== false) {
-            throw new InvalidArgumentException("A name must not contain a backtick (`)");
-        }
+        return self::escapeRaw($name);
+    }
+
+    /**
+     * Escapes the given $name to be used directly in a CYPHER query.
+     * Note: according to https://github.com/neo4j/neo4j/issues/12901 backslashes might give problems in some Neo4j versions.
+     */
+    public static function escapeRaw($name)
+    {
+        // Escape backticks that are included in $name by doubling them.
+        $name = str_replace('`', '``', $name);
 
         return sprintf("`%s`", $name);
     }

tests/Unit/ParameterTest.php

@@ -68,10 +68,7 @@ public function provideThrowsExceptionOnInvalidData(): array
     {
         return [
             [""],
-            ["@"],
-            ["!"],
-            ["-"],
-            [''],
+            [str_repeat('a', 65535)],
         ];
     }
 }

tests/Unit/Patterns/NodeTest.php

@@ -21,7 +21,6 @@
 
 namespace WikibaseSolutions\CypherDSL\Tests\Unit\Patterns;
 
-use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
 use WikibaseSolutions\CypherDSL\ExpressionList;
 use WikibaseSolutions\CypherDSL\Literals\Decimal;
@@ -49,13 +48,13 @@ public function testEmptyNode(): void
         $this->assertSame($name, $node->getVariable());
     }
 
-    public function testBacktickThrowsException(): void
+    // Further tests can be found in Traits/EscapeTraitTest
+    public function testBacktickIsEscaped(): void
     {
         $node = new Node();
 
-        $this->expectException(InvalidArgumentException::class);
-        $this->expectDeprecationMessage('A name can only contain alphanumeric characters and underscores');
         $node->named('abcdr`eer');
+        $this->assertEquals('(`abcdr``eer`)', $node->toQuery());
     }
 
     /**

tests/Unit/QueryTest.php

@@ -762,7 +762,7 @@ public function testWikiExamples(): void
             ->returning([$tom, $tomHanksMovies])
             ->build();
 
-        $this->assertSame("MATCH (tom:Person {name: 'Tom Hanks'})-[:`ACTED_IN`]->(tomHanksMovies) RETURN tom, tomHanksMovies", $statement);
+        $this->assertSame("MATCH (tom:Person {name: 'Tom Hanks'})-[:ACTED_IN]->(tomHanksMovies) RETURN tom, tomHanksMovies", $statement);
 
         $cloudAtlas = Query::variable("cloudAtlas");
         $cloudAtlasNode = Query::node()
@@ -795,7 +795,7 @@ public function testWikiExamples(): void
             ->returning($coActors->property("name"))
             ->build();
 
-        $this->assertSame("MATCH (tom:Person {name: 'Tom Hanks'})-[:`ACTED_IN`]->(m)<-[:`ACTED_IN`]-(coActors) RETURN coActors.name", $statement);
+        $this->assertSame("MATCH (tom:Person {name: 'Tom Hanks'})-[:ACTED_IN]->(m)<-[:ACTED_IN]-(coActors) RETURN coActors.name", $statement);
 
         /*
          * @see https://gitlab.wikibase.nl/community/libraries/php-cypher-dsl/-/wikis/Usage/Clauses/CALL-procedure-clause

tests/Unit/Traits/EscapeTraitTest.php

@@ -21,7 +21,6 @@
 
 namespace WikibaseSolutions\CypherDSL\Tests\Unit\Traits;
 
-use InvalidArgumentException;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use WikibaseSolutions\CypherDSL\Traits\EscapeTrait;
@@ -66,58 +65,29 @@ public function testUnsafeValueIsEscaped(string $value)
         $this->assertSame($expected, $actual);
     }
 
-    public function testValueWithBacktickThrowsException()
-    {
-        $this->expectException(InvalidArgumentException::class);
-
-        $this->trait->escape("foo`bar");
-    }
-
     public function provideSafeValueIsNotEscapedData(): array
     {
         return [
             ['foobar'],
             ['fooBar'],
             ['FOOBAR'],
+            ['foo_bar'],
+            ['FOO_BAR'],
             ['aaa'],
-            ['bbb'],
-            ['ccc'],
-            ['ddd'],
-            ['eee'],
-            ['fff'],
-            ['ggg'],
-            ['hhh'],
-            ['iii'],
-            ['jjj'],
-            ['kkk'],
-            ['lll'],
-            ['mmm'],
-            ['nnn'],
-            ['ooo'],
-            ['ppp'],
-            ['qqq'],
-            ['rrr'],
-            ['sss'],
-            ['ttt'],
-            ['uuu'],
-            ['vvv'],
-            ['www'],
-            ['xxx'],
-            ['yyy'],
-            ['zzz'],
-            [''],
             ['aaa100'],
             ['a0'],
             ['z10'],
             ['z99'],
+            ['ça'],
+            ['日'],
         ];
     }
 
     public function provideUnsafeValueIsEscapedData(): array
     {
         return [
-            ['foo_bar'],
-            ['FOO_BAR'],
+            [''],
+            ['__FooBar__'],
             ['_'],
             ['__'],
             ['\''],
@@ -129,4 +99,23 @@ public function provideUnsafeValueIsEscapedData(): array
             ['2'],
         ];
     }
+
+    /**
+     * @dataProvider provideValueWithBacktickIsProperlyEscapedData
+     */
+    public function testValueWithBacktickIsProperlyEscaped($input, $expected)
+    {
+        $this->assertSame('`foo``bar`', $this->trait->escape("foo`bar"));
+    }
+
+    public function provideValueWithBacktickIsProperlyEscapedData(): array
+    {
+        return [
+            ['foo`bar','`foo``bar`'],
+            ['`foo','```foo`'],
+            ['foo`','`foo```'],
+            ['foo``bar','`foo````bar`'],
+            ['`foo`','```foo```'],
+        ];
+    }
 }