Skip to content

Commit

Permalink
ast: Making rego.v1 import errors more specific
Browse files Browse the repository at this point in the history 
to whether they concern a rule or function.

Signed-off-by: Johan Fylling <[email protected]>
  • Loading branch information
@johanfylling @ashutosh-narkar
johanfylling authored and ashutosh-narkar committed Nov 6, 2023
1 parent f916659 commit 17aafaa
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 19 deletions.
11 changes: 9 additions & 2 deletions ast/parser_ext.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,11 +696,18 @@ func parseModule(filename string, stmts []Statement, comments []*Comment) (*Modu
if mod.regoV1Compatible {
for _, rule := range mod.Rules {
for r := rule; r != nil; r = r.Else {
var t string
if r.isFunction() {
t = "function"
} else {
t = "rule"
}

if r.generatedBody && r.Head.generatedValue {
errs = append(errs, NewError(ParseErr, r.Location, "rule must have value assignment and/or body declaration"))
errs = append(errs, NewError(ParseErr, r.Location, "%s must have value assignment and/or body declaration", t))
}
if r.Body != nil && !r.generatedBody && !ruleDeclarationHasKeyword(r, tokens.If) && !r.Default {
errs = append(errs, NewError(ParseErr, r.Location, "`if` keyword is required before rule body"))
errs = append(errs, NewError(ParseErr, r.Location, "`if` keyword is required before %s body", t))
}
if r.Head.RuleKind() == MultiValue && !ruleDeclarationHasKeyword(r, tokens.Contains) {
errs = append(errs, NewError(ParseErr, r.Location, "`contains` keyword is required for partial set rules"))
Expand Down
34 changes: 17 additions & 17 deletions ast/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ import rego.v1
f(x) := x {
x == 1
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, no value assignment, body, with if",
Expand All @@ -1575,7 +1575,7 @@ import rego.v1
f(x) {
x == 1
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, else without body, value assignment",
Expand Down Expand Up @@ -1622,7 +1622,7 @@ f(x) := x if {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, else with body and no if, no value assignment",
Expand All @@ -1633,7 +1633,7 @@ f(x) if {
} else {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, else with body and no if, value assignment on primary head",
Expand All @@ -1644,7 +1644,7 @@ f(x) if {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, else with body and no if, value assignment on else",
Expand All @@ -1655,7 +1655,7 @@ f(x) := x if {
} else {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on last else, value assignment",
Expand All @@ -1668,7 +1668,7 @@ f(x) := x if {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on last else, value assignment on primary head",
Expand All @@ -1681,7 +1681,7 @@ f(x) := x if {
} else {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on last else, value assignment on first else",
Expand All @@ -1694,7 +1694,7 @@ f(x) if {
} else {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on last else, value assignment on last else",
Expand All @@ -1707,7 +1707,7 @@ f(x) if {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on first else, value assignment",
Expand All @@ -1720,7 +1720,7 @@ f(x) := x if {
} else := 42 if {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on first else, value assignment on primary head",
Expand All @@ -1733,7 +1733,7 @@ f(x) := x if {
} else if {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on first else, value assignment on first else",
Expand All @@ -1746,7 +1746,7 @@ f(x) if {
} else if {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on first else, value assignment on last else",
Expand All @@ -1759,7 +1759,7 @@ f(x) if {
} else := 42 if {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if on any else, value assignment",
Expand All @@ -1772,7 +1772,7 @@ f(x) := x if {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function, multiple else with body, no if, value assignment",
Expand All @@ -1785,7 +1785,7 @@ f(x) := x {
} else := 42 {
x == 2
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "rule with chained bodies, no `if`",
Expand Down Expand Up @@ -1865,7 +1865,7 @@ f(x) {
} {
x == 3
}`,
expectedErrors: []string{"rego_parse_error: `if` keyword is required before rule body"},
expectedErrors: []string{"rego_parse_error: `if` keyword is required before function body"},
},
{
note: "function with chained bodies, `if` on first body",
Expand Down
4 changes: 4 additions & 0 deletions ast/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,10 @@ func (rule *Rule) String() string {
return strings.Join(buf, " ")
}

func (rule *Rule) isFunction() bool {
return len(rule.Head.Args) > 0
}

func (rule *Rule) setJSONOptions(opts astJSON.Options) {
rule.jsonOptions = opts
if rule.Location != nil {
Expand Down

0 comments on commit 17aafaa

Please sign in to comment.