Parser should fail with misspelled if keyword?

[pippenpaddleopsicopolis]

trafficstars

Short description

I'm actually not sure if this syntax is valid but I mistyped if (ifv) and the policy parsing didn't fail.

package test

import future.keywords.if

eval(resource) = "pass" ifv {
    # Logic that evaluates to true if the resource is compliant
    true
}

Steps To Reproduce

https://play.openpolicyagent.org/p/ygexaGdhII

Expected behavior

Parser should fail?

Additional context

[srenatus]

eval(resource) = "pass"

ifv {
    # Logic that evaluates to true if the resource is compliant
    true
}

This is what it looks like to the parser: defining a function and a rule.

We could look into requiring a being after a shorthand rule out function definition, maybe?

[anderseknert]

This is similar to the oddness I encountered some week ago, where forgetting to import future keywords would still allow the "if" construct — it would just do something else than what was expected. I wonder if there is any point in us allowing more than one rule to be defined on a single line? It seems like an awful style preference, and one that's much more likely to be the result of a mistake.

[srenatus]

How about making this a strict mode check? It'll be the gracious way out of this. After all, awkward or not, it's valid syntax right now and we'd break it if we changed it.

OTOH, it have a really hard time imaging anyone depending on this possibility. 🤔

[pippenpaddleopsicopolis]

I like the idea of adding this to the strict mode first.

[stale[bot]]

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

[anderseknert]

I looked into this for a rule in Regal, but I think the recommendation here would be to automatically format the package using opa fmt, which would reformat the code similarly to what @srenatus demonstrated above. While the formatter has a few quirks of its own, there are many odd-looking constructs and probable bugs that is solved by presenting the code in a better way. I'm not sure strict mode should flag issues fixed by the formatter.

[stale[bot]]

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

[anderseknert]

Replacing the no longer future import with rego.v1 and this is a compile time error:

1 error occurred: policy.rego:5: rego_parse_error: `if` keyword is required before rule body

Since import rego.v1 effectively replaces import future.keywords and makes both if and contains mandatory, I think we can close this now.