Omit inferred type annotations for 'coalesce' arguments

[anandthakker]

As described in #5755, such inferred annotations cause a runtime type error for null inputs, thus preempting the desired null-coalescing behavior of something like ["coalesce", ["get", "a"], ["get", "b"]].

[jfirebaugh]

Rather than wrapping and unwrapping, did you consider an approach where parse gets a flag from Coalesce.parse indicating not to insert inferred assertions?

[anandthakker]

Rather than wrapping and unwrapping, did you consider an approach where parse gets a flag from Coalesce.parse indicating not to insert inferred assertions?

I considered that, but Coalesce.parse also needs to know if an assertion "should" have been added, in order to know whether or not to wrap itself in an outer assertion. Given that, it seemed simpler to contain the logic within Coalesce.parse.

Realizing now that an alternative would be to just always wrap the coalesce in an outer assertion, even if none of its arguments are typed value. Maybe that's better.

[jfirebaugh]

👍 always wrapping with an outer assertion.

[jfirebaugh]

Hmm, not really satisfied with how this turned out. The logic is complex and quite hard to follow.

Can we try another approach?

[anandthakker]

This bit isn't strictly necessary -- we could just always return new Coalesce(ValueType, parsedArgs) -- I figured including it would be more explicit & therefore clearer.

[jfirebaugh]

How about:

                    // When we expect a number, string, boolean, or array but
                    // have a Value, we can wrap it in a refining assertion.
                    // When we expect a Color but have a String or Value, we
                    // can wrap it in "to-color" coercion.
                    // Otherwise, we do static type-checking.
                    if ((expected.kind === 'string' || expected.kind === 'number' || expected.kind === 'boolean') && actual.kind === 'value') {
                        if (!options.omitTypeAnnotations) {
                            parsed = new Assertion(expected, [parsed]);
                        }
                    } else if (expected.kind === 'array' && actual.kind === 'value') {
                        if (!options.omitTypeAnnotations) {
                            parsed = new ArrayAssertion(expected, parsed);
                        }
                    } else if (expected.kind === 'color' && (actual.kind === 'value' || actual.kind === 'string')) {
                        if (!options.omitTypeAnnotations) {
                            parsed = new Coercion(expected, [parsed]);
                        }
                    } else if (context.checkSubtype(context.expectedType, parsed.type)) {
                        return null;
                    }

[anandthakker]

👌

[anandthakker]

Updated with this approach.