Less verbose function -> expression conversion

[mourner]

Fixes #7336. Note that typeof obj === 'object' should be true for all non-primitive values in the spec (including arrays), so we don't need more exhaustive checks.

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality covered by existing tests
• document any changes to public APIs
• post benchmark scores n/a
• manually test the debug page
• tagged @mapbox/studio if this PR includes style spec changes

[samanpwbb]

Thanks for taking care of this so quickly – looks like exactly the change we need in Studio.

[ChrisLoer]

This looks right to me -- it's basically specifically string/boolean/number that we're allowing without the "literal" wrapper, right?

Can we add a legacy expression test that uses @samanpwbb 's function from #7336 ?

[mourner]

@ChrisLoer there are already legacy tests like this, e.g. this one, it's just that the tests don't test conversion output itself — only the output after creating a style expression object and serializing it, which somehow strips the unnecessary "literal" wrappers. Not sure how to go about this.

[mourner]

@ChrisLoer tried adding comparison of raw-converted legacy function with serialized one, but there are many expressions which don't roundtrip like this — e.g. ["linear"] will turn into ["exponential", 1], and "black" will turn into ["rgba", 0, 0, 0, 1]. It's reassuring though that 44 failing tests with the test change get down to 22 in this branch.

[ChrisLoer]

Ah right, that's happening from the serialize logic built into Literal:

mapbox-gl-js/src/style-spec/expression/definitions/literal.js

Lines 56 to 74 in c612e07

	serialize(): Array<mixed> {
	if (this.type.kind === 'array' || this.type.kind === 'object') {
	return ["literal", this.value];
	} else if (this.value instanceof Color) {
	// Constant-folding can generate Literal expressions that you
	// couldn't actually generate with a "literal" expression,
	// so we have to implement an equivalent serialization here
	return ["rgba"].concat(this.value.toArray());
	} else if (this.value instanceof Formatted) {
	// Same as Color
	return this.value.serialize();
	} else {
	assert(this.value === null ||
	typeof this.value === 'string' ||
	typeof this.value === 'number' ||
	typeof this.value === 'boolean');
	return (this.value: any);
	}
	}

What about something similar to the "converts token strings into expressions" test in migrate.test.js?

[ChrisLoer]

I went ahead and added a simple test using @samanpwbb's expression to migrate.test.js, along with another variant that requires the literal syntax. The coverage in that unit test is pretty small and I think this issue highlights that maybe relying on test-expressions to cover the migration isn't as robust as we'd hope. Would it make sense to check in an entire style sheet for use by this test in order to get better coverage? At any rate, I think what I added is enough coverage for this change.