Avoid inferred type annotations for 'coalesce' arguments

Fixes #5755

src/style-spec/expression/definitions/array.js

@@ -27,10 +27,12 @@ const types = {
 class ArrayAssertion implements Expression {
     type: ArrayType;
     input: Expression;
+    _inferred: boolean;
 
-    constructor(type: ArrayType, input: Expression) {
+    constructor(type: ArrayType, input: Expression, inferred: boolean = false) {
         this.type = type;
         this.input = input;
+        this._inferred = inferred;
     }
 
     static parse(args: Array<mixed>, context: ParsingContext): ?Expression {

src/style-spec/expression/definitions/assertion.js

@@ -29,9 +29,12 @@ class Assertion implements Expression {
     type: Type;
     args: Array<Expression>;
 
-    constructor(type: Type, args: Array<Expression>) {
+    _inferred: boolean;
+
+    constructor(type: Type, args: Array<Expression>, inferred: boolean = false) {
         this.type = type;
         this.args = args;
+        this._inferred = inferred;
     }
 
     static parse(args: Array<mixed>, context: ParsingContext): ?Expression {

src/style-spec/expression/definitions/coalesce.js

@@ -1,6 +1,7 @@
 // @flow
 
 const assert = require('assert');
+const {checkSubtype, ValueType} = require('../types');
 
 import type { Expression } from '../expression';
 import type ParsingContext from '../parsing_context';
@@ -21,18 +22,31 @@ class Coalesce implements Expression {
             return context.error("Expectected at least one argument.");
         }
         let outputType: Type = (null: any);
-        if (context.expectedType && context.expectedType.kind !== 'value') {
-            outputType = context.expectedType;
+        const expectedType = context.expectedType;
+        if (expectedType && expectedType.kind !== 'value') {
+            outputType = expectedType;
         }
         const parsedArgs = [];
+
         for (const arg of args.slice(1)) {
-            const parsed = context.parse(arg, 1 + parsedArgs.length, outputType);
+            const parsed = context.parse(arg, 1 + parsedArgs.length, outputType, undefined, {omitTypeAnnotations: true});
             if (!parsed) return null;
             outputType = outputType || parsed.type;
             parsedArgs.push(parsed);
         }
         assert(outputType);
-        return new Coalesce((outputType: any), parsedArgs);
+
+        // Above, we parse arguments without inferred type annotation so that
+        // they don't produce a runtime error for `null` input, which would
+        // preempt the desired null-coalescing behavior.
+        // Thus, if any of our arguments would have needed an annotation, we
+        // need to wrap the enclosing coalesce expression with it instead.
+        const needsAnnotation = expectedType &&
+            parsedArgs.some(arg => checkSubtype(expectedType, arg.type));
+
+        return needsAnnotation ?
+            new Coalesce(ValueType, parsedArgs) :
+            new Coalesce((outputType: any), parsedArgs);
     }
 
     evaluate(ctx: EvaluationContext) {

src/style-spec/expression/definitions/coercion.js

@@ -31,9 +31,12 @@ class Coercion implements Expression {
     type: Type;
     args: Array<Expression>;
 
-    constructor(type: Type, args: Array<Expression>) {
+    _inferred: boolean;
+
+    constructor(type: Type, args: Array<Expression>, inferred: boolean = false) {
         this.type = type;
         this.args = args;
+        this._inferred = inferred;
     }
 
     static parse(args: Array<mixed>, context: ParsingContext): ?Expression {

src/style-spec/expression/parsing_context.js

@@ -43,7 +43,20 @@ class ParsingContext {
         this.expectedType = expectedType;
     }
 
-    parse(expr: mixed, index?: number, expectedType?: ?Type, bindings?: Array<[string, Expression]>): ?Expression {
+    /**
+     * @param expr the JSON expression to parse
+     * @param index the optional argument index if this expression is an argument of a parent expression that's being parsed
+     * @param options
+     * @param options.omitTypeAnnotations set true to omit inferred type annotations.  Caller beware: with this option set, the parsed expression's type will NOT satisfy `expectedType` if it would normally be wrapped in an inferred annotation.
+     * @private
+     */
+    parse(
+        expr: mixed,
+        index?: number,
+        expectedType?: ?Type,
+        bindings?: Array<[string, Expression]>,
+        options: {omitTypeAnnotations?: boolean} = {}
+    ): ?Expression {
         let context = this;
         if (index) {
             context = context.concat(index, expectedType, bindings);
@@ -68,26 +81,33 @@ class ParsingContext {
             if (Expr) {
                 let parsed = Expr.parse(expr, context);
                 if (!parsed) return null;
-                const expected = context.expectedType;
-                const actual = parsed.type;
-                if (expected) {
+
+                if (context.expectedType) {
+                    const expected = context.expectedType;
+                    const actual = parsed.type;
+
                     // When we expect a number, string, or boolean but have a
                     // Value, wrap it in a refining assertion, and when we expect
                     // a Color but have a String or Value, wrap it in "to-color"
                     // coercion.
                     const canAssert = expected.kind === 'string' ||
                         expected.kind === 'number' ||
                         expected.kind === 'boolean';
-
-                    if (canAssert && actual.kind === 'value') {
-                        parsed = new Assertion(expected, [parsed]);
-                    } else if (expected.kind === 'array' && actual.kind === 'value') {
-                        parsed = new ArrayAssertion(expected, parsed);
-                    } else if (expected.kind === 'color' && (actual.kind === 'value' || actual.kind === 'string')) {
-                        parsed = new Coercion(expected, [parsed]);
+                    const canAnnotate =
+                        (actual.kind === 'value' && (canAssert || expected.kind === 'array')) ||
+                        expected.kind === 'color' && (actual.kind === 'value' || actual.kind === 'string');
+
+                    if (canAnnotate && !options.omitTypeAnnotations) {
+                        if (canAssert) {
+                            parsed = new Assertion(expected, [parsed], true);
+                        } else if (expected.kind === 'array') {
+                            parsed = new ArrayAssertion(expected, parsed, true);
+                        } else if (expected.kind === 'color') {
+                            parsed = new Coercion(expected, [parsed], true);
+                        }
                     }
 
-                    if (context.checkSubtype(expected, parsed.type)) {
+                    if (!canAnnotate && context.checkSubtype(context.expectedType, parsed.type)) {
                         return null;
                     }
                 }