diff --git a/cmd/fix.go b/cmd/fix.go index 336ab54b2..f327cec64 100644 --- a/cmd/fix.go +++ b/cmd/fix.go @@ -17,7 +17,8 @@ import ( rio "github.com/styrainc/regal/internal/io" "github.com/styrainc/regal/pkg/config" "github.com/styrainc/regal/pkg/fixer" - "github.com/styrainc/regal/pkg/fixer/fp" + "github.com/styrainc/regal/pkg/fixer/fileprovider" + "github.com/styrainc/regal/pkg/fixer/fixes" "github.com/styrainc/regal/pkg/linter" ) @@ -213,10 +214,10 @@ func fix(args []string, params *fixCommandParams) error { log.Println("no user-provided config file found, will use the default config") } - f := fixer.Fixer{} - f.RegisterFixes(fixer.NewDefaultFixes()...) + f := fixer.NewFixer() + f.RegisterFixes(fixes.NewDefaultFixes()...) - fileProvider := fp.NewFSFileProvider(args...) + fileProvider := fileprovider.NewFSFileProvider(args...) fixReport, err := f.Fix(ctx, &l, fileProvider) if err != nil { diff --git a/pkg/fixer/fp/fp.go b/pkg/fixer/fileprovider/fp.go similarity index 90% rename from pkg/fixer/fp/fp.go rename to pkg/fixer/fileprovider/fp.go index 3756f6751..ab7832c89 100644 --- a/pkg/fixer/fp/fp.go +++ b/pkg/fixer/fileprovider/fp.go @@ -1,4 +1,4 @@ -package fp +package fileprovider import "github.com/styrainc/regal/pkg/rules" diff --git a/pkg/fixer/fp/fs.go b/pkg/fixer/fileprovider/fs.go similarity index 98% rename from pkg/fixer/fp/fs.go rename to pkg/fixer/fileprovider/fs.go index e1bf58714..155a34d5a 100644 --- a/pkg/fixer/fp/fs.go +++ b/pkg/fixer/fileprovider/fs.go @@ -1,4 +1,4 @@ -package fp +package fileprovider import ( "fmt" diff --git a/pkg/fixer/fp/inmem.go b/pkg/fixer/fileprovider/inmem.go similarity index 98% rename from pkg/fixer/fp/inmem.go rename to pkg/fixer/fileprovider/inmem.go index efdde081f..099be797c 100644 --- a/pkg/fixer/fp/inmem.go +++ b/pkg/fixer/fileprovider/inmem.go @@ -1,4 +1,4 @@ -package fp +package fileprovider import ( "fmt" diff --git a/pkg/fixer/fixer.go b/pkg/fixer/fixer.go index 451bd575e..2018fd42c 100644 --- a/pkg/fixer/fixer.go +++ b/pkg/fixer/fixer.go @@ -3,84 +3,16 @@ package fixer import ( "context" "fmt" - "slices" "github.com/open-policy-agent/opa/ast" - "github.com/open-policy-agent/opa/format" + "github.com/styrainc/regal/pkg/fixer/fileprovider" "github.com/styrainc/regal/pkg/fixer/fixes" - "github.com/styrainc/regal/pkg/fixer/fp" "github.com/styrainc/regal/pkg/linter" ) -// Report contains updated file contents and summary information about the fixes that were applied -// during a fix operation. -type Report struct { - totalFixes int - fileFixedViolations map[string]map[string]struct{} -} - -func NewReport() *Report { - return &Report{ - fileFixedViolations: make(map[string]map[string]struct{}), - } -} - -func (r *Report) SetFileFixedViolation(file string, violation string) { - if _, ok := r.fileFixedViolations[file]; !ok { - r.fileFixedViolations[file] = make(map[string]struct{}) - } - - _, ok := r.fileFixedViolations[file][violation] - if !ok { - r.fileFixedViolations[file][violation] = struct{}{} - r.totalFixes++ - } -} - -func (r *Report) FixedViolationsForFile(file string) []string { - fixedViolations := make([]string, 0) - for violation := range r.fileFixedViolations[file] { - fixedViolations = append(fixedViolations, violation) - } - - // sort the violations for deterministic output - slices.Sort(fixedViolations) - - return fixedViolations -} - -func (r *Report) FixedFiles() []string { - fixedFiles := make([]string, 0) - for file := range r.fileFixedViolations { - fixedFiles = append(fixedFiles, file) - } - - // sort the files for deterministic output - slices.Sort(fixedFiles) - - return fixedFiles -} - -func (r *Report) TotalFixes() int { - // totalFixes is incremented for each unique violation that is fixed - return r.totalFixes -} - -// NewDefaultFixes returns a list of default fixes that are applied by the fix command. -// When a new fix is added, it should be added to this list. -func NewDefaultFixes() []fixes.Fix { - return []fixes.Fix{ - &fixes.Fmt{}, - &fixes.Fmt{ - KeyOverride: "use-rego-v1", - OPAFmtOpts: format.Opts{ - RegoVersion: ast.RegoV0CompatV1, - }, - }, - &fixes.UseAssignmentOperator{}, - &fixes.NoWhitespaceComment{}, - } +func NewFixer() *Fixer { + return &Fixer{} } type Fixer struct { @@ -111,8 +43,8 @@ func (f *Fixer) GetFixForKey(key string) (fixes.Fix, bool) { return fixInstance, true } -func (f *Fixer) Fix(ctx context.Context, l *linter.Linter, fp fp.FileProvider) (*Report, error) { - enabledRules, err := l.EnabledRules(ctx) +func (f *Fixer) Fix(ctx context.Context, l *linter.Linter, fp fileprovider.FileProvider) (*Report, error) { + enabledRules, err := l.DetermineEnabledRules(ctx) if err != nil { return nil, fmt.Errorf("failed to determine enabled rules: %w", err) } diff --git a/pkg/fixer/fixer_test.go b/pkg/fixer/fixer_test.go index e40dbcb84..8f98f2556 100644 --- a/pkg/fixer/fixer_test.go +++ b/pkg/fixer/fixer_test.go @@ -6,7 +6,8 @@ import ( "slices" "testing" - "github.com/styrainc/regal/pkg/fixer/fp" + "github.com/styrainc/regal/pkg/fixer/fileprovider" + "github.com/styrainc/regal/pkg/fixer/fixes" "github.com/styrainc/regal/pkg/linter" ) @@ -24,7 +25,7 @@ deny = true `), } - memfp := fp.NewInMemoryFileProvider(policies) + memfp := fileprovider.NewInMemoryFileProvider(policies) input, err := memfp.ToInput() if err != nil { @@ -35,8 +36,8 @@ deny = true WithEnableAll(true). WithInputModules(&input) - f := Fixer{} - f.RegisterFixes(NewDefaultFixes()...) + f := NewFixer() + f.RegisterFixes(fixes.NewDefaultFixes()...) fixReport, err := f.Fix(context.Background(), &l, memfp) if err != nil { diff --git a/pkg/fixer/fixes/fixes.go b/pkg/fixer/fixes/fixes.go index 84140ae20..a20c6aedd 100644 --- a/pkg/fixer/fixes/fixes.go +++ b/pkg/fixer/fixes/fixes.go @@ -1,6 +1,25 @@ package fixes -import "github.com/open-policy-agent/opa/ast" +import ( + "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/format" +) + +// NewDefaultFixes returns a list of default fixes that are applied by the fix command. +// When a new fix is added, it should be added to this list. +func NewDefaultFixes() []Fix { + return []Fix{ + &Fmt{}, + &Fmt{ + KeyOverride: "use-rego-v1", + OPAFmtOpts: format.Opts{ + RegoVersion: ast.RegoV0CompatV1, + }, + }, + &UseAssignmentOperator{}, + &NoWhitespaceComment{}, + } +} // Fix is the interface that must be implemented by all fixes. type Fix interface { diff --git a/pkg/fixer/report.go b/pkg/fixer/report.go new file mode 100644 index 000000000..4828a848e --- /dev/null +++ b/pkg/fixer/report.go @@ -0,0 +1,57 @@ +package fixer + +import "slices" + +// Report contains updated file contents and summary information about the fixes that were applied +// during a fix operation. +type Report struct { + totalFixes int + fileFixedViolations map[string]map[string]struct{} +} + +func NewReport() *Report { + return &Report{ + fileFixedViolations: make(map[string]map[string]struct{}), + } +} + +func (r *Report) SetFileFixedViolation(file string, violation string) { + if _, ok := r.fileFixedViolations[file]; !ok { + r.fileFixedViolations[file] = make(map[string]struct{}) + } + + _, ok := r.fileFixedViolations[file][violation] + if !ok { + r.fileFixedViolations[file][violation] = struct{}{} + r.totalFixes++ + } +} + +func (r *Report) FixedViolationsForFile(file string) []string { + fixedViolations := make([]string, 0) + for violation := range r.fileFixedViolations[file] { + fixedViolations = append(fixedViolations, violation) + } + + // sort the violations for deterministic output + slices.Sort(fixedViolations) + + return fixedViolations +} + +func (r *Report) FixedFiles() []string { + fixedFiles := make([]string, 0) + for file := range r.fileFixedViolations { + fixedFiles = append(fixedFiles, file) + } + + // sort the files for deterministic output + slices.Sort(fixedFiles) + + return fixedFiles +} + +func (r *Report) TotalFixes() int { + // totalFixes is incremented for each unique violation that is fixed + return r.totalFixes +} diff --git a/pkg/fixer/reporter.go b/pkg/fixer/reporter.go index 7d9cdb137..379c79e7c 100644 --- a/pkg/fixer/reporter.go +++ b/pkg/fixer/reporter.go @@ -5,10 +5,12 @@ import ( "io" ) +// Reporter is responsible for outputting a fix report in a specific format. type Reporter interface { Report(*Report) error } +// ReporterForFormat returns a suitable Reporter for outputting a fix report in the given format. func ReporterForFormat(format string, outputWriter io.Writer) (Reporter, error) { switch format { case "pretty": @@ -18,6 +20,7 @@ func ReporterForFormat(format string, outputWriter io.Writer) (Reporter, error) } } +// PrettyReporter outputs a fix report in a human-readable format. type PrettyReporter struct { outputWriter io.Writer } diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go index 60a169a03..d4d3aa325 100644 --- a/pkg/linter/linter.go +++ b/pkg/linter/linter.go @@ -346,7 +346,10 @@ func (l Linter) Lint(ctx context.Context) (report.Report, error) { return finalReport, nil } -func (l Linter) EnabledRules(ctx context.Context) ([]string, error) { +// DetermineEnabledRules returns the list of rules that are enabled based on the supplied configuration. +// This makes use of the Rego and Go rule settings to produce a single list of the rules that are to be run +// on this linter instance. +func (l Linter) DetermineEnabledRules(ctx context.Context) ([]string, error) { enabledRules := make([]string, 0) goRules, err := l.enabledGoRules() diff --git a/pkg/linter/linter_test.go b/pkg/linter/linter_test.go index 5dfcc1193..3d852a818 100644 --- a/pkg/linter/linter_test.go +++ b/pkg/linter/linter_test.go @@ -581,7 +581,7 @@ func TestEnabledRules(t *testing.T) { WithDisableAll(true). WithEnabledRules("opa-fmt", "no-whitespace-comment") - enabledRules, err := linter.EnabledRules(context.Background()) + enabledRules, err := linter.DetermineEnabledRules(context.Background()) if err != nil { t.Fatalf("expected no error, got %v", err) }