Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unary operator precedence, allow some expressions as object values #3053

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Feb 27, 2024

Currently, the precedence of the unary operator - is too low so it causes some unintuitive results. For example, as I pointed out in #2704 (comment), [-1 as $x | 1,$x] results in [-1,-1] in jq 1.7 (and all previous versions), while most of us will expect to see [1,-1]. When I noticed the behavior a few years ago, I was thinking we should keep the behavior for compatibility, but the issue #2704 was created and convinced me that we should fix the behavior. Actually, We can fix by simply changing the unary operator to be parsed as a part of Term. However, this fix disallows to use the unary operator before reduce, foreach, if, and try expressions. So I would like to make these expression to Term as well, to keep supporting unary operator before them. This change allows these expression as object values; something like { x: if . then 1 else 2 end, y: try error catch ., z: reduce range(3) as $x (0; . + $x) }. Also allows us to use the unary operator in reduce expressions; reduce -.[] as $x (...). I think the changes are unlikely to break existing scripts, but let me clarify what I intentionally dropped supporting; unary operator before function declaration, and labels; [-def f: 1; f], [-label $x | 1].

@itchyny itchyny added the bug label Feb 27, 2024
@wader
Copy link
Member

wader commented Mar 20, 2024

Seems like an improvement and fair compromise. Also nice bonus to allow more things as object literal values, have annoyed me a few times. The parser change looks fine to me but i'm that experience with yacc.

@wader
Copy link
Member

wader commented Apr 29, 2024

@itchyny would it be possible to allow most binops also (except ,)? {a: .b // 1}, {a: 1 + 2} etc

@itchyny
Copy link
Contributor Author

itchyny commented Apr 29, 2024

@wader We'll need to duplicate yacc rules for Exp and ExpD, but yes technically that's possible.

@emanuele6
Copy link
Member

emanuele6 commented Apr 29, 2024

We could also add a special rule to allow foo?//bar since ?+// and ?// are never both accepted in the same context.

@wader
Copy link
Member

wader commented Apr 29, 2024

If not too messy to implement i think it would be nice "quality of life" and consistency improvement. I guessing new users find it confusing that [1+2] works but not {a: 1+2}

@itchyny
Copy link
Contributor Author

itchyny commented May 5, 2024

Now I improved the parser rules to allow binary operator expressions as object values. It wasn't messy than I thought.

 $ ./jq -n '{x: 1 + 2, y: false or true, z: null // 3}'
{
  "x": 3,
  "y": true,
  "z": 3
}

@emanuele6
Copy link
Member

I have just noticed that this patch also makes reduce ... as $x (...; ...)[] work.
Before, you had to write it as reduce ... as $x (...; ...) | .[].
Awesome!

@pkoppstein
Copy link
Contributor

Also the parens previously required for (foreach ...) as $x and (reduce ... ) as $x can now be dropped!

For example:

   ./jq -n 'reduce range(0;1) as $x([]; .+[$x]) as $x | $x'

@wader
Copy link
Member

wader commented May 5, 2024

@itchyny nice work! really like this. coming to gojq soon? :)

@wader
Copy link
Member

wader commented May 5, 2024

Had a quick look at jaq and xq's suppor for object value expressions. jaq seems to already support it (maybe @01mf02 can confirm?) but xq seem to not support it (@MiSawa maybe something to look into?). And jqjq has a wip branch to support it.

@wader
Copy link
Member

wader commented Jun 6, 2024

Also fixes:

$ jq -n '[1,2,3]?[0]'
jq: error: syntax error, unexpected '[', expecting end of file (Unix shell quoting issues?) at <top-level>, line 1:
[1,2,3]?[0]
jq: 1 compile error

$ gojq -n '[1,2,3]?[0]'
1

Add regression test for it?

@01mf02
Copy link

01mf02 commented Jun 19, 2024

@wader, just for reference, in jaq, the predence of negation (-) is already what @itchyny proposed, so of course, I'm all for adopting this. ;) Furthermore, jaq indeed already supports object value expressions. That means that you get:

$ jaq -n '[-1 as $x | 1,$x]'
[
  1,
  -1
]
$ jaq -n '{x: 1 + 2, y: false or true, z: null // 3}'
{
  "x": 3,
  "y": true,
  "z": 3
}
$ jaq -n 'reduce range(0;1) as $x([]; .+[$x]) as $x | $x'
[
  0
]

On the other hand, jaq currently cannot handle reduce range(0;1) as $x([]; .+[$x])[] and [1,2,3]?[0]; however, my new parser (WIP) can handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants