From 98c300c0770f9ee7263111c1a892da87ca466996 Mon Sep 17 00:00:00 2001 From: Ashutosh Narkar Date: Tue, 24 Oct 2023 14:05:09 -0700 Subject: [PATCH] Rename future.compat import This change renames the `future.compat` import to `rego.v1`. The latter is clear that it's a declaration that a module is compatible with a v1 version of OPA. Signed-off-by: Ashutosh Narkar --- ast/compile.go | 14 ++-- ast/compile_test.go | 67 ++++++++------- ast/internal/scanner/scanner.go | 10 +-- ast/parser.go | 73 +++++++++++------ ast/parser_ext.go | 6 +- ast/parser_test.go | 82 +++++++++++-------- ast/policy.go | 6 +- docs/content/policy-language.md | 11 +++ format/format.go | 10 ++- ...t_future_compat.rego => test_rego_v1.rego} | 2 +- ....formatted => test_rego_v1.rego.formatted} | 2 +- internal/future/filter_imports.go | 8 -- 12 files changed, 172 insertions(+), 119 deletions(-) rename format/testfiles/{test_future_compat.rego => test_rego_v1.rego} (89%) rename format/testfiles/{test_future_compat.rego.formatted => test_rego_v1.rego.formatted} (89%) diff --git a/ast/compile.go b/ast/compile.go index 9d423fbca9..7e46fc287b 100644 --- a/ast/compile.go +++ b/ast/compile.go @@ -1457,7 +1457,7 @@ func (c *Compiler) checkUnsafeBuiltins() { func (c *Compiler) checkDeprecatedBuiltins() { for _, name := range c.sorted { mod := c.Modules[name] - errs := checkDeprecatedBuiltins(c.deprecatedBuiltinsMap, mod, c.strict || mod.futureCompatible) + errs := checkDeprecatedBuiltins(c.deprecatedBuiltinsMap, mod, c.strict || mod.regoV1Compatible) for _, err := range errs { c.err(err) } @@ -1599,7 +1599,7 @@ func (c *Compiler) GetAnnotationSet() *AnnotationSet { func (c *Compiler) checkDuplicateImports() { for _, name := range c.sorted { mod := c.Modules[name] - if !c.strict && !mod.futureCompatible { + if !c.strict && !mod.regoV1Compatible { continue } @@ -1620,7 +1620,7 @@ func (c *Compiler) checkDuplicateImports() { func (c *Compiler) checkKeywordOverrides() { for _, name := range c.sorted { mod := c.Modules[name] - errs := checkKeywordOverrides(mod, c.strict || mod.futureCompatible) + errs := checkKeywordOverrides(mod, c.strict || mod.regoV1Compatible) for _, err := range errs { c.err(err) } @@ -1698,8 +1698,8 @@ func (c *Compiler) resolveAllRefs() { if c.strict { // check for unused imports for _, imp := range mod.Imports { path := imp.Path.Value.(Ref) - if FutureRootDocument.Equal(path[0]) { - continue // ignore future imports + if FutureRootDocument.Equal(path[0]) || RegoRootDocument.Equal(path[0]) { + continue // ignore future and rego imports } for v, u := range globals { @@ -4004,8 +4004,8 @@ func getGlobals(pkg *Package, rules []Ref, imports []*Import) map[Var]*usedRef { // Populate globals with imports. for _, imp := range imports { path := imp.Path.Value.(Ref) - if FutureRootDocument.Equal(path[0]) { - continue // ignore future imports + if FutureRootDocument.Equal(path[0]) || RegoRootDocument.Equal(path[0]) { + continue // ignore future and rego imports } globals[imp.Name()] = &usedRef{ref: path} } diff --git a/ast/compile_test.go b/ast/compile_test.go index 3fa5df12f8..c072d9e766 100644 --- a/ast/compile_test.go +++ b/ast/compile_test.go @@ -2683,6 +2683,13 @@ func TestCompilerCheckUnusedImports(t *testing.T) { }, }, }, + { + note: "ignore unused rego import", + module: `package p + import rego.v1 + r if { 10 == 10 } + `, + }, { note: "import used in comparison", module: `package p @@ -3138,7 +3145,7 @@ func assertErrors(t *testing.T, actual Errors, expected Errors, assertLocation b } } -func TestCompileFutureCompatImport(t *testing.T) { +func TestCompileRegoV1Import(t *testing.T) { cases := []struct { note string modules map[string]string @@ -3149,10 +3156,10 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "duplicate imports", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 import data.foo import data.bar.foo - p if { + p if { foo == "bar" }`, }, @@ -3167,7 +3174,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "duplicate imports (alias)", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 import data.foo import data.bar as foo p if { @@ -3185,7 +3192,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "duplicate imports (alias, different order)", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 import data.bar as foo import data.foo p if { @@ -3203,7 +3210,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "duplicate imports (repeat)", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 import data.foo import data.foo p if { @@ -3221,14 +3228,14 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "duplicate imports (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 import data.foo import data.bar.foo p if { foo == "bar" }`, "policy2.rego": `package test - import future.compat + import rego.v1 import data.foo import data.bar.foo q if { @@ -3257,7 +3264,7 @@ func TestCompileFutureCompatImport(t *testing.T) { foo == "bar" }`, "policy2.rego": `package test - import future.compat + import rego.v1 import data.foo import data.bar.foo q if { @@ -3276,7 +3283,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "var shadows input", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 p if { input := 1 input == 1 @@ -3293,13 +3300,13 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "var shadows input (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 p if { input := 1 input == 1 }`, "policy2.rego": `package test - import future.compat + import rego.v1 q if { input := 1 input == 1 @@ -3326,7 +3333,7 @@ func TestCompileFutureCompatImport(t *testing.T) { input == 1 }`, "policy2.rego": `package test - import future.compat + import rego.v1 q if { input := 1 input == 1 @@ -3343,7 +3350,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "var shadows data", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 p if { data := 1 data == 1 @@ -3360,13 +3367,13 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "var shadows data (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 p if { data := 1 data == 1 }`, "policy2.rego": `package test - import future.compat + import rego.v1 q if { data := 1 data == 1 @@ -3393,7 +3400,7 @@ func TestCompileFutureCompatImport(t *testing.T) { data == 1 }`, "policy2.rego": `package test - import future.compat + import rego.v1 q if { data := 1 data == 1 @@ -3411,7 +3418,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "rule shadows input", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 input := 1`, }, expectedErrors: Errors{ @@ -3425,10 +3432,10 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "rule shadows input (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 input := 1`, "policy2.rego": `package test2 - import future.compat + import rego.v1 input := 2`, }, expectedErrors: Errors{ @@ -3448,7 +3455,7 @@ func TestCompileFutureCompatImport(t *testing.T) { "policy1.rego": `package test input := 1`, "policy2.rego": `package test2 - import future.compat + import rego.v1 input := 2`, }, expectedErrors: Errors{ @@ -3462,7 +3469,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "rule shadows data", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 data := 1`, }, expectedErrors: Errors{ @@ -3476,10 +3483,10 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "rule shadows data (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 data := 1`, "policy2.rego": `package test2 - import future.compat + import rego.v1 data := 2`, }, expectedErrors: Errors{ @@ -3499,7 +3506,7 @@ func TestCompileFutureCompatImport(t *testing.T) { "policy1.rego": `package test data := 1`, "policy2.rego": `package test2 - import future.compat + import rego.v1 data := 2`, }, expectedErrors: Errors{ @@ -3514,7 +3521,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "deprecated built-in", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 p := all([true, false])`, }, expectedErrors: Errors{ @@ -3528,7 +3535,7 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "deprecated built-in (multiple)", modules: map[string]string{ "policy.rego": `package test - import future.compat + import rego.v1 p := all([true, false]) q := any([true, false])`, }, @@ -3547,10 +3554,10 @@ func TestCompileFutureCompatImport(t *testing.T) { note: "deprecated built-in (multiple modules)", modules: map[string]string{ "policy1.rego": `package test - import future.compat + import rego.v1 p := all([true, false])`, "policy2.rego": `package test - import future.compat + import rego.v1 q := all([true, false])`, }, expectedErrors: Errors{ @@ -3570,7 +3577,7 @@ func TestCompileFutureCompatImport(t *testing.T) { "policy1.rego": `package test p := all([true, false])`, "policy2.rego": `package test - import future.compat + import rego.v1 q := all([true, false])`, }, expectedErrors: Errors{ diff --git a/ast/internal/scanner/scanner.go b/ast/internal/scanner/scanner.go index 4a679f25d1..97ee8bde01 100644 --- a/ast/internal/scanner/scanner.go +++ b/ast/internal/scanner/scanner.go @@ -26,7 +26,7 @@ type Scanner struct { width int errors []Error keywords map[string]tokens.Token - futureCompatible bool + regoV1Compatible bool } // Error represents a scanner error. @@ -112,12 +112,12 @@ func (s *Scanner) HasKeyword(keywords map[string]tokens.Token) bool { return false } -func (s *Scanner) SetFutureCompatible() { - s.futureCompatible = true +func (s *Scanner) SetRegoV1Compatible() { + s.regoV1Compatible = true } -func (s *Scanner) FutureCompatible() bool { - return s.futureCompatible +func (s *Scanner) RegoV1Compatible() bool { + return s.regoV1Compatible } // WithKeywords returns a new copy of the Scanner struct `s`, with the set diff --git a/ast/parser.go b/ast/parser.go index 643092c030..c24bbb505a 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -25,7 +25,7 @@ import ( "github.com/open-policy-agent/opa/ast/location" ) -var futureCompatibleRef = Ref{VarTerm("future"), StringTerm("compat")} +var regoV1CompatibleRef = Ref{VarTerm("rego"), StringTerm("v1")} // Note: This state is kept isolated from the parser so that we // can do efficient shallow copies of these values when doing a @@ -330,9 +330,14 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { s = p.save() if imp := p.parseImport(); imp != nil { + if RegoRootDocument.Equal(imp.Path.Value.(Ref)[0]) { + p.regoV1Import(imp) + } + if FutureRootDocument.Equal(imp.Path.Value.(Ref)[0]) { p.futureImport(imp, allowedFutureKeywords) } + stmts = append(stmts, imp) continue } else if len(p.s.errors) > 0 { @@ -535,9 +540,9 @@ func (p *Parser) parseImport() *Import { path := imp.Path.Value.(Ref) - if !RootDocumentNames.Contains(path[0]) && !FutureRootDocument.Equal(path[0]) { + if !RootDocumentNames.Contains(path[0]) && !FutureRootDocument.Equal(path[0]) && !RegoRootDocument.Equal(path[0]) { p.errorf(imp.Path.Location, "unexpected import path, must begin with one of: %v, got: %v", - RootDocumentNames.Union(NewSet(FutureRootDocument)), + RootDocumentNames.Union(NewSet(FutureRootDocument, RegoRootDocument)), path[0]) return nil } @@ -2510,8 +2515,8 @@ var futureKeywords = map[string]tokens.Token{ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]tokens.Token) { path := imp.Path.Value.(Ref) - if len(path) == 1 || (!path[1].Equal(StringTerm("keywords")) && !path[1].Equal(futureCompatibleRef[1])) { - p.errorf(imp.Path.Location, "invalid import, must be `future.keywords` or `%s`", futureCompatibleRef) + if len(path) == 1 || !path[1].Equal(StringTerm("keywords")) { + p.errorf(imp.Path.Location, "invalid import, must be `future.keywords`") return } @@ -2520,33 +2525,16 @@ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]toke return } + if p.s.s.RegoV1Compatible() { + p.errorf(imp.Path.Location, "the `%s` import implies `future.keywords`, these are therefore mutually exclusive", regoV1CompatibleRef) + return + } + kwds := make([]string, 0, len(allowedFutureKeywords)) for k := range allowedFutureKeywords { kwds = append(kwds, k) } - if path[1].Equal(futureCompatibleRef[1]) { - if len(path) > 2 { - p.errorf(imp.Path.Location, "invalid import, must be `%s`", futureCompatibleRef) - return - } - if p.s.s.HasKeyword(futureKeywords) && !p.s.s.FutureCompatible() { - // We have imported future keywords, but they didn't come from another `future.compat` import. - p.errorf(imp.Path.Location, "the `%s` import implies `future.keywords`, these are therefore mutually exclusive", futureCompatibleRef) - return - } - p.s.s.SetFutureCompatible() - for _, kw := range kwds { - p.s.s.AddKeyword(kw, allowedFutureKeywords[kw]) - } - return - } - - if p.s.s.FutureCompatible() { - p.errorf(imp.Path.Location, "the `%s` import implies `future.keywords`, these are therefore mutually exclusive", futureCompatibleRef) - return - } - switch len(path) { case 2: // all keywords imported, nothing to do case 3: // one keyword imported @@ -2569,3 +2557,34 @@ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]toke p.s.s.AddKeyword(kw, allowedFutureKeywords[kw]) } } + +func (p *Parser) regoV1Import(imp *Import) { + path := imp.Path.Value.(Ref) + + if len(path) == 1 || !path[1].Equal(regoV1CompatibleRef[1]) || len(path) > 2 { + p.errorf(imp.Path.Location, "invalid import, must be `%s`", regoV1CompatibleRef) + return + } + + if imp.Alias != "" { + p.errorf(imp.Path.Location, "`rego` imports cannot be aliased") + return + } + + // import all future keywords with the rego.v1 import + kwds := make([]string, 0, len(futureKeywords)) + for k := range futureKeywords { + kwds = append(kwds, k) + } + + if p.s.s.HasKeyword(futureKeywords) && !p.s.s.RegoV1Compatible() { + // We have imported future keywords, but they didn't come from another `rego.v1` import. + p.errorf(imp.Path.Location, "the `%s` import implies `future.keywords`, these are therefore mutually exclusive", regoV1CompatibleRef) + return + } + + p.s.s.SetRegoV1Compatible() + for _, kw := range kwds { + p.s.s.AddKeyword(kw, futureKeywords[kw]) + } +} diff --git a/ast/parser_ext.go b/ast/parser_ext.go index 56038f84b5..733b2418a4 100644 --- a/ast/parser_ext.go +++ b/ast/parser_ext.go @@ -663,8 +663,8 @@ func parseModule(filename string, stmts []Statement, comments []*Comment) (*Modu switch stmt := stmt.(type) { case *Import: mod.Imports = append(mod.Imports, stmt) - if Compare(stmt.Path.Value, futureCompatibleRef) == 0 { - mod.futureCompatible = true + if Compare(stmt.Path.Value, regoV1CompatibleRef) == 0 { + mod.regoV1Compatible = true } case *Rule: setRuleModule(stmt, mod) @@ -693,7 +693,7 @@ func parseModule(filename string, stmts []Statement, comments []*Comment) (*Modu } } - if mod.futureCompatible { + if mod.regoV1Compatible { for _, rule := range mod.Rules { for r := rule; r != nil; r = r.Else { if r.generatedBody && r.Head.generatedValue { diff --git a/ast/parser_test.go b/ast/parser_test.go index 73335ae2a7..9800a59f1f 100644 --- a/ast/parser_test.go +++ b/ast/parser_test.go @@ -1213,7 +1213,7 @@ func TestImport(t *testing.T) { assertParseImport(t, "white space", "import input.foo.bar[\"white space\"]", &Import{Path: whitespace}) assertParseErrorContains(t, "non-ground ref", "import data.foo[x]", "rego_parse_error: unexpected var token: expecting string") assertParseErrorContains(t, "non-string", "import input.foo[0]", "rego_parse_error: unexpected number token: expecting string") - assertParseErrorContains(t, "unknown root", "import foo.bar", "rego_parse_error: unexpected import path, must begin with one of: {data, future, input}, got: foo") + assertParseErrorContains(t, "unknown root", "import foo.bar", "rego_parse_error: unexpected import path, must begin with one of: {data, future, input, rego}, got: foo") assertParseErrorContains(t, "bad variable term", "import input as A(", "rego_parse_error: unexpected eof token: expected var") _, _, err := ParseStatements("", "package foo\nimport bar.data\ndefault foo=1") @@ -1236,7 +1236,6 @@ func TestFutureImports(t *testing.T) { assertParseErrorContains(t, "unknown keyword", "import future.keywords.xyz", "unexpected keyword, must be one of [contains every if in]") assertParseErrorContains(t, "all keyword import + alias", "import future.keywords as xyz", "`future` imports cannot be aliased") assertParseErrorContains(t, "keyword import + alias", "import future.keywords.in as xyz", "`future` imports cannot be aliased") - assertParseErrorContains(t, "future.compat.abc", "import future.compat.abc", "invalid import, must be `future.compat`") assertParseImport(t, "import kw with kw in options", "import future.keywords.in", &Import{Path: RefTerm(VarTerm("future"), StringTerm("keywords"), StringTerm("in"))}, @@ -1244,9 +1243,6 @@ func TestFutureImports(t *testing.T) { assertParseImport(t, "import kw with all kw in options", "import future.keywords.in", &Import{Path: RefTerm(VarTerm("future"), StringTerm("keywords"), StringTerm("in"))}, ParserOptions{AllFutureKeywords: true}) - assertParseImport(t, "import compat", - "import future.compat", &Import{Path: RefTerm(VarTerm("future"), StringTerm("compat"))}, - ParserOptions{}) mod := ` package p @@ -1265,14 +1261,21 @@ func TestFutureImports(t *testing.T) { mod = ` package p - import future.compat + import rego.v1 import future.keywords.in ` - assertParseModuleErrorMatch(t, "compat and keywords imported", mod, "rego_parse_error: the `future.compat` import implies `future.keywords`, these are therefore mutually exclusive") + assertParseModuleErrorMatch(t, "rego.v1 and future.keywords.in imported", mod, "rego_parse_error: the `rego.v1` import implies `future.keywords`, these are therefore mutually exclusive") + + mod = ` + package p + import future.keywords + import rego.v1 + ` + assertParseModuleErrorMatch(t, "rego.v1 and future.keywords imported", mod, "rego_parse_error: the `rego.v1` import implies `future.keywords`, these are therefore mutually exclusive") } -func TestFutureImportsExtraction(t *testing.T) { - // These tests assert that "import future..." statements in policies cause +func TestFutureAndRegoV1ImportsExtraction(t *testing.T) { + // These tests assert that "import future..." and "import rego.v1" statements in policies cause // the proper keywords to be added to the parser's list of known keywords. tests := []struct { note, imp string @@ -1306,8 +1309,8 @@ func TestFutureImportsExtraction(t *testing.T) { }, }, { - note: "future.compat imported", - imp: "import future.compat", + note: "rego.v1 imported", + imp: "import rego.v1", exp: map[string]tokens.Token{ "in": tokens.In, "every": tokens.Every, @@ -1333,32 +1336,41 @@ func TestFutureImportsExtraction(t *testing.T) { } } -func TestFutureCompatImport(t *testing.T) { +func TestRegoV1Import(t *testing.T) { + assertParseErrorContains(t, "rego", "import rego", "invalid import, must be `rego.v1`") + assertParseErrorContains(t, "rego.foo", "import rego.foo", "invalid import, must be `rego.v1`") + assertParseErrorContains(t, "rego.foo.bar", "import rego.foo.bar", "invalid import, must be `rego.v1`") + assertParseErrorContains(t, "rego.v1 + alias", "import rego.v1 as xyz", "`rego` imports cannot be aliased") + + assertParseImport(t, "import rego.v1", + "import rego.v1", &Import{Path: RefTerm(VarTerm("rego"), StringTerm("v1"))}, + ParserOptions{}) + tests := []struct { note string module string expectedErrors []string }{ { - note: "only future.compat imported", + note: "only rego.v1 imported", module: `package test -import future.compat +import rego.v1 p contains 1 if 1 == 1`, }, { - note: "future.compat and future.keywords imported", + note: "rego.v1 and future.keywords imported", module: `package test -import future.compat +import rego.v1 import future.keywords p contains 1 if { input.x == 1 }`, - expectedErrors: []string{"rego_parse_error: the `future.compat` import implies `future.keywords`, these are therefore mutually exclusive"}, + expectedErrors: []string{"rego_parse_error: the `rego.v1` import implies `future.keywords`, these are therefore mutually exclusive"}, }, { note: "`if` keyword used on rule", module: `package test -import future.compat +import rego.v1 p if { input.x == 1 }`, @@ -1366,7 +1378,7 @@ p if { { note: "`if` keyword not used on rule", module: `package test -import future.compat +import rego.v1 p { input.x == 1 }`, @@ -1375,13 +1387,13 @@ p { { note: "constant definition", module: `package test -import future.compat +import rego.v1 p := 1`, }, { note: "`if` keyword used before else body", module: `package test -import future.compat +import rego.v1 p if { input.x == 1 } else if { @@ -1391,7 +1403,7 @@ p if { { note: "`if` keyword used before else body (value assignment)", module: `package test -import future.compat +import rego.v1 p := "foo" if { input.x == 1 } else := "bar" if { @@ -1402,7 +1414,7 @@ else := "qux"`, { note: "`if` keyword not used before else body", module: `package test -import future.compat +import rego.v1 p if { input.x == 1 } else { @@ -1413,7 +1425,7 @@ p if { { note: "`if` keyword not used before else body (value assignment)", module: `package test -import future.compat +import rego.v1 p := "foo" if { input.x == 1 } else := "bar" { @@ -1424,65 +1436,65 @@ p := "foo" if { { note: "`contains` keyword used on partial set rule (const key)", module: `package test -import future.compat +import rego.v1 p contains "q"`, }, { note: "`contains` keyword used on partial set rule (ref-head, const key)", module: `package test -import future.compat +import rego.v1 p.q contains "r"`, }, { note: "`contains` keyword not used on partial set rule (const key)", module: `package test -import future.compat +import rego.v1 p.q`, expectedErrors: []string{"rego_parse_error: `contains` keyword is required for partial set rules"}, }, { note: "object definition (naked ref-head with implicit `true` value)", module: `package test -import future.compat +import rego.v1 p.q.r`, expectedErrors: []string{"rego_parse_error: rule must have value assignment and/or body declaration"}, }, { note: "`contains` keyword used on partial set rule (var key, no body)", module: `package test -import future.compat +import rego.v1 p contains input.x`, }, { note: "`contains` keyword not used on partial set rule (var key, no body)", module: `package test -import future.compat +import rego.v1 p[input.x]`, expectedErrors: []string{"rego_parse_error: `contains` keyword is required for partial set rules"}, }, { note: "`if` keyword not used on partial object rule (ref-head, var key, implicit `true` value, no body)", module: `package test -import future.compat +import rego.v1 p.q[input.x]`, expectedErrors: []string{"rego_parse_error: rule must have value assignment and/or body declaration"}, }, { note: "`contains` keyword used on partial set rule (var key)", module: `package test -import future.compat +import rego.v1 p contains x if { x = input.x}`, }, { note: "`if` keyword used on partial map rule (would be multi-value without `if`)", module: `package test -import future.compat +import rego.v1 p[x] if { x = input.x}`, }, { note: "`contains` and `if` keyword not used on partial rule", module: `package test -import future.compat +import rego.v1 p[x] { x = input.x}`, // The developer likely intended a partial set. expectedErrors: []string{ @@ -1493,7 +1505,7 @@ p[x] { x = input.x}`, { note: "`if` keyword not used on partial object rule (ref-head)", module: `package test -import future.compat +import rego.v1 p.q[x] { x = input.x}`, expectedErrors: []string{ "rego_parse_error: `if` keyword is required before rule body", diff --git a/ast/policy.go b/ast/policy.go index 933c78a4e2..478ddf655b 100644 --- a/ast/policy.go +++ b/ast/policy.go @@ -44,6 +44,10 @@ var FunctionArgRootDocument = VarTerm("args") // features. var FutureRootDocument = VarTerm("future") +// RegoRootDocument names the document containing new, to-become-default, +// features in a future versioned release. +var RegoRootDocument = VarTerm("rego") + // RootDocumentNames contains the names of top-level documents that can be // referred to in modules and queries. // @@ -147,7 +151,7 @@ type ( Rules []*Rule `json:"rules,omitempty"` Comments []*Comment `json:"comments,omitempty"` stmts []Statement - futureCompatible bool + regoV1Compatible bool } // Comment contains the raw text from the comment in the definition. diff --git a/docs/content/policy-language.md b/docs/content/policy-language.md index be8cce01d4..ef68c92d9e 100644 --- a/docs/content/policy-language.md +++ b/docs/content/policy-language.md @@ -1690,6 +1690,11 @@ become a no-op that can safely be removed. This should give all users ample time update their policies, so that the new keyword will not cause clashes with existing variable names. +{{< info >}} +If the `rego.v1` import is present in a module, then `future.keywords` and `future.keywords.*` import is implied, and not allowed. +{{< /info >}} + + Note that some future keyword imports have consequences on pretty-printing: If `contains` or `if` are imported, the pretty-printer will use them as applicable when formatting the modules. @@ -3802,6 +3807,12 @@ Unused imports | Unused [imports](../policy-language/#imports) are prohibited. `input` and `data` reserved keywords | `input` and `data` are reserved keywords, and may not be used as names for rules and variable assignment. | 1.0 Use of deprecated built-ins | Use of deprecated functions is prohibited, and these will be removed in OPA 1.0. Deprecated built-in functions: `any`, `all`, `re_match`, `net.cidr_overlap`, `set_diff`, `cast_array`, `cast_set`, `cast_string`, `cast_boolean`, `cast_null`, `cast_object` | 1.0 +{{< info >}} +If the `rego.v1` import is present in a module, all strict mode checks documented above expect the unused local assignment and unused imports checks are enforced on the module. + +Additionally the `rego.v1` import also requires the usage of `if` and `contains` keywords when declaring certain rules. The `if` keyword is required before a rule body and the `contains` keyword is required for partial set rules. +{{< /info >}} + ## Ecosystem Projects {{< ecosystem_feature_embed key="learning-rego" topic="learning Rego" >}} diff --git a/format/format.go b/format/format.go index 6016b91b56..38799505f4 100644 --- a/format/format.go +++ b/format/format.go @@ -120,7 +120,7 @@ func AstWithOpts(x interface{}, opts Opts) ([]byte, error) { case *ast.Import: switch { - case future.IsFutureCompatible(n): + case isRegoV1Compatible(n): o.contains = true o.ifs = true case future.IsAllFutureKeywords(n): @@ -1435,3 +1435,11 @@ func (d *ArityFormatErrDetail) Lines() []string { "want: " + "(" + strings.Join(d.Want, ",") + ")", } } + +// isRegoV1Compatible returns true if the passed *ast.Import is `rego.v1` +func isRegoV1Compatible(imp *ast.Import) bool { + path := imp.Path.Value.(ast.Ref) + return len(path) == 2 && + ast.RegoRootDocument.Equal(path[0]) && + path[1].Equal(ast.StringTerm("v1")) +} diff --git a/format/testfiles/test_future_compat.rego b/format/testfiles/test_rego_v1.rego similarity index 89% rename from format/testfiles/test_future_compat.rego rename to format/testfiles/test_rego_v1.rego index 8489ffbe75..9fe8495e1a 100644 --- a/format/testfiles/test_future_compat.rego +++ b/format/testfiles/test_rego_v1.rego @@ -1,6 +1,6 @@ package example -import future.compat +import rego.v1 # R1: constant a := 1 diff --git a/format/testfiles/test_future_compat.rego.formatted b/format/testfiles/test_rego_v1.rego.formatted similarity index 89% rename from format/testfiles/test_future_compat.rego.formatted rename to format/testfiles/test_rego_v1.rego.formatted index 8489ffbe75..9fe8495e1a 100644 --- a/format/testfiles/test_future_compat.rego.formatted +++ b/format/testfiles/test_rego_v1.rego.formatted @@ -1,6 +1,6 @@ package example -import future.compat +import rego.v1 # R1: constant a := 1 diff --git a/internal/future/filter_imports.go b/internal/future/filter_imports.go index 37eff88c72..2863aad4e9 100644 --- a/internal/future/filter_imports.go +++ b/internal/future/filter_imports.go @@ -35,11 +35,3 @@ func IsFutureKeyword(imp *ast.Import, kw string) bool { path[1].Equal(ast.StringTerm("keywords")) && path[2].Equal(ast.StringTerm(kw)) } - -// IsFutureCompatible returns true if the passed *ast.Import is `future.compat` -func IsFutureCompatible(imp *ast.Import) bool { - path := imp.Path.Value.(ast.Ref) - return len(path) == 2 && - ast.FutureRootDocument.Equal(path[0]) && - path[1].Equal(ast.StringTerm("compat")) -}