Self review refactor and tidy

Signed-off-by: Charlie Egan <[email protected]>

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 {

pkg/fixer/fp/fp.go → pkg/fixer/fileprovider/fp.go

@@ -1,4 +1,4 @@
-package fp
+package fileprovider
 
 import "github.com/styrainc/regal/pkg/rules"
 

pkg/fixer/fp/fs.go → pkg/fixer/fileprovider/fs.go

@@ -1,4 +1,4 @@
-package fp
+package fileprovider
 
 import (
 	"fmt"

pkg/fixer/fp/inmem.go → pkg/fixer/fileprovider/inmem.go

@@ -1,4 +1,4 @@
-package fp
+package fileprovider
 
 import (
 	"fmt"

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,7 +43,7 @@ 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) {
+func (f *Fixer) Fix(ctx context.Context, l *linter.Linter, fp fileprovider.FileProvider) (*Report, error) {
 	enabledRules, err := l.EnabledRules(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to determine enabled rules: %w", err)

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 {

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 {

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
+}

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
 }