jmespath-community / jmespath.spec

JMESPath Specification
6 stars 3 forks source link

`index-expression` should be allowed on the rhs of a `sub-expression`. #77

Closed springcomp closed 2 years ago

springcomp commented 2 years ago

The current GRAMMAR does not allow expressions like:

foo.bar[0]

As it would parse as a sub-expression where the right-hand-side part is an index-expression. However, this expression is perfectly valid and supported in all current library implementations. This suggests that the grammar is slightly wrong.

This PR changes the grammar like so:

springcomp commented 2 years ago

Hum, that is wrong actually.

In fact, index-expression is both

However, a bare bracket-specifier is not allowed in the rhs of a sub-expression.

I will prepare a PR that distinguish between the two.

innovate-invent commented 2 years ago

I think this is getting into what I think was a mistake on James' part. We have three operators for the same concept. a | [0] vs a[0] vs a.([0]). Some of these operators are even doing two things in the same operation and the difference is subtle.

If you consider each syntax to have a terminator: | is terminated by | or ) or EOL [ is terminated by ] . is terminated by . or ) or ] or EOL

Each evaluates the lhs and applies the rhs expression to the result.

* or [*] is terminated by | or ) or EOL Projection redefines the function of the next ., allowing any expression to follow, which is inconsistent with not allowing a.[0].

This is what I eliminated in that proposed "upath" syntax I sent you. (Not that I am arguing for it, just to contrast.)

Edit: I just double checked. paren-expression is not permitted in a sub-expression for some reason.

[*][0] vs [*].foo is also strangely inconsistent given how the grammar is described.

springcomp commented 2 years ago

As I’m reworking the .NET parser code for better performance it gives me the opportunity to look at the grammar a with a little more focus.

I think my PR is wrong and that index-expression should not indeed be supported on the right-hand-side of a sub-expression. I agree with you, maybe a paren-expression should probably be supported, even though it does not bring any value to use one on the right-hand-side of a sub-expression.

Anyway, I took some time to go through all the permutations and found one thing strange. I took the liberty to name syntaxes explicitly where some are not named in the original grammar:

bracket-specifier        =  flatten-projection / index / slice-projection
bracket-specifier        =/ list-filter-expression / list-wildcard-projection

flatten-projection       =  "[]"
index                    =  "[" number "]"
list-filter-expression   =  "[?" expression "]"
list-wildcard-projection =  "[" "*" "]"
slice-projection         =  "[" slice-expression"]"

slice-expression         =  [number] ":" [number] [ ":" [number] ] 

index-expression         = expression bracket-specifier / bracket-specifier

What I’m interested in is the interactions between the index-expression and the sub-expression. But first let’s consider the following input document which is just a simple JSON array:

[ {"bar": "baz" } ]

With this input document, all the following expressions, using a bracket-specifier as prefix are valid:

Now, let’s consider the following input document, which wraps the previous JSON array inside an object:

{
  "foo": [ {"bar": "baz" } ]
}

Again, all the following index-expressions – using a bracket-specifier as postfix – are valid:

So that is nice and consistent.

Now, where things get tricky is when you try to use the bracket-specifier on the right-hand-side of a sub-expression. The original grammar does not allow this. However, out of the following potential expressions, the only one that does not raise a syntax error in the JavaScript implementation is the one using the list-wildcard-projection. However, that expression still does not return the "correct" result.

So this leads me to conclude that, no, indeed, an bracket-sepcifier is not supported on the right-hand-side of a sub-expression. The list-wildcard-projection expression should raise a syntax error and the fact that it does not is a bug in the JavaScript implementation.

However, a full index-expression should still be valid on the right-hand-side of a sub-expression.

springcomp commented 2 years ago

So, long story short, it finally clicked !

foo.[*] is a valid expression.

However the right-hand-side [*] is not a list-wildcard-projection because that is illegal from the grammar. It actually is a multi-select-list with a single hash-wildcard-projection entry 😲.

In conclusion, an index-expression is definitely not valid on the right-hand-side of a sub-expression !

innovate-invent commented 2 years ago

Good catch! I think I am going to save this one as another "problems with JMESPath" as this is not intuitive at all.