[core] Convert token strings to expressions

[jfirebaugh]

Fixes #11659.

This may need some additional coordination with SDK code. Specifically, the constructor DataDrivenPropertyValue(T) (where T is std::string) no longer accepts token strings for icon-image or text-field. Such strings should be converted to expressions, and then the DataDrivenPropertyValue(PropertyExpression<T>) constructor should be used. It's difficult to tell if the former constructor is in use by SDK code (I can't simply delete it and see what fails to compile, because there are lots of uses that aren't affected).

[1ec5]

Per #11659 (comment), it’s likely that the iOS and macOS map SDKs still support setting properties to constant string expressions containing {token} syntax. A constant string expression still gets converted to a bare string for consumption by mbgl:

mapbox-gl-native/platform/darwin/src/MGLStyleValue_Private.h

Lines 74 to 78 in 192c611

	if (expression.expressionType == NSConstantValueExpressionType) {
	MBGLType mbglValue;
	getMBGLValue(expression.constantValue, mbglValue);
	return mbglValue;
	}

However, we no longer document the token syntax, and removing support for it could be chalked up to a fix for a roundtripping bug:

Fixed an issue where some constant value expressions were treated as key path expressions. It is now possible to set the MGLSymbolStyleLayer.text property to a string that literally contains a word in curly braces.

[1ec5]

we no longer document the token syntax

I stand corrected. Nonetheless, we could implement the reverse of the transformation described in #11659 (comment) at the SDK level to unblock this work: scan the string for curly braces and convert to key path expressions. The downside is that the SDK has no context about the feature’s fields, so #11953 would effectively regress.

[jfirebaugh]

Nonetheless, we could implement the reverse of the transformation described in #11659 (comment) at the SDK level to unblock this work: scan the string for curly braces and convert to key path expressions.

I've done so in the latest commit; could you please take a look?

The downside is that the SDK has no context about the feature’s fields, so #11953 would effectively regress.

#11953 applied only to URLs; tokens for non-existent feature properties have always been replaced with an empty string.

[jfirebaugh]

@tobrun How about Android? Does the SDK need to continue to support e.g. symbolLayer.setProperties(iconImage("{token}"))? In contrast to iOS, this doesn't seem to be documented for Android.

[1ec5]

This part of the template is only for layout properties. If paint properties like background-pattern can also accept tokens, then this change needs to be repeated further down in the template.

[jfirebaugh]

text-field and icon-image, both layout properties, are the only ones that accept tokens, and we don't plan to add any token-accepting properties in the future.

[tobrun]

@tobrun How about Android? Does the SDK need to continue to support e.g. symbolLayer.setProperties(iconImage("{token}"))? In contrast to iOS, this doesn't seem to be documented for Android.

Long term, I feel not as afaik that use-case is completely covered with the get expression. For now, since next release isn't planned to be a major release, it seems that we should as it can potentially break existing client code.

Our documentation is indeed a little sparse on the token syntax, we only show case usage through examples. I have been trying to up our documentation game with the introduction of expressions.

[jfirebaugh]

Looks like Android will continue to support setProperties(iconImage("{token}")) without any additional changes. I'll add a couple of tests to confirm.

[tobrun]

@jfirebaugh did a quick manual test on this PR and still works 👍

[anandthakker]

LGTM, just one additional test case suggestion

[anandthakker]

Maybe add "{token}{token}" as well