ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

ast: Apply location to inner ast.Not expressions (#8727)

Browse Source 
Also fixing locations of and/or expressions.

---------

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This commit is contained in:
Johan Fylling
2026-06-03 12:37:20 +02:00
committed by GitHub
parent 299760c6ab
commit 353033c65c
6 changed files with 526 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
v1/ast/compile_test.go
+132 -49
View File
@@ -13290,15 +13290,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated call with vars, inside comprehension, unsafe assignment",
module: `package negation
import future.keywords.not
p := [x | not x := "foo" ]
f(_) := true
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var x is unsafe",
Code: CompileErr,
Message: "var x is unsafe",
Location: &Location{File: "mod.rego", Row: 4, Col: 15, Text: []byte(`not x := "foo"`)},
},
},
},
@@ -13333,15 +13334,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated assignment",
module: `package negation
import future.keywords.not
p if {
not a := 1
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe", // FIXME: Use more specific error msg: "cannot assign vars inside negated expression"
Code: CompileErr,
Message: "var a is unsafe", // FIXME: Use more specific error msg: "cannot assign vars inside negated expression"
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a := 1")},
},
},
},
@@ -13364,15 +13366,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated assignment, call",
module: `package negation
import future.keywords.not
p if {
not a := 1 + 2
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a := 1 + 2")},
},
},
},
@@ -13398,15 +13401,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated assignment, call, output var",
module: `package negation
import future.keywords.not
p if {
not plus(1, 2, a)
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not plus(1, 2, a)")},
},
},
},
@@ -13431,15 +13435,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated equality, unsafe var",
module: `package negation
import future.keywords.not
p if {
not a == 1
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a == 1")},
},
},
},
@@ -13447,15 +13452,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated equality, unsafe var, explicit not-body",
module: `package negation
import future.keywords.not
p if {
not { a == 1 }
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 12, Text: []byte("a == 1")},
},
},
},
@@ -13463,15 +13469,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated unification, unsafe var",
module: `package negation
import future.keywords.not
p if {
not a = 1
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a = 1")},
},
},
},
@@ -13494,15 +13501,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated unification, unsafe var, call",
module: `package negation
import future.keywords.not
p if {
not a = 1 + 2
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a = 1 + 2")},
},
},
},
@@ -13528,15 +13536,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated enumeration, unsafe var (wildcard)",
module: `package negation
import future.keywords.not
p if {
not input.a[_]
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var _ is unsafe",
Code: CompileErr,
Message: "var _ is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not input.a[_]")},
},
},
},
@@ -13639,15 +13648,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated unsafe var",
module: `package negation
import future.keywords.not
p if {
not a + 2 = 3
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 6, Text: []byte("not a + 2 = 3")},
},
},
},
@@ -13655,15 +13665,16 @@ func TestCompilerNotImport(t *testing.T) {
note: "negated unsafe var, explicit not-body",
module: `package negation
import future.keywords.not
p if {
not { a + 2 = 3 }
}
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 5, Col: 12, Text: []byte("a + 2")},
},
},
},
@@ -13930,7 +13941,7 @@ func TestCompilerNotImport(t *testing.T) {
note: "nested negation (comprehension), unsafe var reference",
module: `package negation
import future.keywords.not
p if {
not [42 |
not v = 2
@@ -13938,8 +13949,9 @@ func TestCompilerNotImport(t *testing.T) {
}`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var v is unsafe",
Code: CompileErr,
Message: "var v is unsafe",
Location: &Location{File: "mod.rego", Row: 6, Col: 7, Text: []byte("not v = 2")},
},
},
},
@@ -14140,7 +14152,7 @@ func TestCompilerNotImport(t *testing.T) {
note: "explicit not-body, var indirection, unsafe var",
module: `package negation
import future.keywords.not
p if {
not {
a = x
@@ -14151,16 +14163,19 @@ func TestCompilerNotImport(t *testing.T) {
`,
expErrs: Errors{
&Error{
Code: CompileErr,
Message: "var a is unsafe",
Code: CompileErr,
Message: "var a is unsafe",
Location: &Location{File: "mod.rego", Row: 6, Col: 7, Text: []byte("a = x")},
},
&Error{
Code: CompileErr,
Message: "var x is unsafe",
Code: CompileErr,
Message: "var x is unsafe",
Location: &Location{File: "mod.rego", Row: 6, Col: 7, Text: []byte("a = x")},
},
&Error{
Code: CompileErr,
Message: "var b is unsafe",
Code: CompileErr,
Message: "var b is unsafe",
Location: &Location{File: "mod.rego", Row: 7, Col: 7, Text: []byte("b = a")},
},
},
},
@@ -14271,18 +14286,72 @@ func TestCompilerNotImport(t *testing.T) {
}
`, popts),
},
{
note: "no import, negated undefined function (regression: GH#8717)",
module: `package negation
p if {
not f(1)
}
`,
expErrs: Errors{
&Error{
Code: TypeErr,
Message: "undefined function f",
Location: &Location{File: "mod.rego", Row: 4, Col: 7, Text: []byte("not f(1)")},
},
},
},
{
note: "implicit not-body, negated undefined function (regression: GH#8717)",
module: `package negation
import future.keywords.not
p if {
not f(1)
}
`,
expErrs: Errors{
&Error{
Code: TypeErr,
Message: "undefined function f",
Location: &Location{File: "mod.rego", Row: 5, Col: 7, Text: []byte("not f(1)")},
},
},
},
{
note: "explicit not-body, negated undefined function (regression: GH#8717)",
module: `package negation
import future.keywords.not
p if {
not { f(1) }
}
`,
expErrs: Errors{
&Error{
Code: TypeErr,
Message: "undefined function f",
Location: &Location{File: "mod.rego", Row: 5, Col: 13, Text: []byte("f(1)")},
},
},
},
}
for _, tc := range tests {
t.Run(tc.note, func(t *testing.T) {
mod, err := ParseModuleWithOpts("mod.rego", tc.module, ParserOptions{
Capabilities: CapabilitiesForThisVersion(),
})
if err != nil {
t.Fatalf("unexpected parse error: %v", err)
}
c := NewCompiler()
c.Compile(map[string]*Module{"mod.rego": MustParseModuleWithOpts(tc.module,
ParserOptions{
Capabilities: CapabilitiesForThisVersion(),
})})
c.Compile(map[string]*Module{"mod.rego": mod})
if len(tc.expErrs) > 0 {
assertErrors(t, c.Errors, tc.expErrs, false)
assertErrors(t, c.Errors, tc.expErrs, true)
} else if len(c.Errors) > 0 {
if c.Failed() {
t.Fatalf("unexpected compile errors: %v", c.Errors)
@@ -14293,6 +14362,20 @@ func TestCompilerNotImport(t *testing.T) {
if diff := cmp.Diff(tc.expMod, c.Modules["mod.rego"]); diff != "" {
t.Errorf("unexpected module (-want, +got):\n%s", diff)
}
// Regression guard for the future.keywords.not location bug (GH#8717)
WalkExprs(c.Modules["mod.rego"], func(expr *Expr) bool {
not, ok := expr.Terms.(*Not)
if !ok {
return false
}
for i, inner := range not.Body {
if inner.Location == nil {
t.Errorf("Not.Body[%d] missing Location: %v", i, inner)
}
}
return false
})
}
for n, m := range c.Modules {
v1/ast/location/location.go
+3
View File
@@ -29,6 +29,9 @@ func NewLocation(text []byte, file string, row int, col int) *Location {
// Equal checks if two locations are equal to each other.
func (loc *Location) Equal(other *Location) bool {
if loc == nil || other == nil {
return loc == other
}
return loc.File == other.File &&
loc.Row == other.Row &&
loc.Col == other.Col &&
v1/ast/marshal_test.go
+6 -5
View File
@@ -5,6 +5,7 @@ import (
"strings"
"testing"
"github.com/google/go-cmp/cmp"
astJSON "github.com/open-policy-agent/opa/v1/ast/json"
"github.com/open-policy-agent/opa/v1/util"
)
@@ -1193,17 +1194,17 @@ func TestNot_MarshalJSON(t *testing.T) {
"implicit body: location included": {
Not: module.Rules[0].Body[0].Terms.(*Not),
Options: astJSON.Options{MarshalOptions: astJSON.MarshalOptions{IncludeLocation: astJSON.NodeToggle{Not: true}}},
ExpectedJSON: `{"body":[{"index":0,"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]},{"type":"number","value":2}]},{"type":"number","value":42}]}],"explicit_body":false,"location":{"file":"example.rego","row":8,"col":3},"type":"not"}`,
ExpectedJSON: `{"body":[{"index":0,"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]},{"type":"number","value":2}]},{"type":"number","value":42}]}],"explicit_body":false,"location":{"file":"example.rego","row":7,"col":4},"type":"not"}`,
},
"explicit body: location included": {
Not: module.Rules[1].Body[0].Terms.(*Not),
Options: astJSON.Options{MarshalOptions: astJSON.MarshalOptions{IncludeLocation: astJSON.NodeToggle{Not: true}}},
ExpectedJSON: `{"body":[{"index":0,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"x"},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]}]},{"index":1,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"y"},{"type":"number","value":2}]},{"index":2,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"z"},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"var","value":"x"},{"type":"var","value":"y"}]}]},{"index":3,"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"var","value":"z"},{"type":"number","value":42}]}],"explicit_body":true,"location":{"file":"example.rego","row":11,"col":8},"type":"not"}`,
ExpectedJSON: `{"body":[{"index":0,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"x"},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]}]},{"index":1,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"y"},{"type":"number","value":2}]},{"index":2,"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"z"},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"var","value":"x"},{"type":"var","value":"y"}]}]},{"index":3,"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"var","value":"z"},{"type":"number","value":42}]}],"explicit_body":true,"location":{"file":"example.rego","row":11,"col":4},"type":"not"}`,
},
"explicit body: location included, also for nested expressions": {
Not: module.Rules[1].Body[0].Terms.(*Not),
Options: astJSON.Options{MarshalOptions: astJSON.MarshalOptions{IncludeLocation: astJSON.NodeToggle{Not: true, Expr: true}}},
ExpectedJSON: `{"body":[{"index":0,"location":{"file":"example.rego","row":12,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"x"},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]}]},{"index":1,"location":{"file":"example.rego","row":13,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"y"},{"type":"number","value":2}]},{"index":2,"location":{"file":"example.rego","row":14,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"z"},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"var","value":"x"},{"type":"var","value":"y"}]}]},{"index":3,"location":{"file":"example.rego","row":15,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"var","value":"z"},{"type":"number","value":42}]}],"explicit_body":true,"location":{"file":"example.rego","row":11,"col":8},"type":"not"}`,
ExpectedJSON: `{"body":[{"index":0,"location":{"file":"example.rego","row":12,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"x"},{"type":"ref","value":[{"type":"var","value":"input"},{"type":"string","value":"x"}]}]},{"index":1,"location":{"file":"example.rego","row":13,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"y"},{"type":"number","value":2}]},{"index":2,"location":{"file":"example.rego","row":14,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"assign"}]},{"type":"var","value":"z"},{"type":"call","value":[{"type":"ref","value":[{"type":"var","value":"plus"}]},{"type":"var","value":"x"},{"type":"var","value":"y"}]}]},{"index":3,"location":{"file":"example.rego","row":15,"col":5},"terms":[{"type":"ref","value":[{"type":"var","value":"equal"}]},{"type":"var","value":"z"},{"type":"number","value":42}]}],"explicit_body":true,"location":{"file":"example.rego","row":11,"col":4},"type":"not"}`,
},
}
@@ -1216,8 +1217,8 @@ func TestNot_MarshalJSON(t *testing.T) {
got := string(bs)
exp := data.ExpectedJSON
if got != exp {
t.Fatalf("expected:\n%s got\n%s", exp, got)
if diff := cmp.Diff(exp, got); diff != "" {
t.Errorf("unexpected json: (-want, +got):\n%s", diff)
}
})
}
v1/ast/parser.go
+77 -45
View File
@@ -1221,6 +1221,13 @@ func (p *Parser) parseLiteral() (expr *Expr) {
if expr != nil {
loc.Text = p.s.Text(offset, p.s.lastEnd)
expr.SetLoc(loc)
// For implicit not-body wrapping (future.keywords.not), propagate
// the outer `not <op>` span to the inner expression.
if not, ok := expr.Terms.(*Not); ok && !not.ExplicitBody {
for _, inner := range not.Body {
inner.SetLoc(loc)
}
}
}
}()
@@ -1260,14 +1267,19 @@ func (p *Parser) parseLiteral() (expr *Expr) {
if tok == tokens.Dot || tok == tokens.LBrack {
p.s.tok = tokens.Ident
return p.parseLiteralExpr(false)
return p.parseLiteralExpr(false, nil)
}
}
negated := isNegatedExpression(p)
var notLoc *Location
negated := isNegated(p)
if negated {
notLoc = p.s.Loc()
p.scan()
}
if negated && p.notBodies && p.s.tok == tokens.LBrace {
nb := p.parseNotBody()
nb := p.parseNotBody(notLoc)
if nb != nil && p.s.tok == tokens.With {
if nb.With = p.parseWith(); nb.With == nil {
@@ -1292,7 +1304,7 @@ func (p *Parser) parseLiteral() (expr *Expr) {
}
return p.parseEvery()
default:
return p.parseLiteralExpr(negated)
return p.parseLiteralExpr(negated, notLoc)
}
}
@@ -1308,7 +1320,7 @@ func (p *Parser) isAllowedRefKeywordStr(s string) bool {
return false
}
func (p *Parser) parseLiteralExpr(negated bool) *Expr {
func (p *Parser) parseLiteralExpr(negated bool, notLoc *Location) *Expr {
startOffset := p.s.loc.Offset
startLoc := p.s.Loc()
s := p.save()
@@ -1343,7 +1355,14 @@ func (p *Parser) parseLiteralExpr(negated bool) *Expr {
// Move 'with' statement to outer not expr
w := expr.With
expr.With = nil
expr = NewExpr(&Not{Body: NewBody(expr), Location: p.s.Loc()})
var spanned *Location
if notLoc != nil {
// Extend the location to also include the 'not ' prefix
spanned = p.extendLoc(notLoc)
}
expr = NewExpr(&Not{Body: NewBody(expr), Location: spanned}).SetLocation(spanned)
expr.With = w
} else {
expr.Negated = negated
@@ -1506,18 +1525,19 @@ func (p *Parser) parseSome() *Expr {
return NewExpr(decl).SetLocation(decl.Location)
}
func (p *Parser) parseNotBody() *Expr {
loc := p.s.Loc()
p.scan()
func (p *Parser) parseNotBody(notLoc *Location) *Expr {
p.scan() // consume `{`
body := p.parseBody(tokens.RBrace)
if body == nil {
return nil
}
p.scan()
p.scan() // consume `}`
not := &Not{Body: body, ExplicitBody: true, Location: loc}
return NewExpr(not).SetLocation(loc)
// Extend the location to also include the 'not ' prefix
spanned := p.extendLoc(notLoc)
not := &Not{Body: body, ExplicitBody: true, Location: spanned}
return NewExpr(not).SetLocation(spanned)
}
// logicalKeywordsActive reports whether the scanner currently treats `and` or
@@ -1549,19 +1569,19 @@ func (p *Parser) parseLogicalOrChain(lhsBody Body, lhsExplicit bool, lhsLoc *Loc
}
lhsBody = NewBody(andExpr)
lhsExplicit = false
lhsLoc = andExpr.Location
}
for p.s.tok == tokens.LogicalOr {
p.scan()
rhsBody, rhsExplicit := p.parseLogicalOperand()
rhsBody, rhsExplicit, rhsLoc := p.parseLogicalOperand()
if rhsBody == nil {
return nil
}
// RHS may extend into a higher-precedence `and`-chain.
if p.s.tok == tokens.LogicalAnd {
rhsLoc := rhsBody[0].Location
andExpr := p.parseLogicalAndChain(rhsBody, rhsExplicit, rhsLoc)
if andExpr == nil {
return nil
@@ -1570,16 +1590,18 @@ func (p *Parser) parseLogicalOrChain(lhsBody Body, lhsExplicit bool, lhsLoc *Loc
rhsExplicit = false
}
exprLoc := p.extendLoc(lhsLoc)
node := &LogicalOr{
Lhs: lhsBody,
Rhs: rhsBody,
ExplicitLhs: lhsExplicit,
ExplicitRhs: rhsExplicit,
Location: lhsLoc,
Location: exprLoc,
}
wrapper := NewExpr(node).SetLocation(lhsLoc)
wrapper := NewExpr(node).SetLocation(exprLoc)
lhsBody = NewBody(wrapper)
lhsExplicit = false
lhsLoc = exprLoc
}
return lhsBody[0]
@@ -1600,72 +1622,81 @@ func (p *Parser) parseLogicalAndChain(lhsBody Body, lhsExplicit bool, lhsLoc *Lo
for p.s.tok == tokens.LogicalAnd {
p.scan()
rhsBody, rhsExplicit := p.parseLogicalOperand()
rhsBody, rhsExplicit, _ := p.parseLogicalOperand()
if rhsBody == nil {
return nil
}
exprLoc := p.extendLoc(lhsLoc)
node := &LogicalAnd{
Lhs: lhsBody,
Rhs: rhsBody,
ExplicitLhs: lhsExplicit,
ExplicitRhs: rhsExplicit,
Location: lhsLoc,
Location: exprLoc,
}
wrapper := NewExpr(node).SetLocation(lhsLoc)
wrapper := NewExpr(node).SetLocation(exprLoc)
lhsBody = NewBody(wrapper)
lhsExplicit = false
lhsLoc = exprLoc
}
return lhsBody[0]
}
func isNegatedExpression(p *Parser) bool {
if p.s.tok == tokens.Not {
// Distinguish the `not` keyword from a ref like `not.x`.
s := p.save()
p.scanWS()
tok := p.s.tok
p.restore(s)
if tok != tokens.Dot && tok != tokens.LBrack {
p.scan()
return true
}
// extendLoc returns a copy of start with Text re-spanned from start.Offset
// to the scanner's current lastEnd.
func (p *Parser) extendLoc(start *Location) *Location {
cpy := *start
cpy.Text = p.s.Text(start.Offset, p.s.lastEnd)
return &cpy
}
func isNegated(p *Parser) bool {
if p.s.tok != tokens.Not {
return false
}
return false
// Distinguish the `not` keyword from a ref like `not.x`.
s := p.save()
p.scanWS()
tok := p.s.tok
p.restore(s)
return tok != tokens.Dot && tok != tokens.LBrack
}
// parseLogicalOperand parses a single operand of an `and`/`or` expression.
// Returns the operand body and whether it was parsed from an explicit `{...}`
// body. Returns (nil, false) on parse error.
func (p *Parser) parseLogicalOperand() (Body, bool) {
func (p *Parser) parseLogicalOperand() (Body, bool, *Location) {
if p.s.tok == tokens.LBrace {
loc := p.s.Loc()
p.scan()
body := p.parseBody(tokens.RBrace)
if body == nil {
return nil, false
return nil, false, nil
}
p.scan()
_ = loc
return body, true
return body, true, loc
}
negated := isNegatedExpression(p)
var notLoc *Location
negated := isNegated(p)
if negated {
notLoc = p.s.Loc()
p.scan()
}
if negated && p.notBodies && p.s.tok == tokens.LBrace {
nb := p.parseNotBody()
nb := p.parseNotBody(notLoc)
if nb == nil {
return nil, false
return nil, false, nil
}
return NewBody(nb), false
return NewBody(nb), false, nb.Location
}
startOffset := p.s.loc.Offset
startLoc := p.s.Loc()
expr := p.parseExpr()
if expr == nil {
return nil, false
return nil, false, nil
}
if expr.Location == nil {
@@ -1675,13 +1706,14 @@ func (p *Parser) parseLogicalOperand() (Body, bool) {
if negated && p.notBodies {
// Don't attach any existing 'with' statements, they belong to the and/or, not the negated expression.
notNode := &Not{Body: NewBody(expr), Location: expr.Location}
expr = NewExpr(notNode).SetLocation(expr.Location)
spanned := p.extendLoc(notLoc)
notNode := &Not{Body: NewBody(expr), Location: spanned}
expr = NewExpr(notNode).SetLocation(spanned)
} else if negated {
expr.Negated = true
}
return NewBody(expr), false
return NewBody(expr), false, expr.Location
}
func (p *Parser) parseEvery() *Expr {
v1/ast/parser_logical_test.go
+268
View File
@@ -1,6 +1,7 @@
package ast
import (
"bytes"
"strings"
"testing"
)
@@ -601,3 +602,270 @@ func TestParseLogical_RoundTripString(t *testing.T) {
})
}
}
func TestParseLogical_ChainLocations(t *testing.T) {
// `not` enabled so the `not { … }` explicit not-body RHS case parses.
opts := logicalParserOpts("not")
type chainLoc struct {
col int
text string
}
tests := []struct {
note string
input string
chains []chainLoc // expected (Col, Text) on every chain wrapper, in walk order
}{
{
note: "simple and",
input: "x and y",
chains: []chainLoc{{col: 1, text: "x and y"}},
},
{
note: "simple or",
input: "x or y",
chains: []chainLoc{{col: 1, text: "x or y"}},
},
{
note: "and-chain left-associative",
input: "x and y and z",
chains: []chainLoc{
{col: 1, text: "x and y and z"},
{col: 1, text: "x and y"},
},
},
{
note: "or-chain left-associative",
input: "x or y or z",
chains: []chainLoc{
{col: 1, text: "x or y or z"},
{col: 1, text: "x or y"},
},
},
{
note: "mixed precedence",
input: "x or y and z",
chains: []chainLoc{
{col: 1, text: "x or y and z"},
{col: 6, text: "y and z"},
},
},
{
note: "mixed precedence — and tighter than or",
input: "x and y or z",
chains: []chainLoc{
{col: 1, text: "x and y or z"},
{col: 1, text: "x and y"},
},
},
{
note: "mixed precedence — and-chains on both sides of or",
input: "x and y or z and w",
chains: []chainLoc{
{col: 1, text: "x and y or z and w"},
{col: 1, text: "x and y"},
{col: 12, text: "z and w"},
},
},
{
note: "LHS explicit body, and",
input: "{ x; y } and z",
chains: []chainLoc{{col: 1, text: "{ x; y } and z"}},
},
{
note: "LHS explicit body, or",
input: "{ x; y } or z",
chains: []chainLoc{{col: 1, text: "{ x; y } or z"}},
},
{
note: "RHS explicit body, and",
input: "x and { y; z }",
chains: []chainLoc{{col: 1, text: "x and { y; z }"}},
},
{
note: "RHS explicit body extending into and-chain — anchors at `{`",
input: "a or { x; y } and z",
chains: []chainLoc{
{col: 1, text: "a or { x; y } and z"},
{col: 6, text: "{ x; y } and z"},
},
},
{
// Branch coverage for the `negated && notBodies && LBrace` arm
// of parseLogicalOperand. The constructed *Not's Location
// anchors at the `not` keyword and spans the full `not { ... }`
// text, so the surrounding chain inherits that span.
note: "RHS negated explicit not-body extending into and-chain",
input: "a or not { x; y } and z",
chains: []chainLoc{
{col: 1, text: "a or not { x; y } and z"},
{col: 6, text: "not { x; y } and z"},
},
},
}
for _, tc := range tests {
t.Run(tc.note, func(t *testing.T) {
body, err := ParseBodyWithOpts(tc.input, opts)
if err != nil {
t.Fatalf("parse error: %v", err)
}
if len(body) != 1 {
t.Fatalf("expected 1 expr in body, got %d", len(body))
}
var got []chainLoc
collectChainLocs(t, body[0], func(col int, text string) {
got = append(got, chainLoc{col: col, text: text})
})
if len(got) != len(tc.chains) {
t.Fatalf("expected %d chain wrappers, got %d: %+v", len(tc.chains), len(got), got)
}
for i, want := range tc.chains {
if got[i].col != want.col {
t.Errorf("chain[%d] Col = %d, want %d", i, got[i].col, want.col)
}
if got[i].text != want.text {
t.Errorf("chain[%d] Text = %q, want %q", i, got[i].text, want.text)
}
}
})
}
}
func collectChainLocs(t *testing.T, e *Expr, emit func(col int, text string)) {
t.Helper()
if e == nil {
return
}
var nodeLoc *Location
var lhs, rhs Body
switch terms := e.Terms.(type) {
case *LogicalAnd:
nodeLoc = terms.Location
lhs, rhs = terms.Lhs, terms.Rhs
case *LogicalOr:
nodeLoc = terms.Location
lhs, rhs = terms.Lhs, terms.Rhs
default:
return
}
if e.Location == nil {
t.Fatalf("chain wrapper Expr.Location is nil")
}
if nodeLoc == nil {
t.Fatalf("chain node.Location is nil")
}
if e.Location.Col != nodeLoc.Col || e.Location.Row != nodeLoc.Row || !bytes.Equal(e.Location.Text, nodeLoc.Text) {
t.Errorf("chain wrapper Expr.Location %+v != node.Location %+v", e.Location, nodeLoc)
}
emit(e.Location.Col, string(e.Location.Text))
for _, x := range lhs {
collectChainLocs(t, x, emit)
}
for _, x := range rhs {
collectChainLocs(t, x, emit)
}
}
func TestParseLogical_InnerExprHasLocation(t *testing.T) {
module := `package test
import future.keywords.and
import future.keywords.or
p if {
f(1) and f(2)
f(3) or f(4)
}
`
popts := ParserOptions{
Capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
}
mod, err := ParseModuleWithOpts("test.rego", module, popts)
if err != nil {
t.Fatalf("unexpected parse error: %v", err)
}
outer := mod.Rules[0].Body[0]
if outer.Location == nil {
t.Fatalf("outer and Expr has nil Location")
}
and := outer.Terms.(*LogicalAnd)
if and.Location == nil {
t.Fatalf("LogicalAnd.Location is nil")
}
inner := and.Lhs[0]
if inner.Location == nil {
t.Fatalf("inner Expr inside LogicalAnd.Lhs has nil Location")
}
if inner.Location.Col != 4 {
t.Errorf("Expected column to be 4 but got: %v", inner.Location.Col)
}
if inner.Location.Row != 6 {
t.Errorf("Expected row to be 6 but got: %v", inner.Location.Row)
}
if inner.Location.File != "test.rego" {
t.Errorf("Expected file to be test.rego but got: %v", inner.Location.File)
}
inner = and.Rhs[0]
if inner.Location == nil {
t.Fatalf("inner Expr inside LogicalAnd.Rhs has nil Location")
}
if inner.Location.Col != 13 {
t.Errorf("Expected column to be 4 but got: %v", inner.Location.Col)
}
if inner.Location.Row != 6 {
t.Errorf("Expected row to be 6 but got: %v", inner.Location.Row)
}
if inner.Location.File != "test.rego" {
t.Errorf("Expected file to be test.rego but got: %v", inner.Location.File)
}
outer = mod.Rules[0].Body[1]
if outer.Location == nil {
t.Fatalf("outer or Expr has nil Location")
}
or := outer.Terms.(*LogicalOr)
if and.Location == nil {
t.Fatalf("LogicalOr.Location is nil")
}
inner = or.Lhs[0]
if inner.Location == nil {
t.Fatalf("inner Expr inside LogicalOr.Lhs has nil Location")
}
if inner.Location.Col != 4 {
t.Errorf("Expected column to be 4 but got: %v", inner.Location.Col)
}
if inner.Location.Row != 7 {
t.Errorf("Expected row to be 7 but got: %v", inner.Location.Row)
}
if inner.Location.File != "test.rego" {
t.Errorf("Expected file to be test.rego but got: %v", inner.Location.File)
}
inner = or.Rhs[0]
if inner.Location == nil {
t.Fatalf("inner Expr inside LogicalOr.Lhs has nil Location")
}
if inner.Location.Col != 12 {
t.Errorf("Expected column to be 12 but got: %v", inner.Location.Col)
}
if inner.Location.Row != 7 {
t.Errorf("Expected row to be 7 but got: %v", inner.Location.Row)
}
if inner.Location.File != "test.rego" {
t.Errorf("Expected file to be test.rego but got: %v", inner.Location.File)
}
}
v1/ast/parser_test.go
+40
View File
@@ -9458,3 +9458,43 @@ func TestNotImport(t *testing.T) {
})
}
}
// TestParseNotBody_InnerExprHasLocation regression test for: GH#8717
func TestParseNotBody_InnerExprHasLocation(t *testing.T) {
module := `package test
import future.keywords.not
p if {
not f(1)
}
`
mod, err := ParseModule("test.rego", module)
if err != nil {
t.Fatalf("unexpected parse error: %v", err)
}
outer := mod.Rules[0].Body[0]
if outer.Location == nil {
t.Fatalf("outer Expr has nil Location")
}
not := outer.Terms.(*Not)
if not.Location == nil {
t.Fatalf("Not.Location is nil")
}
inner := not.Body[0]
if inner.Location == nil {
t.Fatalf("inner Expr inside Not.Body has nil Location")
}
if inner.Location.Col != 4 {
t.Errorf("Expected column to be 4 but got: %v", inner.Location.Col)
}
if inner.Location.Row != 5 {
t.Errorf("Expected row to be 5 but got: %v", inner.Location.Row)
}
if inner.Location.File != "test.rego" {
t.Errorf("Expected file to be test.rego but got: %v", inner.Location.File)
}
}