opa icon indicating copy to clipboard operation
opa copied to clipboard

unary `-` sign breaks when used for `input` document

Open philipaconrad opened this issue 2 years ago • 3 comments

Short description

Quoting from the PR discussion on #4955, @shaded-enmity dug up this test case that fails mysteriously:

One thing I just discovered when refining the grammar for floating point numbers is the handling of unary -:

numeric := -1 # works fine

other := -input.number # error

The second example fails with:

1 error occurred: test_unary.rego:3: rego_parse_error: unexpected assign token: expected rule value term (e.g., other := <VALUE> { ... })
	other := -input.number

This looks like a possible parser bug, where a ref (input.number) is being treated differently somehow than a raw numeric constant.

Steps To Reproduce

opa eval --format=pretty 'output := -input.number'

Expected behavior

Ideally, the error should be different or non-existent for the opa eval example.

Additional Context

This behavior can also be observed on the Rego Playground.

philipaconrad avatar Aug 15 '22 14:08 philipaconrad

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

stale[bot] avatar Sep 14 '22 17:09 stale[bot]

This issue may be stale, but it's still an issue! :sweat_smile:

Hopefully, I or someone else will have time to triage this issue in the parser soon.

philipaconrad avatar Sep 14 '22 20:09 philipaconrad

@philipaconrad I've put it into the backlog; that way, it won't get picked up (and picked on) by the stale bot.

srenatus avatar Sep 15 '22 09:09 srenatus

The underlying issue here is that the parser has been treating unary minus parsing as part of how numeric literals are parsed (ex: -.05 is fine, but -input.x is not).

I suspect the best way to fix this might be to do rewriting into something like numbers.negate(input.x), since that would easily cover most of the obvious syntactic variations around numbers/variables/refs that are possible, at the expense of slightly more complicated parsing/AST structure. :slightly_frowning_face:

philipaconrad avatar Nov 14 '22 22:11 philipaconrad

I also ran into this issue recently.

Rego lists the following production

unary-expr      = "-" expr

which indicates that all expressions must be allowed after the - sign, at least syntactically.

anakrish avatar Dec 08 '22 06:12 anakrish