Strict mode: fail on unused imports [PR]

ast/compile.go

@@ -259,6 +259,7 @@ func NewCompiler() *Compiler {
 		f          func()
 	}{
 		{"CheckDuplicateImports", "compile_stage_check_duplicate_imports", c.checkDuplicateImports},
+		{"CheckUnusedImports", "compile_stage_check_unused_imports", c.checkUnusedImports},
 		{"CheckKeywordOverrides", "compile_stage_check_keyword_overrides", c.checkKeywordOverrides},
 		// Reference resolution should run first as it may be used to lazily
 		// load additional modules. If any stages run before resolution, they
@@ -1321,6 +1322,62 @@ func (c *Compiler) GetAnnotationSet() *AnnotationSet {
 	return c.annotationSet
 }
 
+func (c *Compiler) checkUnusedImports() {
+	if !c.strict {
+		return
+	}
+
+	for _, name := range c.sorted {
+		mod := c.Modules[name]
+
+		for _, imp := range mod.Imports {
+			var impCounter int
+			importName := imp.Name().String()
+
+			WalkExprs(mod, func(expr *Expr) bool {
+				switch t := expr.Terms.(type) {
+				case *Every:
+					_ = t // term is unused
+					if strings.Contains(expr.String(), importName) {
+						impCounter++
+					}
+				default:
+					WalkTerms(mod, func(term *Term) bool {
+						termStr := term.Value.String()
+						if importName == "in" {
+							switch {
+							case termStr == "internal.member_2":
+								if importName == "in" {
+									impCounter++
+								}
+							case termStr == "internal.member_3":
+								if importName == "in" {
+									impCounter++
+								}
+							default:
+								if strings.Contains(expr.String(), termStr) {
+									impCounter++
+								}
+							}
+						} else {
+							if importName == termStr {
+								impCounter++
+							}
+						}
+						return false
+					})
+				}
+
+				return true
+			})
+
+			if impCounter == 0 {
+				c.err(NewError(CompileErr, imp.Location, "%v unused", imp.String()))
+			}
+		}
+	}
+}
+
 func (c *Compiler) checkDuplicateImports() {
 	if !c.strict {
 		return
@@ -1332,6 +1389,7 @@ func (c *Compiler) checkDuplicateImports() {
 
 		for _, imp := range mod.Imports {
 			name := imp.Name()
+
 			if processed, conflict := processedImports[name]; conflict {
 				c.err(NewError(CompileErr, imp.Location, "import must not shadow %v", processed))
 			} else {

ast/compile_test.go

@@ -1500,6 +1500,147 @@ func TestCompilerRewriteExprTerms(t *testing.T) {
 	}
 }
 
+func TestCompilerCheckUnusedImports(t *testing.T) {
+	cases := []strictnessTestCase{
+		{
+			note: "fail_case_1",
+			module: `package p
+			import data.foo.bar as bar
+			r {
+				input.bar == 11
+			  }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("import"), "", 2, 4),
+					Message:  "import data.foo.bar as bar unused",
+				},
+			},
+		},
+		{
+			note: "fail_case_2",
+			module: `package p
+			import data.foo # unused
+			r { data.foo == 10 }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("import"), "", 2, 4),
+					Message:  "import data.foo unused",
+				},
+			},
+		},
+		{
+			note: "fail_case_3",
+			module: `package p
+			import data.foo
+			import data.x.power #unused
+			r { foo == 10 }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("import"), "", 3, 4),
+					Message:  "import data.x.power unused",
+				},
+			},
+		},
+		{
+			note: "fail_case_4",
+			module: `package p
+			import data.foo
+			import data.x.power #unused
+			r { input.unused == 10 }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("import"), "", 2, 4),
+					Message:  "import data.foo unused",
+				},
+				&Error{
+					Location: NewLocation([]byte("import"), "", 3, 4),
+					Message:  "import data.x.power unused",
+				},
+			},
+		},
+		{
+			note: "pass_case_1",
+			module: `package p
+			import data.foo.x
+			r { x == 10 }
+			`,
+		},
+		{
+			note: "pass_case_2",
+			module: `package p
+			import data.foo.x
+			r { input.data == x }
+			`,
+		},
+		{
+			note: "pass_case_3",
+			module: `package p
+			import data.foo.x
+			import data.power.ranger
+			r { ranger == x }
+			`,
+		},
+		{
+			note: "pass_case_4",
+			module: `package p
+			import data.foo.x
+			import data.power.ranger
+			r { ranger == 23 }
+			t { x == 1 }
+			`,
+		},
+		{
+			note: "pass_case_5",
+			module: `package p
+			import data.foo
+			r = count(foo) > 1 # only one operand
+			`,
+		},
+		{
+			note: "pass_case_6",
+			module: `package p
+			import data.foo
+			r = sprintf("%v %d", [foo, 0]) # more complicated operands
+			`,
+		},
+		{
+			note: "pass_case_7",
+			module: `package p
+			import data.foo
+			r {
+				foo # not a call, plain term
+			}
+			`,
+		},
+		{
+			note: "pass_case_8",
+			module: `package p
+			import future.keywords.every
+			import data.foo
+			r {
+				every x in foo { x > 1 } # import in 'every' domain
+			}
+			`,
+		},
+		{
+			note: "pass_case_9",
+			module: `package p
+			import future.keywords.every
+			import data.foo
+			r {
+				every x in [1,2,3] { x > foo } # import in 'every' body
+			}
+			`,
+		},
+	}
+
+	runStrictnessTestCase(t, cases, true)
+}
+
 func TestCompilerCheckDuplicateImports(t *testing.T) {
 	cases := []strictnessTestCase{
 		{

ast/fuzz.go

@@ -1,3 +1,4 @@
+//go:build gofuzz
 // +build gofuzz
 
 package ast

docs/content/strict.md

@@ -20,5 +20,6 @@ Name | Description | Enforced by default in OPA version
 --- | --- | ---
 Duplicate imports | Duplicate [imports](../policy-language/#imports), where one import shadows another, are prohibited. | 1.0
 Unused local assignments | Unused [assignments](../policy-reference/#assignment-and-equality) local to a rule, function or comprehension are prohibited | 1.0
+Unused imports | Unused [imports](../policy-language/#imports) are prohibited. | 1.0
 `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 
 `any()` and `all()` removed | The `any()` and `all()` built-in functions have been deprecated, and will be removed in OPA 1.0. Use of these functions is prohibited. | 1.0