Merge pull request #11 from jg-rp/fix-canonical-paths

Fix normalized and canonical paths

.gitmodules

@@ -1,3 +1,6 @@
 [submodule "tests/cts"]
 	path = tests/cts
 	url = git@github.com:jsonpath-standard/jsonpath-compliance-test-suite.git
+[submodule "tests/nts"]
+	path = tests/nts
+	url = git@github.com:jg-rp/jsonpath-compliance-normalized-paths.git

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Python JSONPath RFC 9535 Change Log
 
+## Version 0.1.4 (unreleased)
+
+**Fixes**
+
+- Fixed normalized paths produced by `JSONPathNode.path()`. Previously we were not handling some escape sequences correctly in name selectors.
+- Fixed serialization of `JSONPathQuery` instances. `JSONPathQuery.__str__()` now serialized name selectors and string literals to the canonical format, similar to normalized paths. We're also now minimizing the use of parentheses when serializing logical expressions.
+
 ## Version 0.1.3
 
 **Fixes**

jsonpath_rfc9535/__about__.py

@@ -1 +1 @@
-__version__ = "0.1.3"
+__version__ = "0.1.4"

jsonpath_rfc9535/filter_expressions.py

@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import json
 from abc import ABC
 from abc import abstractmethod
 from typing import TYPE_CHECKING
@@ -16,6 +15,7 @@
 
 from .exceptions import JSONPathTypeError
 from .node import JSONPathNodeList
+from .serialize import canonical_string
 
 if TYPE_CHECKING:
     from .environment import JSONPathEnvironment
@@ -45,6 +45,12 @@ def evaluate(self, context: FilterContext) -> object:
         """
 
 
+PRECEDENCE_LOWEST = 1
+PRECEDENCE_LOGICAL_OR = 3
+PRECEDENCE_LOGICAL_AND = 4
+PRECEDENCE_PREFIX = 7
+
+
 class FilterExpression(Expression):
     """An expression that evaluates to `true` or `false`."""
 
@@ -55,7 +61,7 @@ def __init__(self, token: Token, expression: Expression) -> None:
         self.expression = expression
 
     def __str__(self) -> str:
-        return str(self.expression)
+        return self._canonical_string(self.expression, PRECEDENCE_LOWEST)
 
     def __eq__(self, other: object) -> bool:
         return (
@@ -66,6 +72,31 @@ def evaluate(self, context: FilterContext) -> bool:
         """Evaluate the filter expression in the given _context_."""
         return _is_truthy(self.expression.evaluate(context))
 
+    def _canonical_string(self, expression: Expression, parent_precedence: int) -> str:
+        if isinstance(expression, LogicalExpression):
+            if expression.operator == "&&":
+                left = self._canonical_string(expression.left, PRECEDENCE_LOGICAL_AND)
+                right = self._canonical_string(expression.right, PRECEDENCE_LOGICAL_AND)
+                expr = f"{left} && {right}"
+                return (
+                    f"({expr})" if parent_precedence >= PRECEDENCE_LOGICAL_AND else expr
+                )
+
+            if expression.operator == "||":
+                left = self._canonical_string(expression.left, PRECEDENCE_LOGICAL_OR)
+                right = self._canonical_string(expression.right, PRECEDENCE_LOGICAL_OR)
+                expr = f"{left} || {right}"
+                return (
+                    f"({expr})" if parent_precedence >= PRECEDENCE_LOGICAL_OR else expr
+                )
+
+        if isinstance(expression, PrefixExpression):
+            operand = self._canonical_string(expression.right, PRECEDENCE_PREFIX)
+            expr = f"!{operand}"
+            return f"({expr})" if parent_precedence > PRECEDENCE_PREFIX else expr
+
+        return str(expression)
+
 
 LITERAL_T = TypeVar("LITERAL_T")
 
@@ -105,7 +136,7 @@ class StringLiteral(FilterExpressionLiteral[str]):
     __slots__ = ()
 
     def __str__(self) -> str:
-        return json.dumps(self.value)
+        return canonical_string(self.value)
 
 
 class IntegerLiteral(FilterExpressionLiteral[int]):

jsonpath_rfc9535/node.py

@@ -7,6 +7,8 @@
 from typing import Tuple
 from typing import Union
 
+from .serialize import canonical_string
+
 if TYPE_CHECKING:
     from .environment import JSONValue
 
@@ -39,7 +41,8 @@ def __init__(
     def path(self) -> str:
         """Return the normalized path to this node."""
         return "$" + "".join(
-            f"['{p}']" if isinstance(p, str) else f"[{p}]" for p in self.location
+            f"[{canonical_string(p)}]" if isinstance(p, str) else f"[{p}]"
+            for p in self.location
         )
 
     def new_child(self, value: object, key: Union[int, str]) -> JSONPathNode:

jsonpath_rfc9535/selectors.py

@@ -15,6 +15,7 @@
 from .exceptions import JSONPathIndexError
 from .exceptions import JSONPathTypeError
 from .filter_expressions import FilterContext
+from .serialize import canonical_string
 
 if TYPE_CHECKING:
     from .environment import JSONPathEnvironment
@@ -60,7 +61,7 @@ def __init__(
         self.name = name
 
     def __str__(self) -> str:
-        return repr(self.name)
+        return canonical_string(self.name)
 
     def __eq__(self, __value: object) -> bool:
         return (

jsonpath_rfc9535/serialize.py

@@ -0,0 +1,9 @@
+"""Helper functions for serializing compiled JSONPath queries."""
+
+import json
+
+
+def canonical_string(value: str) -> str:
+    """Return _value_ as a canonically formatted string literal."""
+    single_quoted = json.dumps(value)[1:-1].replace('\\"', '"').replace("'", "\\'")
+    return f"'{single_quoted}'"

tests/test_nts.py

@@ -0,0 +1,51 @@
+"""Test Python JSONPath against the Normalized Path Test Suite."""
+
+import json
+import operator
+from dataclasses import dataclass
+from typing import List
+
+import pytest
+
+import jsonpath_rfc9535 as jsonpath
+from jsonpath_rfc9535.environment import JSONValue
+
+
+@dataclass
+class NormalizedCase:
+    name: str
+    query: str
+    document: JSONValue
+    paths: List[str]
+
+
+def normalized_cases() -> List[NormalizedCase]:
+    with open("tests/nts/normalized_paths.json", encoding="utf8") as fd:
+        data = json.load(fd)
+    return [NormalizedCase(**case) for case in data["tests"]]
+
+
+@pytest.mark.parametrize("case", normalized_cases(), ids=operator.attrgetter("name"))
+def test_nts_normalized_paths(case: NormalizedCase) -> None:
+    nodes = jsonpath.find(case.query, case.document)
+    paths = [node.path() for node in nodes]
+    assert paths == case.paths
+
+
+@dataclass
+class CanonicalCase:
+    name: str
+    query: str
+    canonical: str
+
+
+def canonical_cases() -> List[CanonicalCase]:
+    with open("tests/nts/canonical_paths.json", encoding="utf8") as fd:
+        data = json.load(fd)
+    return [CanonicalCase(**case) for case in data["tests"]]
+
+
+@pytest.mark.parametrize("case", canonical_cases(), ids=operator.attrgetter("name"))
+def test_nts_canonical_paths(case: CanonicalCase) -> None:
+    query = jsonpath.compile(case.query)
+    assert str(query) == case.canonical

tests/test_parse.py

@@ -89,7 +89,7 @@ class Case:
     Case(
         description="filter with string literal",
         query="$.some[?(@.thing == 'foo')]",
-        want="$['some'][?@['thing'] == \"foo\"]",
+        want="$['some'][?@['thing'] == 'foo']",
     ),
     Case(
         description="filter with integer literal",
@@ -104,32 +104,32 @@ class Case:
     Case(
         description="filter with logical not",
         query="$.some[?(@.thing > 1 && !$.other)]",
-        want="$['some'][?(@['thing'] > 1 && !$['other'])]",
+        want="$['some'][?@['thing'] > 1 && !$['other']]",
     ),
     Case(
         description="filter with grouped expression",
         query="$.some[?(@.thing > 1 && ($.foo || $.bar))]",
-        want="$['some'][?(@['thing'] > 1 && ($['foo'] || $['bar']))]",
+        want="$['some'][?@['thing'] > 1 && ($['foo'] || $['bar'])]",
     ),
     Case(
         description="comparison to single quoted string literal with escape",
         query="$[?@.foo == 'ba\\'r']",
-        want="$[?@['foo'] == \"ba'r\"]",
+        want="$[?@['foo'] == 'ba\\'r']",
     ),
     Case(
         description="comparison to double quoted string literal with escape",
         query='$[?@.foo == "ba\\"r"]',
-        want='$[?@[\'foo\'] == "ba\\"r"]',
+        want="$[?@['foo'] == 'ba\"r']",
     ),
     Case(
         description="not binds more tightly than or",
         query="$[?!@.a || !@.b]",
-        want="$[?(!@['a'] || !@['b'])]",
+        want="$[?!@['a'] || !@['b']]",
     ),
     Case(
         description="not binds more tightly than and",
         query="$[?!@.a && !@.b]",
-        want="$[?(!@['a'] && !@['b'])]",
+        want="$[?!@['a'] && !@['b']]",
     ),
     Case(
         description="control precedence with parens",