Input unset after negative rule evaluation

[gwkunze]

Expected Behavior

package test

test_foo {
  is_fruit with input as {
    "type": "apple",
    "real": true
  }
}

default is_fruit = false

is_fruit {
  input.type == "apple"
  not is_faking_apple with input as {"foo": "bar", "real": input.real}
  input.type == "apple"
}

is_faking_apple {
  input.real == false
  input.foo == "bar"
}

opa test . should be valid

Actual Behavior

The test fails, specifically, it seems input is not there anymore after the not is_faking_apple line. If you comment out the second input.type == "apple" statement, the test passes.

Steps to Reproduce the Problem

See above

Additional Info

Rewriting as follows makes it work:

package test

test_foo {
  is_fruit with input as {
    "type": "apple",
    "real": true
  }
}

default is_fruit = false

is_fruit {
  input.type == "apple"
  not_is_faking_apple with input as {"foo": "bar", "real": input.real}
  input.type == "apple"
}

not_is_faking_apple {
  not is_faking_apple
}

is_faking_apple {
  input.real == false
  input.foo == "bar"
}

but either that should not be necessary or possibly the original example should fail parsing.

[patrick-east]

Poking around at it this does appear to be an issue using with on negated expressions. Confirmed that it doesn't have anything specific to do with input either. A pretty minimal repro looks like:

package repro

foo {
  p with data.bar as {"a": 1}
}

p {
  not false with data.bar as {"a": 2}  # broken
  #true with data.bar as {"a": 2}      # works
  data.bar.a == 1
}

And just query for data.repro.foo

Haven't spent a ton of time looking for the fix yet, but at a guess when we are done evaluating the negated expression we aren't restoring the original with values. Quick look at evalWith shows that we should be pretty safetly restoring the with data back in the iterator function or on error...
https://github.com/open-policy-agent/opa/blob/master/topdown/eval.go#L403-L414

But in evalNot we don't use the iterator function (which does the restore) we do e.next(iter):
https://github.com/open-policy-agent/opa/blob/master/topdown/eval.go#L334-L336

I'll check and see if its as simple as just calling iter(e) instead of e.next(iter). Specific to the with case the iterator does the right thing, restoring state and then calling next(), not sure if that holds for all cases though.