Fix function well-typedness and comparison expr arg parsing

jg-rp · Sep 17, 2023 · ad01f3c · ad01f3c
1 parent c1eeb69
commit ad01f3c
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,8 @@
 
 **Fixes**
 
-- Fix validation of JSONPath function expressions that accept a `ValueType` parameter and is called with a function that returns a `ValueType` (like `value()`). Previously a `JSONPathTypeError` was thrown.
+- Fixed well-typedness checks on function calls. Previously we were throwing a `JSONPathTypError` for some valid queries.
+- Fixed parsing of function calls with comparison expressions as arguments.
 - Fixed a bug with unbalanced parentheses detection in JSONPath expressions when parsing nested function calls. In some cases we were not throwing a `JSONPathSyntaxError` when a function was not closed, but an inner function was.
 
 **Features**

diff --git a/src/path/environment.ts b/src/path/environment.ts
@@ -1,9 +1,9 @@
 import { JSONPathTypeError, UndefinedFilterFunctionError } from "./errors";
 import {
-  BooleanLiteral,
   FilterExpression,
   FilterExpressionLiteral,
   FunctionExtension,
+  InfixExpression,
   JSONPathQuery,
 } from "./expression";
 import { Count as CountFilterFunction } from "./functions/count";
@@ -149,10 +149,9 @@ export class JSONPathEnvironment {
           }
           break;
         case FunctionExpressionType.LogicalType:
-          // XXX: BooleanLiteral != LogicalType
-          // LogicalType can be returned by a function or the result of a test/
-          // comparison expression.
-          if (!(arg instanceof BooleanLiteral)) {
+          if (
+            !(arg instanceof JSONPathQuery || arg instanceof InfixExpression)
+          ) {
             throw new JSONPathTypeError(
               `${token.value}() argument ${idx} must be of LogicalType`,
               arg.token,

diff --git a/src/path/parse.ts b/src/path/parse.ts
@@ -380,7 +380,17 @@ export class Parser {
         );
       }
 
-      args.push(func.bind(this)(stream));
+      let expr = func.bind(this)(stream);
+
+      // Could be a comparison/logical expression
+      let peekKind = stream.peek.kind;
+      while (BINARY_OPERATORS.has(peekKind)) {
+        stream.next();
+        expr = this.parseInfixExpression(stream, expr);
+        peekKind = stream.peek.kind;
+      }
+
+      args.push(expr);
 
       if (stream.peek.kind !== TokenKind.RPAREN) {
         if (stream.peek.kind === TokenKind.RBRACKET) break;

diff --git a/tests/path/ietf_function_well_typedness.test.ts b/tests/path/ietf_function_well_typedness.test.ts
@@ -87,23 +87,58 @@ const TEST_CASES: TestCase[] = [
     valid: false,
   },
   {
-    description: "value, comparison",
+    description: "value, non-singular query param, comparison",
     path: "$[?value(@..color) == 'red']",
     valid: true,
   },
   {
-    description: "value, existence",
+    description: "value, non-singular query param",
     path: "$[?value(@..color)]",
     valid: false,
   },
   {
-    description: "function, logical return type, existence",
+    description:
+      "function, singular query, value type param, logical return type",
     path: "$[?bar(@.a)]",
     valid: true,
   },
+  {
+    description:
+      "function, non-singular query, value type param, logical return type",
+    path: "$[?bar(@.*)]",
+    valid: false,
+  },
+  {
+    description:
+      "function, non-singular query, nodes type param, logical return type",
+    path: "$[?bn(@.*)]",
+    valid: true,
+  },
+  {
+    description:
+      "function, non-singular query, logical type param, logical return type",
+    path: "$[?bl(@.*)]",
+    valid: true,
+  },
+  {
+    description:
+      "function, logical type param, comparison, logical return type",
+    path: "$[?bl(1==1)]",
+    valid: true,
+  },
+  {
+    description: "function, logical type param, literal, logical return type",
+    path: "$[?bl(1)]",
+    valid: false,
+  },
+  {
+    description: "function, value type param, literal, logical return type",
+    path: "$[?bar(1)]",
+    valid: true,
+  },
 ];
 
-class MockFooFunction implements FilterFunction {
+class MockFoo implements FilterFunction {
   argTypes = [FunctionExpressionType.NodesType];
   returnType = FunctionExpressionType.NodesType;
 
@@ -112,7 +147,16 @@ class MockFooFunction implements FilterFunction {
   }
 }
 
-class MockBarFunction implements FilterFunction {
+class MockBar implements FilterFunction {
+  argTypes = [FunctionExpressionType.ValueType];
+  returnType = FunctionExpressionType.LogicalType;
+
+  call(): boolean {
+    return false;
+  }
+}
+
+class MockBn implements FilterFunction {
   argTypes = [FunctionExpressionType.NodesType];
   returnType = FunctionExpressionType.LogicalType;
 
@@ -121,10 +165,21 @@ class MockBarFunction implements FilterFunction {
   }
 }
 
+class MockBl implements FilterFunction {
+  argTypes = [FunctionExpressionType.LogicalType];
+  returnType = FunctionExpressionType.LogicalType;
+
+  call(): boolean {
+    return false;
+  }
+}
+
 describe("IETF function well-typedness examples", () => {
   const env = new JSONPathEnvironment();
-  env.functionRegister.set("foo", new MockFooFunction());
-  env.functionRegister.set("bar", new MockBarFunction());
+  env.functionRegister.set("foo", new MockFoo());
+  env.functionRegister.set("bar", new MockBar());
+  env.functionRegister.set("bn", new MockBn());
+  env.functionRegister.set("bl", new MockBl());
   test.each<TestCase>(TEST_CASES)(
     "$description",
     ({ path, valid }: TestCase) => {