Merge branch 'main' into general_refs/docs

.github/workflows/nightly.yaml

@@ -2,7 +2,7 @@ name: Nightly
 on:
   workflow_dispatch: {}   # Allow for manual triggers
   schedule:
-    - cron:  '0 8 * * *'  # Daily, at 8:00 UTC
+    - cron:  '0 8 * * 0-4' # Sun-Thu, at 8:00 UTC
 
 
 jobs:

ast/annotations.go

@@ -11,6 +11,7 @@ import (
 	"sort"
 	"strings"
 
+	astJSON "github.com/open-policy-agent/opa/ast/json"
 	"github.com/open-policy-agent/opa/internal/deepcopy"
 	"github.com/open-policy-agent/opa/util"
 )
@@ -39,7 +40,7 @@ type (
 
 		comments    []*Comment
 		node        Node
-		jsonOptions JSONOptions
+		jsonOptions astJSON.Options
 	}
 
 	// SchemaAnnotation contains a schema declaration for the document identified by the path.
@@ -76,7 +77,7 @@ type (
 		Annotations *Annotations `json:"annotations,omitempty"`
 		Location    *Location    `json:"location,omitempty"` // The location of the node the annotations are applied to
 
-		jsonOptions JSONOptions
+		jsonOptions astJSON.Options
 
 		node Node // The node the annotations are applied to
 	}
@@ -180,8 +181,11 @@ func (a *Annotations) GetTargetPath() Ref {
 	}
 }
 
-func (a *Annotations) setJSONOptions(opts JSONOptions) {
+func (a *Annotations) setJSONOptions(opts astJSON.Options) {
 	a.jsonOptions = opts
+	if a.Location != nil {
+		a.Location.JSONOptions = opts
+	}
 }
 
 func (a *Annotations) MarshalJSON() ([]byte, error) {

ast/compile.go

@@ -7,7 +7,6 @@ package ast
 import (
 	"fmt"
 	"io"
-	"os"
 	"sort"
 	"strconv"
 	"strings"
@@ -146,7 +145,6 @@ type Compiler struct {
 	keepModules             bool                          // whether to keep the unprocessed, parse modules (below)
 	parsedModules           map[string]*Module            // parsed, but otherwise unprocessed modules, kept track of when keepModules is true
 	useTypeCheckAnnotations bool                          // whether to provide annotated information (schemas) to the type checker
-	generalRuleRefsEnabled  bool
 }
 
 // CompilerStage defines the interface for stages in the compiler.
@@ -335,8 +333,6 @@ func NewCompiler() *Compiler {
 		{"BuildComprehensionIndices", "compile_stage_rebuild_comprehension_indices", c.buildComprehensionIndices},
 	}
 
-	_, c.generalRuleRefsEnabled = os.LookupEnv("EXPERIMENTAL_GENERAL_RULE_REFS")
-
 	return c
 }
 
@@ -1005,50 +1001,8 @@ func (c *Compiler) checkRuleConflicts() {
 			//   data.p.q[r][s] { r := input.r; s := input.s }
 			//   data.p[q].r.s { q := input.q }
 
-			if c.generalRuleRefsEnabled {
-				if r.Ref().IsGround() && len(node.Children) > 0 {
-					conflicts = node.flattenChildren()
-				}
-			} else { // TODO: Remove when general rule refs are enabled by default.
-				if r.Head.RuleKind() == SingleValue && len(node.Children) > 0 {
-					if len(ref) > 1 && !ref[len(ref)-1].IsGround() { // p.q[x] and p.q.s.t => check grandchildren
-						for _, c := range node.Children {
-							grandchildrenFound := false
-
-							if len(c.Values) > 0 {
-								childRules := extractRules(c.Values)
-								for _, childRule := range childRules {
-									childRef := childRule.Ref()
-									if childRule.Head.RuleKind() == SingleValue && !childRef[len(childRef)-1].IsGround() {
-										// The child is a partial object rule, so it's effectively "generating" grandchildren.
-										grandchildrenFound = true
-										break
-									}
-								}
-							}
-
-							if len(c.Children) > 0 {
-								grandchildrenFound = true
-							}
-
-							if grandchildrenFound {
-								conflicts = node.flattenChildren()
-								break
-							}
-						}
-					} else { // p.q.s and p.q.s.t => any children are in conflict
-						conflicts = node.flattenChildren()
-					}
-				}
-
-				// Multi-value rules may not have any other rules in their extent; e.g.:
-				//
-				// data.p[v] { v := ... }
-				// data.p.q := 42 # In direct conflict with data.p[v], which is constructing a set and cannot have values assigned to a sub-path.
-
-				if r.Head.RuleKind() == MultiValue && len(node.Children) > 0 {
-					conflicts = node.flattenChildren()
-				}
+			if r.Ref().IsGround() && len(node.Children) > 0 {
+				conflicts = node.flattenChildren()
 			}
 
 			if r.Head.RuleKind() == SingleValue && r.Head.Ref().IsGround() {
@@ -1811,18 +1765,6 @@ func (c *Compiler) rewriteRuleHeadRefs() {
 			}
 
 			for i := 1; i < len(ref); i++ {
-				// NOTE: Unless enabled via the EXPERIMENTAL_GENERAL_RULE_REFS env var, non-string values in the refs are forbidden
-				// except for the last position, e.g.
-				//     OK: p.q.r[s]
-				// NOT OK: p[q].r.s
-				// TODO: Remove when general rule refs are enabled by default.
-				if !c.generalRuleRefsEnabled && i != len(ref)-1 { // last
-					if _, ok := ref[i].Value.(String); !ok {
-						c.err(NewError(TypeErr, rule.Loc(), "rule head must only contain string terms (except for last): %v", ref[i]))
-						continue
-					}
-				}
-
 				// Rewrite so that any non-scalar elements that in the last position of
 				// the rule are vars:
 				//     p.q.r[y.z] { ... }  =>  p.q.r[__local0__] { __local0__ = y.z }
@@ -2385,8 +2327,9 @@ func (c *Compiler) rewriteLocalVarsInRule(rule *Rule, unusedArgs VarSet, argsSta
 	// Rewrite assignments in body.
 	used := NewVarSet()
 
-	last := rule.Head.Ref()[len(rule.Head.Ref())-1]
-	used.Update(last.Vars())
+	for _, t := range rule.Head.Ref()[1:] {
+		used.Update(t.Vars())
+	}
 
 	if rule.Head.Key != nil {
 		used.Update(rule.Head.Key.Vars())

ast/compile_test.go

@@ -467,8 +467,6 @@ func toRef(s string) Ref {
 }
 
 func TestCompilerCheckRuleHeadRefs(t *testing.T) {
-	t.Setenv("EXPERIMENTAL_GENERAL_RULE_REFS", "true")
-
 	tests := []struct {
 		note     string
 		modules  []*Module
@@ -607,64 +605,6 @@ func TestCompilerCheckRuleHeadRefs(t *testing.T) {
 	}
 }
 
-// TODO: Remove when general rule refs are enabled by default.
-func TestCompilerCheckRuleHeadRefsWithGeneralRuleRefsDisabled(t *testing.T) {
-
-	tests := []struct {
-		note     string
-		modules  []*Module
-		expected *Rule
-		err      string
-	}{
-		{
-			note: "ref contains var",
-			modules: modules(
-				`package x
-				p.q[i].r = 1 { i := 10 }`,
-			),
-			err: "rego_type_error: rule head must only contain string terms (except for last): i",
-		},
-		{
-			note: "invalid: ref in ref",
-			modules: modules(
-				`package x
-				p.q[arr[0]].r { i := 10 }`,
-			),
-			err: "rego_type_error: rule head must only contain string terms (except for last): arr[0]",
-		},
-		{
-			note: "invalid: non-string in ref (not last position)",
-			modules: modules(
-				`package x
-				p.q[10].r { true }`,
-			),
-			err: "rego_type_error: rule head must only contain string terms (except for last): 10",
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.note, func(t *testing.T) {
-			mods := make(map[string]*Module, len(tc.modules))
-			for i, m := range tc.modules {
-				mods[fmt.Sprint(i)] = m
-			}
-			c := NewCompiler()
-			c.Modules = mods
-			compileStages(c, c.rewriteRuleHeadRefs)
-			if tc.err != "" {
-				assertCompilerErrorStrings(t, c, []string{tc.err})
-			} else {
-				if len(c.Errors) > 0 {
-					t.Fatalf("expected no errors, got %v", c.Errors)
-				}
-				if tc.expected != nil {
-					assertRulesEqual(t, tc.expected, mods["0"].Rules[0])
-				}
-			}
-		})
-	}
-}
-
 func TestRuleTreeWithDotsInHeads(t *testing.T) {
 
 	// TODO(sr): multi-val with var key in ref
@@ -1940,8 +1880,6 @@ func TestCompilerCheckRuleConflictsDefaultFunction(t *testing.T) {
 }
 
 func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
-	t.Setenv("EXPERIMENTAL_GENERAL_RULE_REFS", "true")
-
 	tests := []struct {
 		note    string
 		modules []*Module
@@ -2187,70 +2125,6 @@ func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
 	}
 }
 
-// TODO: Remove when general rule refs are enabled by default.
-func TestGeneralRuleRefsDisabled(t *testing.T) {
-	// EXPERIMENTAL_GENERAL_RULE_REFS env var not set
-
-	tests := []struct {
-		note    string
-		modules []*Module
-		err     string
-	}{
-		{
-			note: "single-value with other rule overlap, unknown key",
-			modules: modules(
-				`package pkg
-				p.q[r] = x { r = input.key; x = input.foo }
-				p.q.r.s = x { true }
-				`),
-			err: "rego_type_error: rule data.pkg.p.q[r] conflicts with [data.pkg.p.q.r.s]",
-		},
-		{
-			note: "single-value with other rule overlap, unknown ref var and key",
-			modules: modules(
-				`package pkg
-				p.q[r][s] = x { r = input.key1; s = input.key2; x = input.foo }
-				p.q.r.s.t = x { true }
-				`),
-			err: "rego_type_error: rule head must only contain string terms (except for last): r",
-		},
-		{
-			note: "single-value partial object with other partial object rule overlap, unknown keys (regression test for #5855; invalidated by multi-var refs)",
-			modules: modules(
-				`package pkg
-				p[r] := x { r = input.key; x = input.bar }
-				p.q[r] := x { r = input.key; x = input.bar }
-				`),
-			err: "rego_type_error: rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
-		},
-		{
-			note: "single-value partial object with other partial object (implicit 'true' value) rule overlap, unknown keys",
-			modules: modules(
-				`package pkg
-				p[r] := x { r = input.key; x = input.bar }
-				p.q[r] { r = input.key }
-				`),
-			err: "rego_type_error: rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
-		},
-	}
-	for _, tc := range tests {
-		t.Run(tc.note, func(t *testing.T) {
-			mods := make(map[string]*Module, len(tc.modules))
-			for i, m := range tc.modules {
-				mods[fmt.Sprint(i)] = m
-			}
-			c := NewCompiler()
-			c.Modules = mods
-			compileStages(c, c.checkRuleConflicts)
-			if tc.err != "" {
-				assertCompilerErrorStrings(t, c, []string{tc.err})
-			} else {
-				assertCompilerErrorStrings(t, c, []string{})
-			}
-		})
-	}
-}
-
 func TestCompilerCheckUndefinedFuncs(t *testing.T) {
 
 	module := `
@@ -7278,6 +7152,29 @@ func TestCompilerCheckUnusedAssignedVar(t *testing.T) {
 				&Error{Message: "assigned var y unused"},
 			},
 		},
+		{
+			note: "general ref in rule head",
+			module: `package test
+						p[q].r[s] := 1 {
+							q := "foo"
+							s := "bar"
+							t := "baz"
+						}
+		`,
+			expectedErrors: Errors{
+				&Error{Message: "assigned var t unused"},
+			},
+		},
+		{
+			note: "general ref in rule head (no errors)",
+			module: `package test
+						p[q].r[s] := 1 {
+							q := "foo"
+							s := "bar"
+						}
+		`,
+			expectedErrors: Errors{},
+		},
 	}
 
 	makeTestRunner := func(tc testCase, strict bool) func(t *testing.T) {

ast/json/json.go

@@ -0,0 +1,33 @@
+package json
+
+// Options defines the options for JSON operations,
+// currently only marshaling can be configured
+type Options struct {
+	MarshalOptions MarshalOptions
+}
+
+// MarshalOptions defines the options for JSON marshaling,
+// currently only toggling the marshaling of location information is supported
+type MarshalOptions struct {
+	// IncludeLocation toggles the marshaling of location information
+	IncludeLocation NodeToggle
+	// IncludeLocationText additionally/optionally includes the text of the location
+	IncludeLocationText bool
+}
+
+// NodeToggle is a generic struct to allow the toggling of
+// settings for different ast node types
+type NodeToggle struct {
+	Term           bool
+	Package        bool
+	Comment        bool
+	Import         bool
+	Rule           bool
+	Head           bool
+	Expr           bool
+	SomeDecl       bool
+	Every          bool
+	With           bool
+	Annotations    bool
+	AnnotationsRef bool
+}