simplify expression outputs validation (mapbox#9355)

procore · Mar 18, 2020 · b2b6a84 · b2b6a84
1 parent 2f4d540
commit b2b6a84
Show file tree

Hide file tree

Showing 22 changed files with 45 additions and 59 deletions.
diff --git a/src/style-spec/expression/compound_expression.js b/src/style-spec/expression/compound_expression.js
@@ -39,8 +39,8 @@ class CompoundExpression implements Expression {
         this.args.forEach(fn);
     }
 
-    possibleOutputs() {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize(): Array<mixed> {

diff --git a/src/style-spec/expression/definitions/assertion.js b/src/style-spec/expression/definitions/assertion.js
@@ -18,7 +18,6 @@ import {typeOf} from '../values';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 const types = {
@@ -105,8 +104,8 @@ class Assertion implements Expression {
         this.args.forEach(fn);
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
+    outputDefined(): boolean {
+        return this.args.every(arg => arg.outputDefined());
     }
 
     serialize(): Array<mixed> {

diff --git a/src/style-spec/expression/definitions/at.js b/src/style-spec/expression/definitions/at.js
@@ -58,8 +58,8 @@ class At implements Expression {
         fn(this.input);
     }
 
-    possibleOutputs() {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/case.js b/src/style-spec/expression/definitions/case.js
@@ -7,7 +7,6 @@ import {BooleanType} from '../types';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 type Branches = Array<[Expression, Expression]>;
@@ -72,10 +71,8 @@ class Case implements Expression {
         fn(this.otherwise);
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return []
-            .concat(...this.branches.map(([_, out]) => out.possibleOutputs()))
-            .concat(this.otherwise.possibleOutputs());
+    outputDefined(): boolean {
+        return this.branches.every(([_, out]) => out.outputDefined()) && this.otherwise.outputDefined();
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js
@@ -8,7 +8,6 @@ import ResolvedImage from '../types/resolved_image';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 class Coalesce implements Expression {
@@ -80,8 +79,8 @@ class Coalesce implements Expression {
         this.args.forEach(fn);
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
+    outputDefined(): boolean {
+        return this.args.every(arg => arg.outputDefined());
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js
@@ -13,7 +13,6 @@ import ResolvedImage from '../types/resolved_image';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 const types = {
@@ -112,8 +111,8 @@ class Coercion implements Expression {
         this.args.forEach(fn);
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
+    outputDefined(): boolean {
+        return this.args.every(arg => arg.outputDefined());
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/collator.js b/src/style-spec/expression/definitions/collator.js
@@ -58,12 +58,12 @@ export default class CollatorExpression implements Expression {
         }
     }
 
-    possibleOutputs() {
+    outputDefined() {
         // Technically the set of possible outputs is the combinatoric set of Collators produced
-        // by all possibleOutputs of locale/caseSensitive/diacriticSensitive
+        // by all possible outputs of locale/caseSensitive/diacriticSensitive
         // But for the primary use of Collators in comparison operators, we ignore the Collator's
-        // possibleOutputs anyway, so we can get away with leaving this undefined for now.
-        return [undefined];
+        // possible outputs anyway, so we can get away with leaving this false for now.
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/comparison.js b/src/style-spec/expression/definitions/comparison.js
@@ -164,8 +164,8 @@ function makeComparison(op: ComparisonOperator, compareBasic, compareWithCollato
             }
         }
 
-        possibleOutputs() {
-            return [true, false];
+        outputDefined(): boolean {
+            return true;
         }
 
         serialize() {

diff --git a/src/style-spec/expression/definitions/format.js b/src/style-spec/expression/definitions/format.js
@@ -117,10 +117,10 @@ export default class FormatExpression implements Expression {
         }
     }
 
-    possibleOutputs() {
+    outputDefined() {
         // Technically the combinatoric set of all children
         // Usually, this.text will be undefined anyway
-        return [undefined];
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/format_section_override.js b/src/style-spec/expression/definitions/format_section_override.js
@@ -3,7 +3,6 @@
 import assert from 'assert';
 import type {Expression} from '../expression';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 import type {ZoomConstantExpression} from '../../expression';
 import {NullType} from '../types';
@@ -43,8 +42,8 @@ export default class FormatSectionOverride<T> implements Expression {
     }
 
     // Cannot be statically evaluated, as the output depends on the evaluation context.
-    possibleOutputs(): Array<Value | void> {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js
@@ -43,9 +43,9 @@ export default class ImageExpression implements Expression {
         fn(this.input);
     }
 
-    possibleOutputs() {
+    outputDefined() {
         // The output of image is determined by the list of available images in the evaluation context
-        return [undefined];
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/in.js b/src/style-spec/expression/definitions/in.js
@@ -80,8 +80,8 @@ class In implements Expression {
         fn(this.haystack);
     }
 
-    possibleOutputs() {
-        return [true, false];
+    outputDefined() {
+        return true;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/interpolate.js b/src/style-spec/expression/definitions/interpolate.js
@@ -11,7 +11,6 @@ import type {Stops} from '../stops';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 export type InterpolationType =
@@ -187,8 +186,8 @@ class Interpolate implements Expression {
         }
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
+    outputDefined(): boolean {
+        return this.outputs.every(out => out.outputDefined());
     }
 
     serialize(): Array<mixed> {

diff --git a/src/style-spec/expression/definitions/length.js b/src/style-spec/expression/definitions/length.js
@@ -47,8 +47,8 @@ class Length implements Expression {
         fn(this.input);
     }
 
-    possibleOutputs() {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/let.js b/src/style-spec/expression/definitions/let.js
@@ -55,8 +55,8 @@ class Let implements Expression {
         return new Let(bindings, result);
     }
 
-    possibleOutputs() {
-        return this.result.possibleOutputs();
+    outputDefined() {
+        return this.result.outputDefined();
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/literal.js b/src/style-spec/expression/definitions/literal.js
@@ -49,8 +49,8 @@ class Literal implements Expression {
 
     eachChild() {}
 
-    possibleOutputs() {
-        return [this.value];
+    outputDefined() {
+        return true;
     }
 
     serialize(): Array<mixed> {

diff --git a/src/style-spec/expression/definitions/match.js b/src/style-spec/expression/definitions/match.js
@@ -8,7 +8,6 @@ import {ValueType, type Type} from '../types';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 
 // Map input label values to output expression index
 type Cases = {[number | string]: number};
@@ -112,10 +111,8 @@ class Match implements Expression {
         fn(this.otherwise);
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return []
-            .concat(...this.outputs.map((out) => out.possibleOutputs()))
-            .concat(this.otherwise.possibleOutputs());
+    outputDefined(): boolean {
+        return this.outputs.every(out => out.outputDefined()) && this.otherwise.outputDefined();
     }
 
     serialize(): Array<mixed> {

diff --git a/src/style-spec/expression/definitions/number_format.js b/src/style-spec/expression/definitions/number_format.js
@@ -119,8 +119,8 @@ export default class NumberFormat implements Expression {
         }
     }
 
-    possibleOutputs() {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/step.js b/src/style-spec/expression/definitions/step.js
@@ -8,7 +8,6 @@ import type {Stops} from '../stops';
 import type {Expression} from '../expression';
 import type ParsingContext from '../parsing_context';
 import type EvaluationContext from '../evaluation_context';
-import type {Value} from '../values';
 import type {Type} from '../types';
 
 class Step implements Expression {
@@ -102,8 +101,8 @@ class Step implements Expression {
         }
     }
 
-    possibleOutputs(): Array<Value | void> {
-        return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
+    outputDefined(): boolean {
+        return this.outputs.every(out => out.outputDefined());
     }
 
     serialize() {

diff --git a/src/style-spec/expression/definitions/var.js b/src/style-spec/expression/definitions/var.js
@@ -34,8 +34,8 @@ class Var implements Expression {
 
     eachChild() {}
 
-    possibleOutputs() {
-        return [undefined];
+    outputDefined() {
+        return false;
     }
 
     serialize() {

diff --git a/src/style-spec/expression/expression.js b/src/style-spec/expression/expression.js
@@ -1,7 +1,6 @@
 // @flow
 
 import type {Type} from './types';
-import type {Value} from './values';
 import type ParsingContext from './parsing_context';
 import type EvaluationContext from './evaluation_context';
 
@@ -16,10 +15,9 @@ export interface Expression {
 
     /**
      * Statically analyze the expression, attempting to enumerate possible outputs. Returns
-     * an array of values plus the sentinel value `undefined`, used to indicate that the
-     * complete set of outputs is statically undecidable.
+     * false if the complete set of outputs is statically undecidable, otherwise true.
      */
-    possibleOutputs(): Array<Value | void>;
+    outputDefined(): boolean;
 
     serialize(): SerializedExpression;
 }

diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js
@@ -17,7 +17,7 @@ export default function validateExpression(options: any): Array<ValidationError>
     const expressionObj = (expression.value: any).expression || (expression.value: any)._styleExpression.expression;
 
     if (options.expressionContext === 'property' && (options.propertyKey === 'text-font') &&
-        expressionObj.possibleOutputs().indexOf(undefined) !== -1) {
+        !expressionObj.outputDefined()) {
         return [new ValidationError(options.key, options.value, `Invalid data expression for "${options.propertyKey}". Output values must be contained as literals within the expression.`)];
     }