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.

Sign up for GitHub

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

Jump to bottom

topdown/eval: panic mocking function with rule #5299

Closed
srenatus opened this issue Oct 25, 2022 · 0 comments · Fixed by #5301
Closed

topdown/eval: panic mocking function with rule #5299

srenatus opened this issue Oct 25, 2022 · 0 comments · Fixed by #5301
Assignees
@srenatus
Labels
bug

Comments

@srenatus
Copy link
Contributor 

package play
import future.keywords.if

f(_, _) := []
p if f(1, 2) == [3]

mock_f := [3]
test_p if p with f as mock_f

This panics. It's not a bad idea to use function mocking like this, but it's unsupported right now and results in a panic.

These workarounds do fine:

  1. using a value
mock_f := [3]
test_p if p with f as [3]
  1. using a function
mock_f(_, _) := [3]
test_p if p with f as mock_f
@srenatus srenatus added the bug label Oct 25, 2022
@srenatus srenatus self-assigned this Oct 25, 2022
@srenatus srenatus mentioned this issue Oct 25, 2022
Merged
@srenatus srenatus mentioned this issue Oct 26, 2022
Open
srenatus added a commit to srenatus/opa that referenced this issue Oct 27, 2022
@srenatus
ast/compile: rewrite ref-replacements with non-function values
1e8d2fe 
Before, this was OK:

    test_a {
    . mock_f := true
      allow with f as mock_f
    }

but this had panicked:

    mock_f := true
    test_a {
      allow with f as mock_f
    }

Which, from a user perspective, is quite incomprehensible. Technically,
the first snippet was a (supported) replacement-by-value, and the second
was an unsupported replacement by a rule that was not a function.

Furthermore, the second case wasn't properly caught in the 'with' validations.

Now, we'll capture the situation, and start supporting it. Both snippets will
now work the same, as one would expect from the language surface.

Fixes open-policy-agent#5299.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus added a commit that referenced this issue Oct 27, 2022
@srenatus
ast/compile: rewrite ref-replacements with non-function values (#5301)
3406e96 
Before, this was OK:

    test_a {
    . mock_f := true
      allow with f as mock_f
    }

but this had panicked:

    mock_f := true
    test_a {
      allow with f as mock_f
    }

Which, from a user perspective, is quite incomprehensible. Technically,
the first snippet was a (supported) replacement-by-value, and the second
was an unsupported replacement by a rule that was not a function.

Furthermore, the second case wasn't properly caught in the 'with' validations.

Now, we'll capture the situation, and start supporting it. Both snippets will
now work the same, as one would expect from the language surface.

Fixes #5299.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srenatus srenatus

Labels
bug
Projects
Open Policy Agent
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ast/compile: rewrite ref-replacements with non-function values srenatus/opa
1 participant
@srenatus