rules: OPA fmt rule location fix (#633)

* rules: OPA fmt rule location fix

#624

Signed-off-by: Charlie Egan <charlie@styra.com>

* rules: Use location of package for opa-fmt

This now works if the package is not on the first line of the file too

Signed-off-by: Charlie Egan <charlie@styra.com>

---------

Signed-off-by: Charlie Egan <charlie@styra.com>

pkg/rules/fmt.go

@@ -51,6 +51,13 @@ func (f *OpaFmtRule) Run(ctx context.Context, input Input) (*report.Report, erro
 			}
 
 			if !bytes.Equal(formatted, unformatted) {
+				row := module.Package.Location.Row
+				txt := module.Package.String()
+
+				if lines := bytes.SplitN(unformatted, []byte("\n"), row+1); len(lines) > row {
+					txt = string(lines[row-1])
+				}
+
 				violation := report.Violation{
 					Title:       title,
 					Description: description,
@@ -60,7 +67,10 @@ func (f *OpaFmtRule) Run(ctx context.Context, input Input) (*report.Report, erro
 						Reference:   f.Documentation(),
 					}},
 					Location: report.Location{
-						File: filename,
+						File:   filename,
+						Row:    row,
+						Column: 1,
+						Text:   &txt,
 					},
 					Level: f.ruleConfig.Level,
 				}

pkg/rules/fmt_test.go

@@ -15,24 +15,72 @@ func TestFmtRuleFail(t *testing.T) {
 
 	ctx := context.Background()
 
-	policy := "  package p "
+	testCases := map[string]struct {
+		policy string
+		expRow int
+		expCol int
+		expTxt string
+	}{
+		"simple": {
+			policy: " package p\n",
+			expRow: 1,
+			expCol: 1,
+			expTxt: " package p",
+		},
+		"metadata": {
+			//nolint:dupword
+			policy: `# METADATA
+# scope: package
+# description: A note for the package
+   package     p
 
-	result := testutil.Must(rules.NewOpaFmtRule(config.Config{}).Run(ctx, test.InputPolicy("p.rego", policy)))(t)
 
-	if len(result.Violations) != 1 {
-		t.Errorf("expected 1 violation, got %d", len(result.Violations))
+ allow =  true
+`,
+			expRow: 4,
+			expCol: 1,
+			expTxt: "   package     p",
+		},
 	}
 
-	if result.Violations[0].Title != "opa-fmt" {
-		t.Errorf("expected violation title to be 'opa-fmt', got %s", result.Violations[0].Title)
-	}
+	for name, tc := range testCases {
+		tc := tc
 
-	if result.Violations[0].Category != "style" {
-		t.Errorf("expected violation category to be 'style', got %s", result.Violations[0].Category)
-	}
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			result := testutil.Must(
+				rules.NewOpaFmtRule(config.Config{}).Run(ctx, test.InputPolicy("p.rego", tc.policy)),
+			)(t)
+
+			if len(result.Violations) != 1 {
+				t.Errorf("expected 1 violation, got %d", len(result.Violations))
+			}
+
+			if result.Violations[0].Title != "opa-fmt" {
+				t.Errorf("expected violation title to be 'opa-fmt', got %s", result.Violations[0].Title)
+			}
+
+			if result.Violations[0].Category != "style" {
+				t.Errorf("expected violation category to be 'style', got %s", result.Violations[0].Category)
+			}
+
+			if result.Violations[0].Location.File != "p.rego" {
+				t.Errorf("expected violation location file to be 'p.rego', got %s", result.Violations[0].Location.File)
+			}
+
+			if result.Violations[0].Location.Row != tc.expRow {
+				t.Errorf("expected violation location row to be 1, got %d", result.Violations[0].Location.Row)
+			}
+
+			if result.Violations[0].Location.Column != tc.expCol {
+				t.Errorf("expected violation location column to be 1, got %d", result.Violations[0].Location.Column)
+			}
 
-	if result.Violations[0].Location.File != "p.rego" {
-		t.Errorf("expected violation location file to be 'p.rego', got %s", result.Violations[0].Location.File)
+			if *result.Violations[0].Location.Text != tc.expTxt {
+				t.Errorf("expected violation location text to be '  package p ', got %q", *result.Violations[0].Location.Text)
+			}
+		})
 	}
 }