diff --git a/docs/fixing.md b/docs/fixing.md index 8fab59b6..2efe09b9 100644 --- a/docs/fixing.md +++ b/docs/fixing.md @@ -22,6 +22,7 @@ Currently, the following rules are automatically fixable: - [use-assignment-operator](https://www.openpolicyagent.org/projects/regal/rules/style/use-assignment-operator) - [no-whitespace-comment](https://www.openpolicyagent.org/projects/regal/rules/style/no-whitespace-comment) - [directory-package-mismatch](https://www.openpolicyagent.org/projects/regal/rules/idiomatic/directory-package-mismatch) +- [prefer-equals-comparison](https://www.openpolicyagent.org/projects/regal/rules/idiomatic/prefer-equals-comparison) - [use-rego-v1](https://www.openpolicyagent.org/projects/regal/rules/imports/use-rego-v1) (v0 Rego only) So, how do you go on about automatically fixing reported violations? diff --git a/docs/rules/idiomatic/prefer-equals-comparison.md b/docs/rules/idiomatic/prefer-equals-comparison.md index 964c9f70..b44235f4 100644 --- a/docs/rules/idiomatic/prefer-equals-comparison.md +++ b/docs/rules/idiomatic/prefer-equals-comparison.md @@ -4,6 +4,8 @@ **Category**: Idiomatic +**Automatically fixable**: [Yes](/regal/fixing) + **Avoid** ```rego package policy diff --git a/pkg/fixer/fixes/fixes.go b/pkg/fixer/fixes/fixes.go index 6fcbbbea..f7fc9b5e 100644 --- a/pkg/fixer/fixes/fixes.go +++ b/pkg/fixer/fixes/fixes.go @@ -22,6 +22,7 @@ func NewDefaultFixes() []Fix { &NoWhitespaceComment{}, &DirectoryPackageMismatch{}, &NonRawRegexPattern{}, + &PreferEqualsComparison{}, } } @@ -33,6 +34,7 @@ func NewDefaultFormatterFixes() []Fix { &UseAssignmentOperator{}, &NoWhitespaceComment{}, &NonRawRegexPattern{}, + &PreferEqualsComparison{}, } } diff --git a/pkg/fixer/fixes/preferequalscomparison.go b/pkg/fixer/fixes/preferequalscomparison.go new file mode 100644 index 00000000..48e94b08 --- /dev/null +++ b/pkg/fixer/fixes/preferequalscomparison.go @@ -0,0 +1,44 @@ +package fixes + +import ( + "errors" + "strings" +) + +type PreferEqualsComparison struct{} + +func (*PreferEqualsComparison) Name() string { + return "prefer-equals-comparison" +} + +func (p *PreferEqualsComparison) Fix(fc *FixCandidate, opts *RuntimeOptions) ([]FixResult, error) { + if opts == nil { + return nil, errors.New("missing runtime options") + } + + lines := strings.Split(fc.Contents, "\n") + fixed := false + + for _, loc := range opts.Locations { + line := lines[loc.Row-1] + if loc.Row > len(lines) || loc.Column-1 < 0 || loc.Column-1 >= len(line) { + continue + } + + eqIndex := strings.Index(line, "=") + + // unification operator not found, skipping + if eqIndex == -1 { + continue + } + + lines[loc.Row-1] = line[0:eqIndex] + "=" + line[eqIndex:] + fixed = true + } + + if !fixed { + return nil, nil + } + + return []FixResult{{Title: p.Name(), Root: opts.BaseDir, Contents: strings.Join(lines, "\n")}}, nil +} diff --git a/pkg/fixer/fixes/preferequalscomparison_test.go b/pkg/fixer/fixes/preferequalscomparison_test.go new file mode 100644 index 00000000..3fc16709 --- /dev/null +++ b/pkg/fixer/fixes/preferequalscomparison_test.go @@ -0,0 +1,156 @@ +package fixes + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/open-policy-agent/regal/pkg/report" +) + +func TestPreferEqualsComparison(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + contentAfterFix string + fc *FixCandidate + fixExpected bool + runtimeOptions *RuntimeOptions + }{ + "no change": { + fc: &FixCandidate{Filename: "test.rego", Contents: "package test\n\nallow = true\n"}, + contentAfterFix: "package test\n\nallow = true\n", + fixExpected: false, + runtimeOptions: &RuntimeOptions{}, + }, + "no change because no location": { + fc: &FixCandidate{ + Filename: "test.rego", + Contents: `package test +allow := true + +test_rule { + allow = true +}`, + }, + contentAfterFix: `package test +allow := true + +test_rule { + allow = true +}`, + fixExpected: false, + runtimeOptions: &RuntimeOptions{}, + }, + "single change": { + fc: &FixCandidate{ + Filename: "test.rego", + Contents: `package test +allow := true + +test_rule { + allow = true +}`, + }, + contentAfterFix: `package test +allow := true + +test_rule { + allow == true +}`, + fixExpected: true, + runtimeOptions: &RuntimeOptions{Locations: []report.Location{{Row: 5, Column: 2}}}, + }, + "bad change": { + fc: &FixCandidate{ + Filename: "test.rego", + Contents: `package test +allow := true +allow = true + +test_rule { + allow = false +}`, + }, + contentAfterFix: `package test +allow := true +allow = true + +test_rule { + allow = false +}`, + fixExpected: false, + runtimeOptions: &RuntimeOptions{Locations: []report.Location{{Row: 1, Column: 1}}}, + }, + "many changes": { + fc: &FixCandidate{ + Filename: "test.rego", + Contents: `package test +allow := true + +test_rule { + allow = true + allow = false +}`, + }, + contentAfterFix: `package test +allow := true + +test_rule { + allow == true + allow == false +}`, + fixExpected: true, + runtimeOptions: &RuntimeOptions{Locations: []report.Location{{Row: 5, Column: 2}, {Row: 6, Column: 2}}}, + }, + "different columns": { + fc: &FixCandidate{ + Filename: "test.rego", + Contents: `package test +allow := true + +test_rule { + allow = true + allow = false +}`, + }, + contentAfterFix: `package test +allow := true + +test_rule { + allow == true + allow == false +}`, + fixExpected: true, + runtimeOptions: &RuntimeOptions{Locations: []report.Location{{Row: 5, Column: 2}, {Row: 6, Column: 2}}}, + }, + } + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + t.Parallel() + + pec := PreferEqualsComparison{} + + fixResults, err := pec.Fix(tc.fc, tc.runtimeOptions) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !tc.fixExpected && len(fixResults) != 0 { + t.Fatalf("unexpected fix applied") + } + + if !tc.fixExpected { + return + } + + if diff := cmp.Diff(fixResults[0].Contents, tc.contentAfterFix); tc.fixExpected && diff != "" { + t.Fatalf( + "unexpected content, got:\n%s---\nexpected:\n%s---", + fixResults[0].Contents, + tc.contentAfterFix, + ) + } + }) + } +}