Fix arg coercion for multiple overloads, update tests for overloads t…

…hat now successfully parse.
mapbox · Jul 23, 2018 · b807828 · b807828
1 parent c43024d
commit b807828
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 38 deletions.
diff --git a/src/style-spec/expression/compound_expression.js b/src/style-spec/expression/compound_expression.js
@@ -70,23 +70,32 @@ class CompoundExpression implements Expression {
         let signatureContext: ParsingContext = (null: any);
 
         for (const [params, evaluate] of overloads) {
+            // Use a fresh context for each attempted signature so that, if
+            // we eventually succeed, we haven't polluted `context.errors`.
+            signatureContext = new ParsingContext(context.registry, context.path, null, context.scope);
+
             // First parse all the args, potentially coercing to the
             // types expected by this overload.
             const parsedArgs: Array<Expression> = [];
+            let argParseFailed = false;
             for (let i = 1; i < args.length; i++) {
                 const arg = args[i];
                 const expectedType = Array.isArray(params) ?
                     params[i - 1] :
                     params.type;
 
-                const parsed = context.parse(arg, 1 + parsedArgs.length, expectedType);
-                if (!parsed) return null;
+                const parsed = signatureContext.parse(arg, 1 + parsedArgs.length, expectedType);
+                if (!parsed) {
+                    argParseFailed = true;
+                    break;
+                }
                 parsedArgs.push(parsed);
             }
-
-            // Use a fresh context for each attempted signature so that, if
-            // we eventually succeed, we haven't polluted `context.errors`.
-            signatureContext = new ParsingContext(context.registry, context.path, null, context.scope);
+            if (argParseFailed) {
+                // Couldn't coerce args of this overload to expected type, move
+                // on to next one.
+                continue;
+            }
 
             if (Array.isArray(params)) {
                 if (params.length !== parsedArgs.length) {

diff --git a/test/integration/expression-tests/greater/value/test.json b/test/integration/expression-tests/greater/value/test.json
@@ -2,13 +2,11 @@
   "expression": [">", ["string", ["get", "x"]], ["get", "y"]],
   "expected": {
     "compiled": {
-      "result": "error",
-      "errors": [
-        {
-          "key": "",
-          "error": "Expected arguments of type (number, number) | (string, string), but found (string, value) instead."
-        }
-      ]
-    }
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "serialized": [">", ["string", ["get", "x"]], ["string", ["get", "y"]]]
   }
 }
diff --git a/test/integration/expression-tests/greater_or_equal/value/test.json b/test/integration/expression-tests/greater_or_equal/value/test.json
@@ -2,13 +2,11 @@
   "expression": [">=", ["string", ["get", "x"]], ["get", "y"]],
   "expected": {
     "compiled": {
-      "result": "error",
-      "errors": [
-        {
-          "key": "",
-          "error": "Expected arguments of type (number, number) | (string, string), but found (string, value) instead."
-        }
-      ]
-    }
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "serialized": [">=", ["string", ["get", "x"]], ["string", ["get", "y"]]]
   }
 }
diff --git a/test/integration/expression-tests/less/value/test.json b/test/integration/expression-tests/less/value/test.json
@@ -2,13 +2,11 @@
   "expression": ["<", ["string", ["get", "x"]], ["get", "y"]],
   "expected": {
     "compiled": {
-      "result": "error",
-      "errors": [
-        {
-          "key": "",
-          "error": "Expected arguments of type (number, number) | (string, string), but found (string, value) instead."
-        }
-      ]
-    }
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "serialized": ["<", ["string", ["get", "x"]], ["string", ["get", "y"]]]
   }
 }
diff --git a/test/integration/expression-tests/less_or_equal/value/test.json b/test/integration/expression-tests/less_or_equal/value/test.json
@@ -2,13 +2,11 @@
   "expression": ["<=", ["string", ["get", "x"]], ["get", "y"]],
   "expected": {
     "compiled": {
-      "result": "error",
-      "errors": [
-        {
-          "key": "",
-          "error": "Expected arguments of type (number, number) | (string, string), but found (string, value) instead."
-        }
-      ]
-    }
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "serialized": ["<=", ["string", ["get", "x"]], ["string", ["get", "y"]]]
   }
 }