InExpression and NotInExpression are out of (recommended) spec

[mvarendorff2]

The Mapbox Expression docs about in state the following in regards to the syntax:

["in",
    keyword: InputType (boolean, string, or number),
    input: InputType (array or string)
]: boolean

This library however parses in expressions vastly differently by interpreting the first argument as property-key to ["get"] from the feature and all remaining values (opposed to a single allowed value according to the mapbox spec) as haystack to search in for the received property-value:

https://github.com/greensopinion/dart-vector-tile-renderer/blob/1d9f5f59bb67e221ecc4c3f1c8b464acc0da78ef/lib/src/themes/expression/parser/boolean_operator_expression_parser.dart#L44-L61

This parsing of the in (as well as support for the !in expression) appears to support a now deprecated version of MapLibre styles.

I am open to creating a PR for this but I guess it should first be clear how this change would be handled (since it very much is breaking) across this package as well als vector_map_tiles.

[greensopinion]

Thanks for the issue. It's a little difficult to decipher the docs (what does "keyword" mean?)

It would be really good to have a failing test that demonstrates the newer syntax with assertions that make it clear what the expectations should be. Ideally we could have an implementation that supports both the newer syntax and the older deprecated syntax.

[mvarendorff2]

Oh, I agree on both the docs and having a universal implementation. The second might be harder than it seems at first glance, because there are expressions that match both the deprecated as well as the current "schema". I am not sure if providing an additional argument to the parser may be an option? That could be used to provide an opt-in to the non-deprecated versions of the expressions in a minor release while moving it to an opt-out for a potential major release in the future?

[greensopinion]

Are you aware of any themes affected by this issue? It would be good to have some concrete examples.

On Sat, Aug 26, 2023 at 6:24 AM Michel v. Varendorff < @.***> wrote:

Oh, I agree on both the docs and having a universal implementation. The second might be harder than it seems at first glance, because there are expressions that match both the deprecated as well as the current "schema". I am not sure if providing an additional argument to the parser may be an option? That could be used to provide an opt-in to the non-deprecated versions of the expressions in a minor release while moving it to an opt-out for a potential major release in the future?

— Reply to this email directly, view it on GitHub https://github.com/greensopinion/dart-vector-tile-renderer/issues/90#issuecomment-1694340722, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEKDQ7W5FB2KFBY4IA73LSLXXH2JLANCNFSM6AAAAAA3LGLDT4 . You are receiving this because you commented.Message ID: @.***>

[mvarendorff2]

Assuming with "this issue" you mean the one where expressions can be interpreted either in the deprecated way or not, this could be an example taken from the OpenMapTiles Positron style:

https://github.com/openmaptiles/positron-gl-style/blob/9ff6653df158aa95937a16d213ee6aa7b31b550a/style.json#L232

If with "this issue" you mean the different interpretation of the in expression itself, I am not aware of any in the wild but this is a requirement for a project I am currently working on.

I also found an interesting side-note in the Mapbox docs for in:

In the specific case when the second and third arguments are string literals, you must wrap at least one of them in a literal expression to hint correct interpretation to the type system.

My understanding is that by default, Mapbox will interpret ["in", "foo", "bar"] in the deprecated syntax requiring ["in", "foo", ["literal", "bar"]] to force the interpreter to understand each part as expression instead of key + literal respectively.

Maybe that is the way to go then? I opened an issue in the Maplibre Style Spec repo to ask for clarification on this specific case, maybe a response there can serve as guideline.

[greensopinion]

Thanks for being proactive in opening an issue and diving deep.

I should have been more clear - I intended to ask if you were aware of any styles having expressions that are using the new syntax which could also be interpreted as the old syntax.

Your proposal for a solution is a good one. An alternative would be to interpret the expression to both forms, and when the expression is evaluated select the first one that evaluates to a non-null/non-empty value (e.g. using coalesce)

[mvarendorff2]

Yes, the included light_theme in this project for example has one of those expressions:

https://github.com/greensopinion/dart-vector-tile-renderer/blob/1d9f5f59bb67e221ecc4c3f1c8b464acc0da78ef/lib/src/themes/light_theme.dart#L644-L648

This could then be either interpreted as one of these two ways:

1. Is the class property of the feature equal to minor
2. Is the string class a substring of the string minor

[mvarendorff2]

I got a response back in the maplibre spec repository! Essentially, [operator, string, string] is interpreted as the "old" / deprecated syntax where the first argument is the key to be looked up. [operator, string, array], [operator, array, string] and [operator, arrray, array] all evaluate using the expression syntax so indeed as assumed in this comment: https://github.com/greensopinion/dart-vector-tile-renderer/issues/90#issuecomment-1695063585