diff --git a/.golangci.yaml b/.golangci.yaml index 85ac9ca6..bda034a9 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -18,6 +18,7 @@ linters: - rowserrcheck - wastedassign # annoying + - gocognit - goerr113 - varnamelen - nonamedreturns diff --git a/cmd/lint.go b/cmd/lint.go index ebe89203..11ecacbb 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -183,7 +183,6 @@ func init() { RootCommand.AddCommand(lintCommand) } -//nolint:gocognit func lint(args []string, params *lintCommandParams) (report.Report, error) { var err error diff --git a/internal/lsp/documentsymbol_test.go b/internal/lsp/documentsymbol_test.go index 48b74a6f..1c263753 100644 --- a/internal/lsp/documentsymbol_test.go +++ b/internal/lsp/documentsymbol_test.go @@ -83,7 +83,7 @@ func TestRefToString(t *testing.T) { } } -//nolint:gocognit,nestif +//nolint:nestif func TestDocumentSymbols(t *testing.T) { t.Parallel() diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 52477d35..c730caf0 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -84,6 +84,7 @@ type LanguageServer struct { type fileUpdateEvent struct { Reason string URI string + OldURI string Content string } @@ -173,6 +174,25 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { case <-ctx.Done(): return case evt := <-l.diagnosticRequestFile: + // if file has been deleted, clear diagnostics in the client + if evt.Reason == "textDocument/didDelete" { + err := l.sendFileDiagnostics(ctx, evt.URI) + if err != nil { + l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + } + + continue + } + + if evt.Reason == "textDocument/didRename" && evt.OldURI != "" { + err := l.sendFileDiagnostics(ctx, evt.OldURI) + if err != nil { + l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + } + + continue + } + // if there is new content, we need to update the parse errors or module first success, err := l.processTextContentUpdate(ctx, evt.URI, evt.Content) if err != nil { @@ -226,7 +246,7 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { case <-ctx.Done(): return case evt := <-l.builtinsPositionFile: - _, err := l.processBuiltinsUpdate(ctx, evt.URI, evt.Content) + err := l.processBuiltinsUpdate(ctx, evt.URI, evt.Content) if err != nil { l.logError(fmt.Errorf("failed to process builtin positions update: %w", err)) } @@ -429,25 +449,26 @@ func (l *LanguageServer) processTextContentUpdate( return false, nil } -func (l *LanguageServer) processBuiltinsUpdate( - _ context.Context, - uri string, - content string, -) (bool, error) { +func (l *LanguageServer) processBuiltinsUpdate(_ context.Context, uri string, content string) error { + if _, ok := l.cache.GetFileContents(uri); !ok { + // If the file is not in the cache, exit early or else + // we might accidentally put it in the cache after it's been + // deleted: https://github.com/StyraInc/regal/issues/679 + return nil + } + l.cache.SetFileContents(uri, content) success, err := updateParse(l.cache, uri) if err != nil { - return false, fmt.Errorf("failed to update parse: %w", err) + return fmt.Errorf("failed to update parse: %w", err) } if !success { - return false, nil + return nil } - err = updateBuiltinPositions(l.cache, uri) - - return err == nil, err + return updateBuiltinPositions(l.cache, uri) } func (l *LanguageServer) logError(err error) { @@ -873,7 +894,6 @@ func (l *LanguageServer) handleWorkspaceDidDeleteFiles( } l.diagnosticRequestFile <- evt - l.builtinsPositionFile <- evt } return struct{}{}, nil @@ -904,6 +924,7 @@ func (l *LanguageServer) handleWorkspaceDidRenameFiles( evt := fileUpdateEvent{ Reason: "textDocument/didRename", URI: renameOp.NewURI, + OldURI: renameOp.OldURI, Content: content, } diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index a6e7ec2c..abdb3390 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -48,7 +48,7 @@ const fileURIScheme = "file://" // TestLanguageServerSingleFile tests that changes to a single file and Regal config are handled correctly by the // language server my making updates to both and validating that the correct diagnostics are sent to the client. // -//nolint:gocognit,gocyclo,maintidx +//nolint:gocyclo,maintidx func TestLanguageServerSingleFile(t *testing.T) { t.Parallel() @@ -268,7 +268,7 @@ rules: // workspace diagnostics are run, this test validates that the correct diagnostics are sent to the client in this // scenario. // -//nolint:gocognit,maintidx,gocyclo +// nolint:maintidx,gocyclo func TestLanguageServerMultipleFiles(t *testing.T) { t.Parallel() @@ -509,6 +509,33 @@ allow if input.user in admins.users } } +// https://github.com/StyraInc/regal/issues/679 +func TestProcessBuiltinUpdateExitsOnMissingFile(t *testing.T) { + t.Parallel() + + ls := LanguageServer{ + cache: NewCache(), + } + + err := ls.processBuiltinsUpdate(context.Background(), "file://missing.rego", "foo") + if err != nil { + t.Fatal(err) + } + + if len(ls.cache.builtinPositionsFile) != 0 { + t.Errorf("expected builtin positions to be empty, got %v", ls.cache.builtinPositionsFile) + } + + contents, ok := ls.cache.GetFileContents("file://missing.rego") + if ok { + t.Errorf("expected file contents to be empty, got %s", contents) + } + + if len(ls.cache.GetAllFiles()) != 0 { + t.Errorf("expected files to be empty, got %v", ls.cache.GetAllFiles()) + } +} + // TestLanguageServerParentDirConfig tests that regal config is loaded as it is for the // Regal CLI, and that config files in a parent directory are loaded correctly // even when the workspace is a child directory. diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go index d4d3aa32..d59e8a8f 100644 --- a/pkg/linter/linter.go +++ b/pkg/linter/linter.go @@ -655,7 +655,6 @@ func loadModulesFromCustomRuleFS(customRuleFS fs.FS, rootPath string) (map[strin return files, nil } -//nolint:gocognit func (l Linter) lintWithRegoRules(ctx context.Context, input rules.Input) (report.Report, error) { l.startTimer(regalmetrics.RegalLintRego) defer l.stopTimer(regalmetrics.RegalLintRego)