Merge pull request #34 from jg-rp/fix-decode-location

Fix  and test canonical serialisation of `JSONPathNode`

Author: jg-rp

.gitmodules

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

CHANGELOG.md

@@ -1,5 +1,14 @@
 # JSON P3 Change Log
 
+## Version 2.1.0 (unreleased)
+
+**Changes**
+
+- Fixed `JSONPathQuery` serialization. `JSONPathQuery.toString()` was not handling name selectors containing `'` or `\`, and was a bit vague about the format serialized paths would use. `JSONPathQuery.toString()` now accepts an options object with a single `form` option. `form` can be one of `"pretty"` (the default) or `"canonical"`. The canonical format uses bracket notation and single quotes, whereas the pretty format uses shorthand notation where possible and double quotes. See [issue #30](https://github.com/jg-rp/json-p3/issues/30) and [PR #32](https://github.com/jg-rp/json-p3/pull/32).
+- Added `JSONPathNode.getPath(options?)`, which returns a string representation of the node's location. As above, the `form` option can be one of `"pretty"` (the default) or `"canonical"`.
+- Deprecated `JSONPathNode.path` in favour of `JSONPathNode.getPath(options?)`.
+- Changed the string representation of _filter selectors_. Both canonical and pretty formats now only include parentheses where necessary.
+
 ## Version 2.0.0
 
 **Breaking changes**

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "json-p3",
-  "version": "2.0.0",
+  "version": "2.1.0",
   "author": "James Prior",
   "license": "MIT",
   "description": "JSONPath, JSON Pointer and JSON Patch",

package.json

@@ -6,6 +6,7 @@ import { JSONPathQuery } from "./path";
 import { Token } from "./token";
 import { FilterContext, Nothing, SerializationOptions } from "./types";
 import { isNumber, isString } from "../types";
+import { toCanonical } from "./serialize";
 
 /**
  * Base class for all filter expressions.
@@ -70,7 +71,7 @@ export class StringLiteral extends FilterExpressionLiteral {
   }
 
   public toString(): string {
-    return JSON.stringify(this.value);
+    return toCanonical(this.value);
   }
 }
 
@@ -117,6 +118,10 @@ export class PrefixExpression extends FilterExpression {
   }
 }
 
+const PRECEDENCE_LOGICAL_OR = 4;
+const PRECEDENCE_LOGICAL_AND = 5;
+const PRECEDENCE_PREFIX = 7;
+
 export class InfixExpression extends FilterExpression {
   readonly logical: boolean;
 
@@ -159,6 +164,7 @@ export class InfixExpression extends FilterExpression {
   }
 
   public toString(options?: SerializationOptions): string {
+    // Note that `LogicalExpression.toString()` does not call this.
     if (this.logical) {
       return `(${this.left.toString(options)} ${
         this.operator
@@ -183,7 +189,45 @@ export class LogicalExpression extends FilterExpression {
   }
 
   public toString(options?: SerializationOptions): string {
-    return this.expression.toString(options);
+    // Minimize parentheses in logical expressions.
+    function _toString(
+      expression: FilterExpression,
+      parentPrecedence: number,
+    ): string {
+      if (expression instanceof InfixExpression) {
+        let precedence: number;
+        let op: string;
+        let left: string;
+        let right: string;
+
+        if (expression.operator === "&&") {
+          precedence = PRECEDENCE_LOGICAL_AND;
+          op = "&&";
+          left = _toString(expression.left, precedence);
+          right = _toString(expression.right, precedence);
+        } else if (expression.operator === "||") {
+          precedence = PRECEDENCE_LOGICAL_OR;
+          op = "||";
+          left = _toString(expression.left, precedence);
+          right = _toString(expression.right, precedence);
+        } else {
+          return expression.toString(options);
+        }
+
+        const expr = `${left} ${op} ${right}`;
+        return precedence < parentPrecedence ? `(${expr})` : expr;
+      }
+
+      if (expression instanceof PrefixExpression) {
+        const operand = _toString(expression.right, PRECEDENCE_PREFIX);
+        const expr = `!${operand}`;
+        return parentPrecedence > PRECEDENCE_PREFIX ? `(${expr})` : expr;
+      }
+
+      return expression.toString(options);
+    }
+
+    return _toString(this.expression, 0);
   }
 }
 

src/path/expression.ts

@@ -62,16 +62,21 @@ export class JSONPathNode {
     name: string,
     options: SerializationOptions,
   ): string {
-    const serialize = options.form === "canonical" ? toCanonical : toQuoted;
+    const normalized = options.form === "canonical";
+    const serialize = normalized ? toCanonical : toQuoted;
     const hasKeyMark = name.startsWith(KEY_MARK);
     if (hasKeyMark) name = name.slice(1);
     const shorthand = toShorthand(name);
 
     if (hasKeyMark) {
-      return shorthand == null ? `[~${serialize(name)}]` : `.~${shorthand}`;
+      return normalized || shorthand == null
+        ? `[~${serialize(name)}]`
+        : `.~${shorthand}`;
     }
 
-    return shorthand == null ? `[${serialize(name)}]` : `.${shorthand}`;
+    return normalized || shorthand == null
+      ? `[${serialize(name)}]`
+      : `.${shorthand}`;
   }
 }
 

src/path/node.ts

@@ -102,12 +102,12 @@ const testCases: Case[] = [
   {
     description: "filter, logical and",
     path: "$[?@.foo && @.bar]",
-    want: "$[?(@['foo'] && @['bar'])]",
+    want: "$[?@['foo'] && @['bar']]",
   },
   {
     description: "filter, logical or",
     path: "$[?@.foo || @.bar]",
-    want: "$[?(@['foo'] || @['bar'])]",
+    want: "$[?@['foo'] || @['bar']]",
   },
   {
     description: "filter, logical not",

tests/path/canonical_path.test.ts

@@ -0,0 +1,32 @@
+import { readFileSync } from "fs";
+
+import { JSONPathEnvironment } from "../../src/path/environment";
+import { JSONValue } from "../../src/types";
+
+type Case = {
+  name: string;
+  query: string;
+  document: JSONValue;
+  paths: string[];
+};
+
+const normalized_paths = JSON.parse(
+  readFileSync(
+    process.env.JSONP3_NTS_PATH || "tests/path/nts/normalized_paths.json",
+    {
+      encoding: "utf8",
+    },
+  ),
+);
+
+const env = new JSONPathEnvironment();
+
+describe("compliance normalized paths", () => {
+  test.each<Case>(normalized_paths.tests)(
+    "$name",
+    ({ query, document, paths }: Case) => {
+      const rv = env.query(query, document).paths({ form: "canonical" });
+      expect(paths).toStrictEqual(rv);
+    },
+  );
+});