lsp: Rename correctly when conflicting

This updates the server's rename quick action to use the same name
generation conflict resolution logic as the fixer.

This is done by allowing the server's cache to be used as a new file
provider and consolidating the logic for generating changes required for
the templating worker and command worker.

This also changes the inlay hints to exit early if the file is not
parsed. This reduces errors in the server logs when working with empty
files.

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

internal/lsp/cache/cache.go

@@ -106,6 +106,53 @@ func (c *Cache) SetModule(fileURI string, module *ast.Module) {
 	c.modules.Set(fileURI, module)
 }
 
+func (c *Cache) Rename(oldKey, newKey string) {
+	if content, ok := c.fileContents.Get(oldKey); ok {
+		c.fileContents.Set(newKey, content)
+		c.fileContents.Delete(oldKey)
+	}
+
+	if content, ok := c.ignoredFileContents.Get(oldKey); ok {
+		c.ignoredFileContents.Set(newKey, content)
+		c.ignoredFileContents.Delete(oldKey)
+	}
+
+	if module, ok := c.modules.Get(oldKey); ok {
+		c.modules.Set(newKey, module)
+		c.modules.Delete(oldKey)
+	}
+
+	if aggregates, ok := c.aggregateData.Get(oldKey); ok {
+		c.aggregateData.Set(newKey, aggregates)
+		c.aggregateData.Delete(oldKey)
+	}
+
+	if diagnostics, ok := c.diagnosticsFile.Get(oldKey); ok {
+		c.diagnosticsFile.Set(newKey, diagnostics)
+		c.diagnosticsFile.Delete(oldKey)
+	}
+
+	if parseErrors, ok := c.diagnosticsParseErrors.Get(oldKey); ok {
+		c.diagnosticsParseErrors.Set(newKey, parseErrors)
+		c.diagnosticsParseErrors.Delete(oldKey)
+	}
+
+	if builtinPositions, ok := c.builtinPositionsFile.Get(oldKey); ok {
+		c.builtinPositionsFile.Set(newKey, builtinPositions)
+		c.builtinPositionsFile.Delete(oldKey)
+	}
+
+	if keywordLocations, ok := c.keywordLocationsFile.Get(oldKey); ok {
+		c.keywordLocationsFile.Set(newKey, keywordLocations)
+		c.keywordLocationsFile.Delete(oldKey)
+	}
+
+	if refs, ok := c.fileRefs.Get(oldKey); ok {
+		c.fileRefs.Set(newKey, refs)
+		c.fileRefs.Delete(oldKey)
+	}
+}
+
 // SetFileAggregates will only set aggregate data for the provided URI. Even if
 // data for other files is provided, only the specified URI is updated.
 func (c *Cache) SetFileAggregates(fileURI string, data map[string][]report.Aggregate) {

internal/lsp/cache/cache_test.go

@@ -4,6 +4,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/open-policy-agent/opa/v1/ast"
+
 	"github.com/styrainc/regal/internal/lsp/types"
 	"github.com/styrainc/regal/pkg/report"
 )
@@ -159,3 +161,28 @@ func TestPartialDiagnosticsUpdate(t *testing.T) {
 		t.Fatalf("unexpected diagnostics: %v", foundDiags)
 	}
 }
+
+func TestCacheRename(t *testing.T) {
+	t.Parallel()
+
+	c := NewCache()
+
+	c.SetFileContents("file:///tmp/foo.rego", "package foo")
+	c.SetModule("file:///tmp/foo.rego", &ast.Module{})
+
+	c.Rename("file:///tmp/foo.rego", "file:///tmp/bar.rego")
+
+	_, ok := c.GetFileContents("file:///tmp/foo.rego")
+	if ok {
+		t.Fatalf("expected foo.rego to be removed")
+	}
+
+	contents, ok := c.GetFileContents("file:///tmp/bar.rego")
+	if !ok {
+		t.Fatalf("expected bar.rego to be present")
+	}
+
+	if contents != "package foo" {
+		t.Fatalf("unexpected contents: %s", contents)
+	}
+}

internal/lsp/server.go

@@ -51,6 +51,7 @@ import (
 	"github.com/styrainc/regal/pkg/fixer/fileprovider"
 	"github.com/styrainc/regal/pkg/fixer/fixes"
 	"github.com/styrainc/regal/pkg/linter"
+	"github.com/styrainc/regal/pkg/report"
 	"github.com/styrainc/regal/pkg/version"
 )
 
@@ -708,16 +709,14 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { //nolint:main
 					params,
 				)
 			case "regal.fix.directory-package-mismatch":
-				var renameParams types.ApplyWorkspaceRenameEditParams
-
 				fileURL, ok := params.Arguments[0].(string)
 				if !ok {
 					l.logf(log.LevelMessage, "expected first argument to be a string, got %T", params.Arguments[0])
 
 					break
 				}
 
-				renameParams, err = l.fixRenameParams(
+				params, err := l.fixRenameParams(
 					"Rename file to match package path",
 					&fixes.DirectoryPackageMismatch{},
 					fileURL,
@@ -728,19 +727,10 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { //nolint:main
 					break
 				}
 
-				if err := l.conn.Call(ctx, methodWorkspaceApplyEdit, renameParams, nil); err != nil {
+				if err := l.conn.Call(ctx, methodWorkspaceApplyEdit, params, nil); err != nil {
 					l.logf(log.LevelMessage, "failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())
 				}
 
-				// clean up any empty edits dirs left over
-				if len(renameParams.Edit.DocumentChanges) > 0 {
-					dir := filepath.Dir(uri.ToPath(l.clientIdentifier, renameParams.Edit.DocumentChanges[0].OldURI))
-
-					if err := util.DeleteEmptyDirs(dir); err != nil {
-						l.logf(log.LevelMessage, "failed to delete empty directories: %s", err)
-					}
-				}
-
 				// handle this ourselves as it's a rename and not a content edit
 				fixed = false
 			case "regal.debug":
@@ -1036,63 +1026,27 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) {
 				Edits:        ComputeEdits("", newContents),
 			})
 
-			// determine if a rename is needed based on the new file contents
+			// set the cache contents so that the fix can access this content
+			// when renaming the file if required.
+			l.cache.SetFileContents(job.URI, newContents)
+
+			// determine if a rename is needed based on the new file contents.
+			// renameParams will be empty if there are no renames needed
 			renameParams, err := l.fixRenameParams(
-				"Rename file to match package path",
+				"Template new Rego file",
 				&fixes.DirectoryPackageMismatch{},
 				job.URI,
 			)
 			if err != nil {
-				l.logf(log.LevelMessage, "failed to fix directory package mismatch: %s", err)
+				l.logf(log.LevelMessage, "failed to get rename params: %s", err)
 
 				continue
 			}
 
-			// move the file and clean up any empty directories ifd required
-			fileURI := job.URI
-
-			if len(renameParams.Edit.DocumentChanges) > 0 {
-				edits = append(edits, renameParams.Edit.DocumentChanges[0])
-			}
-
-			// check if there are any dirs to clean
-			if len(renameParams.Edit.DocumentChanges) > 0 {
-				dirs, err := util.DirCleanUpPaths(
-					uri.ToPath(l.clientIdentifier, renameParams.Edit.DocumentChanges[0].OldURI),
-					[]string{
-						// stop at the root
-						l.workspacePath(),
-						// also preserve any dirs needed for the new file
-						uri.ToPath(l.clientIdentifier, renameParams.Edit.DocumentChanges[0].NewURI),
-					},
-				)
-				if err != nil {
-					l.logf(log.LevelMessage, "failed to delete empty directories: %s", err)
-
-					continue
-				}
-
-				for _, dir := range dirs {
-					edits = append(
-						edits,
-						types.DeleteFile{
-							Kind: "delete",
-							URI:  uri.FromPath(l.clientIdentifier, dir),
-							Options: &types.DeleteFileOptions{
-								Recursive:         true,
-								IgnoreIfNotExists: true,
-							},
-						},
-					)
-				}
-
-				l.cache.Delete(renameParams.Edit.DocumentChanges[0].OldURI)
-			}
-
-			if err = l.conn.Call(ctx, methodWorkspaceApplyEdit, map[string]any{
-				"label": "Template new Rego file",
-				"edit": map[string]any{
-					"documentChanges": edits,
+			if err = l.conn.Call(ctx, methodWorkspaceApplyEdit, types.ApplyWorkspaceAnyEditParams{
+				Label: renameParams.Label,
+				Edit: types.WorkspaceAnyEdit{
+					DocumentChanges: append(edits, renameParams.Edit.DocumentChanges...),
 				},
 			}, nil); err != nil {
 				l.logf(log.LevelMessage, "failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())
@@ -1101,7 +1055,7 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) {
 			// finally, trigger a diagnostics run for the new file
 			updateEvent := lintFileJob{
 				Reason: "internal/templateNewFile",
-				URI:    fileURI,
+				URI:    job.URI,
 			}
 
 			l.lintFileJobs <- updateEvent
@@ -1294,60 +1248,122 @@ func (l *LanguageServer) fixRenameParams(
 	label string,
 	fix fixes.Fix,
 	fileURL string,
-) (types.ApplyWorkspaceRenameEditParams, error) {
-	contents, ok := l.cache.GetFileContents(fileURL)
-	if !ok {
-		return types.ApplyWorkspaceRenameEditParams{}, fmt.Errorf("failed to file contents %q", fileURL)
-	}
+) (types.ApplyWorkspaceAnyEditParams, error) {
+	var result types.ApplyWorkspaceAnyEditParams
 
 	roots, err := config.GetPotentialRoots(l.workspacePath())
 	if err != nil {
-		return types.ApplyWorkspaceRenameEditParams{}, fmt.Errorf("failed to get potential roots: %w", err)
+		return types.ApplyWorkspaceAnyEditParams{}, fmt.Errorf("failed to get potential roots: %w", err)
 	}
 
-	file := uri.ToPath(l.clientIdentifier, fileURL)
-	baseDir := util.FindClosestMatchingRoot(file, roots)
+	f := fixer.NewFixer()
+	f.RegisterRoots(roots...)
+	f.RegisterFixes(fix)
+	// the default for the LSP is to rename on conflict
+	f.SetOnConflictOperation(fixer.OnConflictRename)
 
-	results, err := fix.Fix(
-		&fixes.FixCandidate{
-			Filename: file,
-			Contents: rutil.StringToByteSlice(contents),
-		},
-		&fixes.RuntimeOptions{
-			Config:  l.getLoadedConfig(),
-			BaseDir: baseDir,
+	violations := []report.Violation{
+		{
+			Title: fix.Name(),
+			Location: report.Location{
+				File: uri.ToPath(l.clientIdentifier, fileURL),
+			},
 		},
-	)
+	}
+
+	cfp := fileprovider.NewCacheFileProvider(l.cache, l.clientIdentifier)
+
+	fixReport, err := f.FixViolations(violations, cfp, l.getLoadedConfig())
 	if err != nil {
-		return types.ApplyWorkspaceRenameEditParams{}, fmt.Errorf("failed attempted fix: %w", err)
+		return result, fmt.Errorf("failed to fix violations: %w", err)
 	}
 
-	if len(results) == 0 {
-		return types.ApplyWorkspaceRenameEditParams{
+	if fixReport.HasConflicts() {
+		return result, errors.New("fix report has conflicts")
+	}
+
+	ff := fixReport.FixedFiles()
+
+	if len(ff) == 0 {
+		return types.ApplyWorkspaceAnyEditParams{
 			Label: label,
-			Edit:  types.WorkspaceRenameEdit{},
+			Edit:  types.WorkspaceAnyEdit{},
 		}, nil
 	}
 
-	result := results[0]
+	// find the new file and the old location
+	var fixedFile, oldFile string
+
+	var found bool
+
+	for _, f := range ff {
+		var ok bool
+
+		oldFile, ok = fixReport.OldPathForFile(f)
+		if ok {
+			fixedFile = f
+			found = true
+
+			break
+		}
+	}
+
+	if !found {
+		return types.ApplyWorkspaceAnyEditParams{
+			Label: label,
+			Edit:  types.WorkspaceAnyEdit{},
+		}, errors.New("failed to find fixed file's old location")
+	}
+
+	changes := make([]any, 0)
+
+	oldURI := uri.FromPath(l.clientIdentifier, oldFile)
+	newURI := uri.FromPath(l.clientIdentifier, fixedFile)
+	changes = append(changes, types.RenameFile{
+		Kind:   "rename",
+		OldURI: oldURI,
+		NewURI: newURI,
+		Options: &types.RenameFileOptions{
+			Overwrite:      false,
+			IgnoreIfExists: false,
+		},
+	})
 
-	renameEdit := types.WorkspaceRenameEdit{
-		DocumentChanges: []types.RenameFile{
-			{
-				Kind:   "rename",
-				OldURI: uri.FromPath(l.clientIdentifier, result.Rename.FromPath),
-				NewURI: uri.FromPath(l.clientIdentifier, result.Rename.ToPath),
-				Options: &types.RenameFileOptions{
-					Overwrite:      false,
-					IgnoreIfExists: false,
+	// are there old dirs?
+	dirs, err := util.DirCleanUpPaths(
+		uri.ToPath(l.clientIdentifier, oldURI),
+		[]string{
+			// stop at the root
+			l.workspacePath(),
+			// also preserve any dirs needed for the new file
+			uri.ToPath(l.clientIdentifier, newURI),
+		},
+	)
+	if err != nil {
+		return types.ApplyWorkspaceAnyEditParams{}, fmt.Errorf("failed to determine empty directories post rename: %w", err)
+	}
+
+	for _, dir := range dirs {
+		changes = append(
+			changes,
+			types.DeleteFile{
+				Kind: "delete",
+				URI:  uri.FromPath(l.clientIdentifier, dir),
+				Options: &types.DeleteFileOptions{
+					Recursive:         true,
+					IgnoreIfNotExists: true,
 				},
 			},
-		},
+		)
 	}
 
-	return types.ApplyWorkspaceRenameEditParams{
+	l.cache.Delete(oldURI)
+
+	return types.ApplyWorkspaceAnyEditParams{
 		Label: label,
-		Edit:  renameEdit,
+		Edit: types.WorkspaceAnyEdit{
+			DocumentChanges: changes,
+		},
 	}, nil
 }
 
@@ -1686,6 +1702,16 @@ func (l *LanguageServer) handleTextDocumentInlayHint(
 		return partialInlayHints(parseErrors, contents, params.TextDocument.URI, bis), nil
 	}
 
+	// file is blank, nothing to do
+	if contents, ok := l.cache.GetFileContents(params.TextDocument.URI); ok && contents == "" {
+		return []types.InlayHint{}, nil
+	}
+
+	// file could not be parsed, nothing to do
+	if errors, ok := l.cache.GetParseErrors(params.TextDocument.URI); ok && len(errors) > 0 {
+		return []types.InlayHint{}, nil
+	}
+
 	module, ok := l.cache.GetModule(params.TextDocument.URI)
 	if !ok {
 		l.logf(log.LevelMessage, "failed to get inlay hint: no parsed module for uri %q", params.TextDocument.URI)