Rego-based opa-fmt

Well, with the help of a new built-in function, anyway. This allows us to ditch the system for Go-based rules entirely, which feels like a big step for this project! Fixes #1298 Signed-off-by: Anders Eknert <[email protected]>
StyraInc · Feb 4, 2025 · 9c13fa3 · 9c13fa3
1 parent a283bee
commit 9c13fa3
Show file tree

Hide file tree

Showing 21 changed files with 218 additions and 522 deletions.
diff --git a/build/capabilities.json b/build/capabilities.json
@@ -6276,6 +6276,35 @@
         },
         "type": "function"
       }
+    },
+    {
+      "name": "regal.is_formatted",
+      "decl": {
+        "args": [
+          {
+            "description": "input string to check for formatting",
+            "name": "input",
+            "type": "string"
+          },
+          {
+            "description": "formatting options",
+            "dynamic": {
+              "key": {
+                "type": "string"
+              },
+              "value": {
+                "type": "any"
+              }
+            },
+            "name": "options",
+            "type": "object"
+          }
+        ],
+        "result": {
+          "type": "boolean"
+        },
+        "type": "function"
+      }
     }
   ],
   "features": [

diff --git a/bundle/regal/rules/style/opa-fmt/opa_fmt.rego b/bundle/regal/rules/style/opa-fmt/opa_fmt.rego
@@ -0,0 +1,19 @@
+# METADATA
+# description: File should be formatted with `opa fmt`
+package regal.rules.style["opa-fmt"]
+
+import data.regal.result
+
+report contains violation if {
+	# NOTE:
+	# 1. this won't identify CRLF line endings, as we've stripped them from the input previously
+	# 2. this will perform worse than having the text representation of the file in the input
+	not regal.is_formatted(concat("\n", input.regal.file.lines), {"rego_version": input.regal.file.rego_version})
+
+	violation := result.fail(rego.metadata.chain(), {"location": {
+		"file": input.regal.file.name,
+		"row": 1,
+		"col": 1,
+		"text": input.regal.file.lines[0],
+	}})
+}
diff --git a/bundle/regal/rules/style/opa-fmt/opa_fmt_test.rego b/bundle/regal/rules/style/opa-fmt/opa_fmt_test.rego
@@ -0,0 +1,62 @@
+package regal.rules.style["opa-fmt_test"]
+
+import data.regal.config
+import data.regal.rules.style["opa-fmt"] as rule
+
+test_fail_not_formatted if {
+	r := rule.report with input as regal.parse_module("p.rego", `package p    `)
+		with input.regal.file.rego_version as "v1"
+
+	r == {{
+		"category": "style",
+		"description": "File should be formatted with `opa fmt`",
+		"level": "error",
+		"location": {
+			"col": 1,
+			"file": "p.rego",
+			"row": 1,
+			"text": "package p    ",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/opa-fmt", "style"),
+		}],
+		"title": "opa-fmt",
+	}}
+}
+
+test_success_formatted if {
+	r := rule.report with input as regal.parse_module("p.rego", "package p\n")
+		with input.regal.file.rego_version as "v1"
+
+	r == set()
+}
+
+test_fail_v0_required_but_v1_policy if {
+	r := rule.report with input as regal.parse_module("p.rego", "package p\n\none contains 1\n")
+		with input.regal.file.rego_version as "v0"
+
+	r == {{
+		"category": "style",
+		"description": "File should be formatted with `opa fmt`",
+		"level": "error",
+		"location": {
+			"col": 1,
+			"file": "p.rego",
+			"row": 1,
+			"text": "package p",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/opa-fmt", "style"),
+		}],
+		"title": "opa-fmt",
+	}}
+}
+
+test_success_v0_required_and_v0_policy if {
+	r := rule.report with input as regal.parse_module("p_v0.rego", "package p\n\none[\"1\"]\n")
+		with input.regal.file.rego_version as "v0"
+
+	r == set()
+}
diff --git a/cmd/debugger.go b/cmd/debugger.go
@@ -16,8 +16,7 @@ import (
 	"github.com/open-policy-agent/opa/v1/ast/location"
 	"github.com/open-policy-agent/opa/v1/debug"
 	"github.com/open-policy-agent/opa/v1/logging"
-	"github.com/open-policy-agent/opa/v1/rego"
-	"github.com/open-policy-agent/opa/v1/util"
+	outil "github.com/open-policy-agent/opa/v1/util"
 
 	"github.com/styrainc/regal/internal/dap"
 	"github.com/styrainc/regal/pkg/builtins"
@@ -259,10 +258,13 @@ func (s *state) launch(ctx context.Context, r *godap.LaunchRequest) (*godap.Laun
 			return dap.NewLaunchResponse(), fmt.Errorf("invalid launch eval properties: %w", err)
 		}
 
+		funcs := make([]debug.LaunchOption, 0, len(builtins.RegalBuiltinRegoFuncs))
+		for _, f := range builtins.RegalBuiltinRegoFuncs {
+			funcs = append(funcs, debug.RegoOption(f))
+		}
+
 		// FIXME: Should we protect this with a mutex?
-		s.session, err = s.debugger.LaunchEval(ctx, evalProps,
-			debug.RegoOption(rego.Function2(builtins.RegalParseModuleMeta, builtins.RegalParseModule)),
-			debug.RegoOption(rego.Function1(builtins.RegalLastMeta, builtins.RegalLast)))
+		s.session, err = s.debugger.LaunchEval(ctx, evalProps, funcs...)
 	case "test":
 		err = errors.New("test not supported")
 	case "":
@@ -355,7 +357,7 @@ func pos(loc *location.Location) (source *godap.Source, line, col, endLine, endC
 		}
 	}
 
-	lines := strings.Split(util.ByteSliceToString(loc.Text), "\n")
+	lines := strings.Split(outil.ByteSliceToString(loc.Text), "\n")
 	line = loc.Row
 	col = loc.Col
 

diff --git a/cmd/table.go b/cmd/table.go
@@ -21,8 +21,6 @@ import (
 	"github.com/styrainc/regal/internal/compile"
 	"github.com/styrainc/regal/internal/docs"
 	"github.com/styrainc/regal/internal/util"
-	"github.com/styrainc/regal/pkg/config"
-	"github.com/styrainc/regal/pkg/rules"
 )
 
 type tableCommandParams struct {
@@ -130,14 +128,6 @@ func createTable(args []string) (io.Reader, error) {
 		})
 	}
 
-	for _, rule := range rules.AllGoRules(config.Config{}) {
-		tableMap[rule.Category()] = append(tableMap[rule.Category()], []string{
-			rule.Category(),
-			"[" + rule.Name() + "](" + rule.Documentation() + ")",
-			rule.Description(),
-		})
-	}
-
 	// Sort the list of rules in each category by name
 	for category := range tableMap {
 		sort.Slice(tableMap[category], func(i, j int) bool {

diff --git a/docs/rules/performance/defer-assignment.md b/docs/rules/performance/defer-assignment.md
@@ -46,7 +46,7 @@ assignments, code tends to be more readable when assignments are placed close to
 
 This rule uses a fairly simplistic heuristic to determine if an assignment can be deferred:
 
-- The next expressions is not an assignment
+- The next expression is not an assignment
 - The next expression does not depend on the assignment
 - The next expression does not initialize iteration
 

diff --git a/e2e/cli_test.go b/e2e/cli_test.go
@@ -909,8 +909,8 @@ bar/main_test.rego -> wow/foo-bar/baz/main_test.rego:
 
 foo/main.rego -> wow/main.rego:
 - directory-package-mismatch
-- opa-fmt
 - no-whitespace-comment
+- opa-fmt
 
 foo/main_test.rego -> wow/main_test.rego:
 - directory-package-mismatch
@@ -919,13 +919,13 @@ foo/main_test.rego -> wow/main_test.rego:
 
 In project root: %[1]s/v0
 main.rego:
-- opa-fmt
+- use-rego-v1
 - no-whitespace-comment
 
 In project root: %[1]s/v1
 main.rego:
-- opa-fmt
 - no-whitespace-comment
+- opa-fmt
 `, td)
 
 	if act := stdout.String(); exp != act {

diff --git a/go.mod b/go.mod
@@ -7,6 +7,7 @@ toolchain go1.23.3
 require (
 	dario.cat/mergo v1.0.1
 	github.com/anderseknert/roast v0.7.0
+	github.com/arl/statsviz v0.6.0
 	github.com/coreos/go-semver v0.3.1
 	github.com/fatih/color v1.18.0
 	github.com/fsnotify/fsnotify v1.8.0
@@ -32,7 +33,6 @@ require (
 	github.com/OneOfOne/xxhash v1.2.8 // indirect
 	github.com/ProtonMail/go-crypto v1.1.5 // indirect
 	github.com/agnivade/levenshtein v1.2.0 // indirect
-	github.com/arl/statsviz v0.6.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect

diff --git a/go.sum b/go.sum
@@ -110,7 +110,6 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
 github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
-github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM=
 github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
 github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=

diff --git a/internal/compile/compile.go b/internal/compile/compile.go
@@ -20,7 +20,12 @@ func Capabilities() *ast.Capabilities {
 		&ast.Builtin{
 			Name: builtins.RegalLastMeta.Name,
 			Decl: builtins.RegalLastMeta.Decl,
-		})
+		},
+		&ast.Builtin{
+			Name: builtins.RegalIsFormattedMeta.Name,
+			Decl: builtins.RegalIsFormattedMeta.Decl,
+		},
+	)
 
 	return caps
 }

diff --git a/internal/lsp/completions/providers/policy.go b/internal/lsp/completions/providers/policy.go
@@ -132,14 +132,9 @@ func prepareQuery(ctx context.Context, store storage.Store, query string) (*rego
 }
 
 func prepareRegoArgs(store storage.Store, query ast.Body) []func(*rego.Rego) {
-	return []func(*rego.Rego){
+	return append([]func(*rego.Rego){
 		rego.Store(store),
 		rego.ParsedQuery(query),
 		rego.ParsedBundle("regal", &rbundle.LoadedBundle),
-		rego.Function2(builtins.RegalParseModuleMeta, builtins.RegalParseModule),
-		rego.Function1(builtins.RegalLastMeta, builtins.RegalLast),
-		// Uncomment for development
-		// rego.EnablePrintStatements(true),
-		// rego.PrintHook(topdown.NewPrintHook(os.Stderr)),
-	}
+	}, builtins.RegalBuiltinRegoFuncs...)
 }
diff --git a/internal/lsp/completions/refs/used.go b/internal/lsp/completions/refs/used.go
@@ -38,13 +38,11 @@ func prepareQuery() (*rego.PreparedEvalQuery, error) {
 		},
 	}
 
-	regoArgs := []func(*rego.Rego){
+	regoArgs := append([]func(*rego.Rego){
 		rego.ParsedBundle("regal", &rbundle.LoadedBundle),
 		rego.ParsedBundle("internal", &dataBundle),
 		rego.Query(`data.regal.lsp.completion.ref_names`),
-		rego.Function2(builtins.RegalParseModuleMeta, builtins.RegalParseModule),
-		rego.Function1(builtins.RegalLastMeta, builtins.RegalLast),
-	}
+	}, builtins.RegalBuiltinRegoFuncs...)
 
 	preparedQuery, err := rego.New(regoArgs...).PrepareForEval(context.Background())
 	if err != nil {

diff --git a/internal/lsp/eval.go b/internal/lsp/eval.go
@@ -149,14 +149,8 @@ func prepareRegoArgs(
 		bundleArgs = append(bundleArgs, rego.ParsedBundle(key, &b))
 	}
 
-	args := []func(*rego.Rego){
-		rego.ParsedQuery(query),
-		rego.Function2(builtins.RegalParseModuleMeta, builtins.RegalParseModule),
-		rego.Function1(builtins.RegalLastMeta, builtins.RegalLast),
-		rego.EnablePrintStatements(true),
-		rego.PrintHook(printHook),
-	}
-
+	args := []func(*rego.Rego){rego.ParsedQuery(query), rego.EnablePrintStatements(true), rego.PrintHook(printHook)}
+	args = append(args, builtins.RegalBuiltinRegoFuncs...)
 	args = append(args, bundleArgs...)
 
 	var caps *config.Capabilities

diff --git a/internal/lsp/rego/rego.go b/internal/lsp/rego/rego.go
@@ -116,12 +116,12 @@ type policy struct {
 
 func initialize() {
 	createArgs := func(args ...func(*rego.Rego)) []func(*rego.Rego) {
-		return append([]func(*rego.Rego){
+		always := append([]func(*rego.Rego){
 			rego.ParsedBundle("regal", &rbundle.LoadedBundle),
-			rego.Function2(builtins.RegalParseModuleMeta, builtins.RegalParseModule),
-			rego.Function1(builtins.RegalLastMeta, builtins.RegalLast),
 			rego.StoreReadAST(true),
-		}, args...)
+		}, builtins.RegalBuiltinRegoFuncs...)
+
+		return append(always, args...)
 	}
 
 	keywordRegoArgs := createArgs(rego.Query("data.regal.ast.keywords"))

diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go
@@ -13,7 +13,6 @@ const (
 	RegalFilterIgnoredModules = "regal_filter_ignored_modules"
 	RegalInputParse           = "regal_input_parse"
 	RegalLint                 = "regal_lint_total"
-	RegalLintGo               = "regal_lint_go"
 	RegalLintRego             = "regal_lint_rego"
 	RegalLintRegoAggregate    = "regal_lint_rego_aggregate"
 )