prefer-some-in-iteration: except subattribute iteration (#489)

As this is a more expressive form, we should allow it by default
with an option to have it disabled.

Also an unrelated renaming issue in Go code, which did not warrant
its own PR.

Fixes #351

Signed-off-by: Anders Eknert <anders@styra.com>

bundle/regal/rules/style/prefer_some_in_iteration.rego

@@ -34,9 +34,24 @@ report contains violation if {
 	num_output_vars != 0
 	num_output_vars < cfg["ignore-nesting-level"]
 
+	not except_sub_attribute(value.value)
+
 	violation := result.fail(rego.metadata.chain(), result.location(value))
 }
 
+except_sub_attribute(ref) if {
+	cfg["ignore-if-sub-attribute"] == true
+	has_sub_attribute(ref)
+}
+
+has_sub_attribute(ref) if {
+	last_var_pos := regal.last([i |
+		some i, part in ref
+		part.type == "var"
+	])
+	last_var_pos < count(ref) - 1
+}
+
 # don't walk top level iteration refs:
 # https://docs.styra.com/regal/rules/bugs/top-level-iteration
 filter_top_level_ref(rule) := rule.body if {

bundle/regal/rules/style/prefer_some_in_iteration_test.rego

@@ -118,6 +118,36 @@ test_success_complex_comprehension_term if {
 	r == set()
 }
 
+test_success_allow_if_subattribute if {
+	policy := ast.policy(`allow {
+		bar := input.foo[_].bar
+		bar == "baz"
+	}`)
+
+	r := rule.report with config.for_rule as {
+		"level": "error",
+		"ignore-if-sub-attribute": true,
+		"ignore-nesting-level": 5,
+	}
+		with input as policy
+	r == set()
+}
+
+test_fail_ignore_if_subattribute_disabled if {
+	policy := ast.policy(`allow {
+		bar := input.foo[_].bar
+		bar == "baz"
+	}`)
+
+	r := rule.report with config.for_rule as {
+		"level": "error",
+		"ignore-if-sub-attribute": false,
+		"ignore-nesting-level": 5,
+	}
+		with input as policy
+	r == with_location({"col": 10, "file": "policy.rego", "row": 4, "text": "\t\tbar := input.foo[_].bar"})
+}
+
 allow_nesting(i) := {
 	"level": "error",
 	"ignore-nesting-level": i,

cmd/parse.go

@@ -58,7 +58,7 @@ func parse(args []string) error {
 		return err
 	}
 
-	enhancedAST, err := rp.EnhanceAST(filename, content, module)
+	enhancedAST, err := rp.PrepareAST(filename, content, module)
 	if err != nil {
 		return err
 	}

docs/rules/style/prefer-some-in-iteration.md

@@ -103,6 +103,10 @@ rules:
       # except iteration if nested at or above the level i.e. setting of
       # '2' will allow `input[_].users[_]` but not `input[_]`
       ignore-nesting-level: 2
+      # except iteration over items with sub-attributes, like
+      # `name := input.users[_].name`
+      # default is true
+      ignore-if-sub-attribute: true
 ```
 
 ## Related Resources

internal/parse/parse.go

@@ -52,20 +52,20 @@ func Module(filename, policy string) (*ast.Module, error) {
 	return mod, nil
 }
 
-// EnhanceAST TODO rename with https://github.com/StyraInc/regal/issues/86.
-func EnhanceAST(name string, content string, module *ast.Module) (map[string]any, error) {
-	var enhancedAst map[string]any
+// PrepareAST prepares the AST to be used as linter input.
+func PrepareAST(name string, content string, module *ast.Module) (map[string]any, error) {
+	var preparedAST map[string]any
 
-	if err := rio.JSONRoundTrip(module, &enhancedAst); err != nil {
+	if err := rio.JSONRoundTrip(module, &preparedAST); err != nil {
 		return nil, fmt.Errorf("JSON rountrip failed for module: %w", err)
 	}
 
-	enhancedAst["regal"] = map[string]any{
+	preparedAST["regal"] = map[string]any{
 		"file": map[string]any{
 			"name":  name,
 			"lines": strings.Split(strings.ReplaceAll(content, "\r\n", "\n"), "\n"),
 		},
 	}
 
-	return enhancedAst, nil
+	return preparedAST, nil
 }

pkg/builtins/builtins.go

@@ -67,7 +67,7 @@ func RegalParseModule(_ rego.BuiltinContext, filename *ast.Term, policy *ast.Ter
 		return nil, err
 	}
 
-	enhancedAST, err := parse.EnhanceAST(string(filenameStr), string(policyStr), module)
+	enhancedAST, err := parse.PrepareAST(string(filenameStr), string(policyStr), module)
 	if err != nil {
 		return nil, err
 	}

pkg/linter/linter.go

@@ -556,7 +556,7 @@ func (l Linter) lintWithRegoRules(ctx context.Context, input rules.Input) (repor
 		go func(name string) {
 			defer wg.Done()
 
-			enhancedAST, err := parse.EnhanceAST(name, input.FileContent[name], input.Modules[name])
+			enhancedAST, err := parse.PrepareAST(name, input.FileContent[name], input.Modules[name])
 			if err != nil {
 				errCh <- fmt.Errorf("failed preparing AST: %w", err)