Allow using regal fix as a formatter (#960)

Also:
- Removed the code that would send a response back for formatter
  options that `opa fmt` can't handle. This was a good idea, but since
  there isn't any obvious way for users to fix that, it hurts more than
  it helps
- Removed the fmt fix from the default fixer options as formatting is
  done anyway by the rego-v1 fixer

Fixes #839

Signed-off-by: Anders Eknert <anders@styra.com>

Author: anderseknert

docs/fixing.md

@@ -22,3 +22,8 @@ Currently, the following rules are automatically fixable:
 Need to fix individual violations? Checkout the editors Regal supports
 [here](/regal/editor-support).
 :::
+
+## Community
+
+If you'd like to discuss Regal development or just talk about Regal in general, please join us in the `#regal`
+channel in the Styra Community [Slack](https://communityinviter.com/apps/styracommunity/signup)!

docs/language-server.md

@@ -88,13 +88,18 @@ user-defined functions too.
 
 ### Formatting
 
-Regal uses the `opa fmt` formatter for formatting Rego. This is made available as a command in editors, but also via
-a [code action](#code-actions) when unformatted files are encountered.
+By default, Regal uses the `opa fmt` formatter for formatting Rego. This is made available as a command in editors,
+but also via a [code action](#code-actions) when unformatted files are encountered.
 
 <img
   src={require('./assets/lsp/format.png').default}
   alt="Screenshot of diagnostics as displayed in Zed"/>
 
+Two other formatters are also available — `opa fmt --rego-v1` and `regal fix`. See the docs on
+[Fixing Violations](fixing.md) for more information about the `fix` command. Which formatter to use
+can be set via the `formatter` configuration option, which can be passed to Regal via the client (see
+the documentation for your client for how to do that).
+
 ### Code completions
 
 Code completions, or suggestions, is likely one of the most useful features of the Regal language server. And best of

internal/lsp/format.go

@@ -60,31 +60,3 @@ func ComputeEdits(before, after string) []types.TextEdit {
 
 	return edits
 }
-
-// The opa fmt command does not allow configuration options, so we will have
-// to ignore them if provided by the client. We can however log the warnings
-// so that the caller has some chance to be made aware of why their options
-// aren't applied.
-func validateFormattingOptions(opts types.FormattingOptions) []string {
-	warnings := make([]string, 0)
-
-	if opts.InsertSpaces {
-		warnings = append(warnings, "opa fmt: only tabs supported for indentation")
-	}
-
-	if !opts.TrimTrailingWhitespace {
-		warnings = append(warnings, "opa fmt: trailing whitespace always trimmed")
-	}
-
-	if !opts.InsertFinalNewline {
-		warnings = append(warnings, "opa fmt: final newline always inserted")
-	}
-
-	if !opts.TrimFinalNewlines {
-		warnings = append(warnings, "opa fmt: final newlines always trimmed")
-	}
-
-	// opts.TabSize ignored as we don't support using spaces
-
-	return warnings
-}

internal/lsp/server.go

@@ -88,6 +88,8 @@ type LanguageServer struct {
 	workspaceRootURI string
 	clientIdentifier clients.Identifier
 
+	clientInitializationOptions types.InitializationOptions
+
 	cache     *cache.Cache
 	regoStore storage.Store
 
@@ -352,7 +354,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) {
 				}
 			}
 
-			// when a file is 'unignored', we move it's contents to the
+			// when a file is 'unignored', we move its contents to the
 			// standard file list if missing
 			for k, v := range l.cache.GetAllIgnoredFiles() {
 				if l.ignoreURI(k) {
@@ -364,7 +366,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) {
 				if !ok {
 					l.cache.SetFileContents(k, v)
 
-					// updating the parse here will enable things like go-to defn
+					// updating the parse here will enable things like go-to definition
 					// to start working right away without the need for a file content
 					// update to run updateParse.
 					_, err = updateParse(ctx, l.cache, l.regoStore, k)
@@ -529,9 +531,9 @@ func (l *LanguageServer) StartWorkspaceStateWorker(ctx context.Context) {
 				continue
 			}
 
-			for _, uri := range changedOrNewURIs {
+			for _, cnURI := range changedOrNewURIs {
 				l.diagnosticRequestFile <- fileUpdateEvent{
-					URI:    uri,
+					URI:    cnURI,
 					Reason: "internal/workspaceStateWorker/changedOrNewFile",
 				}
 			}
@@ -1296,10 +1298,6 @@ func (l *LanguageServer) handleTextDocumentFormatting(
 		return nil, fmt.Errorf("failed to unmarshal params: %w", err)
 	}
 
-	if warnings := validateFormattingOptions(params.Options); len(warnings) > 0 {
-		l.logError(fmt.Errorf("formatting params validation warnings: %v", warnings))
-	}
-
 	var oldContent string
 
 	var ok bool
@@ -1315,43 +1313,81 @@ func (l *LanguageServer) handleTextDocumentFormatting(
 		return nil, fmt.Errorf("failed to get file contents for uri %q", params.TextDocument.URI)
 	}
 
-	// set up an in-memory file provider to pass to the fixer for this one file
-	memfp := fileprovider.NewInMemoryFileProvider(map[string][]byte{
-		params.TextDocument.URI: []byte(oldContent),
-	})
+	formatter := "opa-fmt"
 
-	input, err := memfp.ToInput()
-	if err != nil {
-		return nil, fmt.Errorf("failed to create fixer input: %w", err)
+	if l.clientInitializationOptions.Formatter != nil {
+		formatter = *l.clientInitializationOptions.Formatter
 	}
 
-	li := linter.NewLinter().
-		WithUserConfig(*l.loadedConfig).
-		WithInputModules(&input)
+	var newContent []byte
+
+	switch formatter {
+	case "opa-fmt", "opa-fmt-rego-v1":
+		opts := format.Opts{}
+		if formatter == "opa-fmt-rego-v1" {
+			opts.RegoVersion = ast.RegoV0CompatV1
+		}
+
+		f := &fixes.Fmt{OPAFmtOpts: opts}
+		p := uri.ToPath(l.clientIdentifier, params.TextDocument.URI)
+
+		fixResults, err := f.Fix(&fixes.FixCandidate{Filename: filepath.Base(p), Contents: []byte(oldContent)}, nil)
+		if err != nil {
+			l.logError(fmt.Errorf("failed to format file: %w", err))
+
+			// return "null" as per the spec
+			return nil, nil
+		}
+
+		if len(fixResults) == 0 {
+			return []types.TextEdit{}, nil
+		}
+
+		newContent = fixResults[0].Contents
+	case "regal-fix":
+		// set up an in-memory file provider to pass to the fixer for this one file
+		memfp := fileprovider.NewInMemoryFileProvider(map[string][]byte{
+			params.TextDocument.URI: []byte(oldContent),
+		})
+
+		input, err := memfp.ToInput()
+		if err != nil {
+			return nil, fmt.Errorf("failed to create fixer input: %w", err)
+		}
 
-	f := fixer.NewFixer()
-	f.RegisterFixes(fixes.NewDefaultFixes()...)
-	f.RegisterMandatoryFixes(
-		&fixes.Fmt{
-			NameOverride: "use-rego-v1",
-			OPAFmtOpts: format.Opts{
-				RegoVersion: ast.RegoV0CompatV1,
+		f := fixer.NewFixer()
+		f.RegisterFixes(fixes.NewDefaultFixes()...)
+		f.RegisterMandatoryFixes(
+			&fixes.Fmt{
+				NameOverride: "use-rego-v1",
+				OPAFmtOpts: format.Opts{
+					RegoVersion: ast.RegoV0CompatV1,
+				},
 			},
-		},
-	)
+		)
 
-	fixReport, err := f.Fix(ctx, &li, memfp)
-	if err != nil {
-		return nil, fmt.Errorf("failed to format: %w", err)
-	}
+		li := linter.NewLinter().
+			WithInputModules(&input)
 
-	if fixReport.TotalFixes() == 0 {
-		return []types.TextEdit{}, nil
-	}
+		if l.loadedConfig != nil {
+			li = li.WithUserConfig(*l.loadedConfig)
+		}
 
-	newContent, err := memfp.GetFile(params.TextDocument.URI)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get formatted contents: %w", err)
+		fixReport, err := f.Fix(ctx, &li, memfp)
+		if err != nil {
+			return nil, fmt.Errorf("failed to format: %w", err)
+		}
+
+		if fixReport.TotalFixes() == 0 {
+			return []types.TextEdit{}, nil
+		}
+
+		newContent, err = memfp.GetFile(params.TextDocument.URI)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get formatted contents: %w", err)
+		}
+	default:
+		return nil, fmt.Errorf("unrecognized formatter %q", formatter)
 	}
 
 	return ComputeEdits(oldContent, string(newContent)), nil
@@ -1507,6 +1543,10 @@ func (l *LanguageServer) handleInitialize(
 		)
 	}
 
+	if params.InitializationOptions != nil {
+		l.clientInitializationOptions = *params.InitializationOptions
+	}
+
 	regoFilter := types.FileOperationFilter{
 		Scheme: "file",
 		Pattern: types.FileOperationPattern{

internal/lsp/types/types.go

@@ -19,15 +19,20 @@ type FileEvent struct {
 	URI  string `json:"uri"`
 }
 
+type InitializationOptions struct {
+	Formatter *string `json:"formatter,omitempty"`
+}
+
 type InitializeParams struct {
-	ProcessID        int                `json:"processId"`
-	ClientInfo       Client             `json:"clientInfo"`
-	Locale           string             `json:"locale"`
-	RootPath         string             `json:"rootPath"`
-	RootURI          string             `json:"rootUri"`
-	Capabilities     ClientCapabilities `json:"capabilities"`
-	Trace            string             `json:"trace"`
-	WorkspaceFolders []WorkspaceFolder  `json:"workspaceFolders"`
+	ProcessID             int                    `json:"processId"`
+	ClientInfo            Client                 `json:"clientInfo"`
+	Locale                string                 `json:"locale"`
+	RootPath              string                 `json:"rootPath"`
+	RootURI               string                 `json:"rootUri"`
+	Capabilities          ClientCapabilities     `json:"capabilities"`
+	Trace                 string                 `json:"trace"`
+	WorkspaceFolders      []WorkspaceFolder      `json:"workspaceFolders"`
+	InitializationOptions *InitializationOptions `json:"initializationOptions,omitempty"`
 }
 
 type WorkspaceFolder struct {

pkg/fixer/fixer.go

@@ -12,10 +12,15 @@ import (
 	"github.com/styrainc/regal/pkg/linter"
 )
 
+// NewFixer instantiates a Fixer.
 func NewFixer() *Fixer {
-	return &Fixer{}
+	return &Fixer{
+		registeredFixes:          make(map[string]any),
+		registeredMandatoryFixes: make(map[string]any),
+	}
 }
 
+// Fixer must be instantiated via NewFixer.
 type Fixer struct {
 	registeredFixes          map[string]any
 	registeredMandatoryFixes map[string]any
@@ -24,25 +29,21 @@ type Fixer struct {
 // RegisterFixes sets the fixes that will be fixed if there are related linter
 // violations that can be fixed by fixes.
 func (f *Fixer) RegisterFixes(fixes ...fixes.Fix) {
-	if f.registeredFixes == nil {
-		f.registeredFixes = make(map[string]any)
-	}
-
 	for _, fix := range fixes {
-		f.registeredFixes[fix.Name()] = fix
+		if _, mandatory := f.registeredMandatoryFixes[fix.Name()]; !mandatory {
+			f.registeredFixes[fix.Name()] = fix
+		}
 	}
 }
 
 // RegisterMandatoryFixes sets fixes which will be run before other registered
 // fixes, against all files which are not ignored, regardless of linter
 // violations.
 func (f *Fixer) RegisterMandatoryFixes(fixes ...fixes.Fix) {
-	if f.registeredMandatoryFixes == nil {
-		f.registeredMandatoryFixes = make(map[string]any)
-	}
-
 	for _, fix := range fixes {
 		f.registeredMandatoryFixes[fix.Name()] = fix
+
+		delete(f.registeredFixes, fix.Name())
 	}
 }
 

pkg/fixer/fixer_test.go

@@ -49,7 +49,7 @@ deny = true
 
 	expectedFileFixedViolations := map[string][]string{
 		// use-assignment-operator is not expected since use-rego-v1 also addresses this in this example
-		"main.rego": {"no-whitespace-comment", "opa-fmt", "use-rego-v1"},
+		"main.rego": {"no-whitespace-comment", "use-rego-v1"},
 	}
 	expectedFileContents := map[string][]byte{
 		"main.rego": []byte(`package test
@@ -64,7 +64,7 @@ deny := true
 `),
 	}
 
-	if got, exp := fixReport.TotalFixes(), 3; got != exp {
+	if got, exp := fixReport.TotalFixes(), 2; got != exp {
 		t.Fatalf("expected %d fixed files, got %d", exp, got)
 	}
 

pkg/fixer/fixes/fixes.go

@@ -9,7 +9,6 @@ import (
 // When a new fix is added, it should be added to this list.
 func NewDefaultFixes() []Fix {
 	return []Fix{
-		&Fmt{},
 		&Fmt{
 			NameOverride: "use-rego-v1",
 			OPAFmtOpts: format.Opts{