diff --git a/cmd/languageserver.go b/cmd/languageserver.go index c0f945ab..5f180b62 100644 --- a/cmd/languageserver.go +++ b/cmd/languageserver.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/styrainc/regal/internal/lsp" + "github.com/styrainc/regal/internal/lsp/log" ) func init() { @@ -25,7 +26,8 @@ func init() { defer cancel() opts := &lsp.LanguageServerOptions{ - ErrorLog: os.Stderr, + LogWriter: os.Stderr, + LogLevel: log.LevelMessage, } ls := lsp.NewLanguageServer(ctx, opts) diff --git a/internal/lsp/config/watcher.go b/internal/lsp/config/watcher.go index fd8e1431..47fec7cb 100644 --- a/internal/lsp/config/watcher.go +++ b/internal/lsp/config/watcher.go @@ -3,16 +3,17 @@ package config import ( "context" "fmt" - "io" "sync" "github.com/fsnotify/fsnotify" + + "github.com/styrainc/regal/internal/lsp/log" ) type Watcher struct { - errorWriter io.Writer - Reload chan string - Drop chan struct{} + logFunc func(log.Level, string, ...any) + Reload chan string + Drop chan struct{} pathUpdates chan string @@ -23,8 +24,8 @@ type Watcher struct { } type WatcherOpts struct { - ErrorWriter io.Writer - Path string + LogFunc func(log.Level, string, ...any) + Path string } func NewWatcher(opts *WatcherOpts) *Watcher { @@ -35,7 +36,7 @@ func NewWatcher(opts *WatcherOpts) *Watcher { } if opts != nil { - w.errorWriter = opts.ErrorWriter + w.logFunc = opts.LogFunc w.path = opts.Path } @@ -70,12 +71,12 @@ func (w *Watcher) loop(ctx context.Context) { if w.path != "" { err := w.fsWatcher.Remove(w.path) if err != nil { - fmt.Fprintf(w.errorWriter, "failed to remove existing watch: %v\n", err) + w.logFunc(log.LevelMessage, "failed to remove existing watch: %v\n", err) } } if err := w.fsWatcher.Add(path); err != nil { - fmt.Fprintf(w.errorWriter, "failed to add watch: %v\n", err) + w.logFunc(log.LevelDebug, "failed to add watch: %v\n", err) } w.path = path @@ -84,7 +85,7 @@ func (w *Watcher) loop(ctx context.Context) { w.Reload <- path case event, ok := <-w.fsWatcher.Events: if !ok { - fmt.Fprintf(w.errorWriter, "config watcher event channel closed\n") + w.logFunc(log.LevelMessage, "config watcher event channel closed\n") return } @@ -98,10 +99,10 @@ func (w *Watcher) loop(ctx context.Context) { w.Drop <- struct{}{} } case err := <-w.fsWatcher.Errors: - fmt.Fprintf(w.errorWriter, "config watcher error: %v\n", err) + w.logFunc(log.LevelMessage, "config watcher error: %v\n", err) case <-ctx.Done(): if err := w.Stop(); err != nil { - fmt.Fprintf(w.errorWriter, "failed to stop watcher: %v\n", err) + w.logFunc(log.LevelMessage, "failed to stop watcher: %v\n", err) } return diff --git a/internal/lsp/config/watcher_test.go b/internal/lsp/config/watcher_test.go index 2b925440..4790317f 100644 --- a/internal/lsp/config/watcher_test.go +++ b/internal/lsp/config/watcher_test.go @@ -5,6 +5,8 @@ import ( "os" "testing" "time" + + "github.com/styrainc/regal/internal/lsp/log" ) func TestWatcher(t *testing.T) { @@ -22,7 +24,9 @@ foo: bar t.Fatal(err) } - watcher := NewWatcher(&WatcherOpts{ErrorWriter: os.Stderr}) + watcher := NewWatcher(&WatcherOpts{LogFunc: func(l log.Level, s string, a ...any) { + t.Logf(l.String()+": "+s, a...) + }}) ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/internal/lsp/eval.go b/internal/lsp/eval.go index b5fc8bb3..d66ebab7 100644 --- a/internal/lsp/eval.go +++ b/internal/lsp/eval.go @@ -15,6 +15,7 @@ import ( "github.com/open-policy-agent/opa/topdown/print" rbundle "github.com/styrainc/regal/bundle" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/internal/lsp/uri" "github.com/styrainc/regal/pkg/builtins" ) @@ -47,7 +48,7 @@ func (l *LanguageServer) Eval( for k, v := range dataBundles { if v.Manifest.Roots == nil { - l.logError(fmt.Errorf("bundle %s has no roots and will be skipped", k)) + l.logf(log.LevelMessage, "bundle %s has no roots and will be skipped", k) continue } diff --git a/internal/lsp/eval_test.go b/internal/lsp/eval_test.go index 909da62f..aa3ddeb8 100644 --- a/internal/lsp/eval_test.go +++ b/internal/lsp/eval_test.go @@ -8,13 +8,19 @@ import ( "testing" rio "github.com/styrainc/regal/internal/io" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/internal/parse" ) func TestEvalWorkspacePath(t *testing.T) { t.Parallel() - ls := NewLanguageServer(context.Background(), &LanguageServerOptions{ErrorLog: os.Stderr}) + logger := newTestLogger(t) + + ls := NewLanguageServer( + context.Background(), + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) policy1 := `package policy1 diff --git a/internal/lsp/log/level.go b/internal/lsp/log/level.go new file mode 100644 index 00000000..2c49827f --- /dev/null +++ b/internal/lsp/log/level.go @@ -0,0 +1,33 @@ +package log + +// Level controls the level of server logging and corresponds to TraceValue in +// the LSP spec: +// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#traceValue. +type Level int + +const ( + // LevelOff is used to disable logging completely. + LevelOff Level = iota + // LogLevelMessage are intended to contain errors and other messages that + // should be shown in normal operation. + LevelMessage + // LogLevelDebug is includes LogLevelMessage, but also information that is + // not expected to be useful unless debugging the server. + LevelDebug +) + +func (l Level) String() string { + return [...]string{"Off", "Messages", "Debug"}[l] +} + +func (l Level) ShouldLog(incoming Level) bool { + if l == LevelOff { + return false + } + + if l == LevelDebug { + return true + } + + return l == LevelMessage && incoming == LevelMessage +} diff --git a/internal/lsp/log/level_test.go b/internal/lsp/log/level_test.go new file mode 100644 index 00000000..3f4d6698 --- /dev/null +++ b/internal/lsp/log/level_test.go @@ -0,0 +1,49 @@ +package log + +import "testing" + +func TestShouldLog(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + Level Level + IncomingMessageLevel Level + ExpectLog bool + }{ + "Off, no logs": { + Level: LevelOff, + IncomingMessageLevel: LevelMessage, + ExpectLog: false, + }, + "Off, no logs from debug": { + Level: LevelOff, + IncomingMessageLevel: LevelDebug, + ExpectLog: false, + }, + "Message, no logs from debug": { + Level: LevelMessage, + IncomingMessageLevel: LevelDebug, + ExpectLog: false, + }, + "Debug, logs from Message": { + Level: LevelDebug, + IncomingMessageLevel: LevelMessage, + ExpectLog: true, + }, + "Debug, all logs": { + Level: LevelDebug, + IncomingMessageLevel: LevelDebug, + ExpectLog: true, + }, + } + + for label, tc := range testCases { + t.Run(label, func(t *testing.T) { + t.Parallel() + + if got := tc.Level.ShouldLog(tc.IncomingMessageLevel); got != tc.ExpectLog { + t.Errorf("expected %v, got %v", tc.ExpectLog, got) + } + }) + } +} diff --git a/internal/lsp/server.go b/internal/lsp/server.go index c2c7476d..af04d966 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -36,6 +36,7 @@ import ( lsconfig "github.com/styrainc/regal/internal/lsp/config" "github.com/styrainc/regal/internal/lsp/examples" "github.com/styrainc/regal/internal/lsp/hover" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/internal/lsp/opa/oracle" "github.com/styrainc/regal/internal/lsp/rego" "github.com/styrainc/regal/internal/lsp/types" @@ -64,7 +65,13 @@ const ( ) type LanguageServerOptions struct { - ErrorLog io.Writer + // LogWriter is the io.Writer where all logged messages will be written. + LogWriter io.Writer + + // log.Level controls the verbosity of the logs, with log.LevelOff, no messages + // are logged, log.LevelMessage logs only messages and errors, and log.LevelDebug + // Logs all messages. + LogLevel log.Level // WorkspaceDiagnosticsPoll, if set > 0 will cause a full workspace lint // to run on this interval. This is intended to be used where eventing @@ -78,16 +85,18 @@ func NewLanguageServer(ctx context.Context, opts *LanguageServerOptions) *Langua c := cache.NewCache() store := NewRegalStore() - ls := &LanguageServer{ + var ls *LanguageServer + ls = &LanguageServer{ cache: c, regoStore: store, - errorLog: opts.ErrorLog, + logWriter: opts.LogWriter, + logLevel: opts.LogLevel, lintFileJobs: make(chan lintFileJob, 10), lintWorkspaceJobs: make(chan lintWorkspaceJob, 10), builtinsPositionJobs: make(chan lintFileJob, 10), commandRequest: make(chan types.ExecuteCommandParams, 10), templateFileJobs: make(chan lintFileJob, 10), - configWatcher: lsconfig.NewWatcher(&lsconfig.WatcherOpts{ErrorWriter: opts.ErrorLog}), + configWatcher: lsconfig.NewWatcher(&lsconfig.WatcherOpts{LogFunc: ls.logf}), completionsManager: completions.NewDefaultManager(ctx, c, store), webServer: web.NewServer(c), loadedBuiltins: make(map[string]map[string]*ast.Builtin), @@ -98,7 +107,8 @@ func NewLanguageServer(ctx context.Context, opts *LanguageServerOptions) *Langua } type LanguageServer struct { - errorLog io.Writer + logWriter io.Writer + logLevel log.Level regoStore storage.Store conn *jsonrpc2.Conn @@ -157,6 +167,8 @@ func (l *LanguageServer) Handle( conn *jsonrpc2.Conn, req *jsonrpc2.Request, ) (result any, err error) { + l.logf(log.LevelDebug, "received request: %s", req.Method) + // null params are allowed, but only for certain methods if req.Params == nil && req.Method != "shutdown" && req.Method != "exit" { return nil, &jsonrpc2.Error{Code: jsonrpc2.CodeInvalidParams} @@ -250,12 +262,13 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { case <-ctx.Done(): return case job := <-l.lintFileJobs: + l.logf(log.LevelDebug, "linting file %s (%s)", job.URI, job.Reason) bis := l.builtinsForCurrentCapabilities() // updateParse will not return an error when the parsing failed, // but only when it was impossible if _, err := updateParse(ctx, l.cache, l.regoStore, job.URI, bis); err != nil { - l.logError(fmt.Errorf("failed to update module for %s: %w", job.URI, err)) + l.logf(log.LevelMessage, "failed to update module for %s: %s", job.URI, err) continue } @@ -271,13 +284,13 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { // of non aggregate rules l.getEnabledNonAggregateRules(), ); err != nil { - l.logError(fmt.Errorf("failed to update file diagnostics: %w", err)) + l.logf(log.LevelMessage, "failed to update file diagnostics: %s", err) continue } if err := l.sendFileDiagnostics(ctx, job.URI); err != nil { - l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + l.logf(log.LevelMessage, "failed to send diagnostic: %s", err) continue } @@ -292,6 +305,8 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { // any other rules globally other than aggregate rules. AggregateReportOnly: true, } + + l.logf(log.LevelDebug, "linting file %s done", job.URI) } } }() @@ -314,7 +329,7 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { // frequently, we stop adding to the channel if there already // jobs set to preserve performance if job.AggregateReportOnly && len(workspaceLintRuns) > workspaceLintRunBufferSize/2 { - fmt.Fprintln(l.errorLog, "rate limiting aggregate reports") + l.log(log.LevelDebug, "rate limiting aggregate reports") continue } @@ -356,6 +371,8 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { case <-ctx.Done(): return case job := <-workspaceLintRuns: + l.logf(log.LevelDebug, "linting workspace: %#v", job) + // if there are no parsed modules in the cache, then there is // no need to run the aggregate report. This can happen if the // server is very slow to start up. @@ -380,14 +397,16 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { targetRules, ) if err != nil { - l.logError(fmt.Errorf("failed to update all diagnostics: %w", err)) + l.logf(log.LevelMessage, "failed to update all diagnostics: %s", err) } for fileURI := range l.cache.GetAllFiles() { if err := l.sendFileDiagnostics(ctx, fileURI); err != nil { - l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + l.logf(log.LevelMessage, "failed to send diagnostic: %s", err) } } + + l.log(log.LevelDebug, "linting workspace done") } } }() @@ -419,7 +438,7 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { success, err := updateParse(ctx, l.cache, l.regoStore, fileURI, bis) if err != nil { - l.logError(fmt.Errorf("failed to update parse: %w", err)) + l.logf(log.LevelMessage, "failed to update parse: %s", err) continue } @@ -429,13 +448,13 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { } if err = hover.UpdateBuiltinPositions(l.cache, fileURI, bis); err != nil { - l.logError(fmt.Errorf("failed to update builtin positions: %w", err)) + l.logf(log.LevelMessage, "failed to update builtin positions: %s", err) continue } if err = hover.UpdateKeywordLocations(ctx, l.cache, fileURI); err != nil { - l.logError(fmt.Errorf("failed to update keyword positions: %w", err)) + l.logf(log.LevelMessage, "failed to update keyword positions: %s", err) continue } @@ -496,7 +515,7 @@ func (l *LanguageServer) loadEnabledRulesFromConfig(ctx context.Context, cfg con func (l *LanguageServer) StartConfigWorker(ctx context.Context) { if err := l.configWatcher.Start(ctx); err != nil { - l.logError(fmt.Errorf("failed to start config watcher: %w", err)) + l.logf(log.LevelMessage, "failed to start config watcher: %s", err) return } @@ -508,7 +527,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { case path := <-l.configWatcher.Reload: configFileBs, err := os.ReadFile(path) if err != nil { - l.logError(fmt.Errorf("failed to open config file: %w", err)) + l.logf(log.LevelMessage, "failed to open config file: %s", err) continue } @@ -517,14 +536,14 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { // EOF errors are ignored here as then we just use the default config if err = yaml.Unmarshal(configFileBs, &userConfig); err != nil && !errors.Is(err, io.EOF) { - l.logError(fmt.Errorf("failed to reload config: %w", err)) + l.logf(log.LevelMessage, "failed to reload config: %s", err) continue } mergedConfig, err := config.LoadConfigWithDefaultsFromBundle(&rbundle.LoadedBundle, &userConfig) if err != nil { - l.logError(fmt.Errorf("failed to load config: %w", err)) + l.logf(log.LevelMessage, "failed to load config: %s", err) continue } @@ -535,7 +554,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { err = l.loadEnabledRulesFromConfig(ctx, mergedConfig) if err != nil { - l.logError(fmt.Errorf("failed to cache enabled rules: %w", err)) + l.logf(log.LevelMessage, "failed to cache enabled rules: %s", err) } // Capabilities URL may have changed, so we should reload it. @@ -548,7 +567,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { caps, err := capabilities.Lookup(ctx, capsURL) if err != nil { - l.logError(fmt.Errorf("failed to load capabilities for URL %q: %w", capsURL, err)) + l.logf(log.LevelMessage, "failed to load capabilities for URL %q: %s", capsURL, err) continue } @@ -576,7 +595,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { } if err := RemoveFileMod(ctx, l.regoStore, k); err != nil { - l.logError(fmt.Errorf("failed to remove mod from store: %w", err)) + l.logf(log.LevelMessage, "failed to remove mod from store: %s", err) } } @@ -596,7 +615,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { // to start working right away without the need for a file content // update to run updateParse. if _, err = updateParse(ctx, l.cache, l.regoStore, k, bis); err != nil { - l.logError(fmt.Errorf("failed to update parse for previously ignored file %q: %w", k, err)) + l.logf(log.LevelMessage, "failed to update parse for previously ignored file %q: %s", k, err) } } @@ -679,7 +698,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai fileURL, ok := params.Arguments[0].(string) if !ok { - l.logError(fmt.Errorf("expected first argument to be a string, got %T", params.Arguments[0])) + l.logf(log.LevelMessage, "expected first argument to be a string, got %T", params.Arguments[0]) break } @@ -690,13 +709,13 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai fileURL, ) if err != nil { - l.logError(fmt.Errorf("failed to fix directory package mismatch: %w", err)) + l.logf(log.LevelMessage, "failed to fix directory package mismatch: %s", err) break } if err := l.conn.Call(ctx, methodWorkspaceApplyEdit, renameParams, nil); err != nil { - l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) + l.logf(log.LevelMessage, "failed %s notify: %v", methodWorkspaceApplyEdit, err.Error()) } // clean up any empty edits dirs left over @@ -704,7 +723,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai dir := filepath.Dir(uri.ToPath(l.clientIdentifier, renameParams.Edit.DocumentChanges[0].OldURI)) if err := util.DeleteEmptyDirs(dir); err != nil { - l.logError(fmt.Errorf("failed to delete empty directories: %w", err)) + l.logf(log.LevelMessage, "failed to delete empty directories: %s", err) } } @@ -712,21 +731,21 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai fixed = false case "regal.debug": if len(params.Arguments) != 3 { - l.logError(fmt.Errorf("expected three arguments, got %d", len(params.Arguments))) + l.logf(log.LevelMessage, "expected three arguments, got %d", len(params.Arguments)) break } file, ok := params.Arguments[0].(string) if !ok { - l.logError(fmt.Errorf("expected first argument to be a string, got %T", params.Arguments[0])) + l.logf(log.LevelMessage, "expected first argument to be a string, got %T", params.Arguments[0]) break } path, ok := params.Arguments[1].(string) if !ok { - l.logError(fmt.Errorf("expected second argument to be a string, got %T", params.Arguments[1])) + l.logf(log.LevelMessage, "expected second argument to be a string, got %T", params.Arguments[1]) break } @@ -747,53 +766,53 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai responseResult := map[string]any{} if err = l.conn.Call(ctx, "regal/startDebugging", responseParams, &responseResult); err != nil { - l.logError(fmt.Errorf("regal/startDebugging failed: %v", err.Error())) + l.logf(log.LevelMessage, "regal/startDebugging failed: %v", err.Error()) } case "regal.eval": if len(params.Arguments) != 3 { - l.logError(fmt.Errorf("expected three arguments, got %d", len(params.Arguments))) + l.logf(log.LevelMessage, "expected three arguments, got %d", len(params.Arguments)) break } file, ok := params.Arguments[0].(string) if !ok { - l.logError(fmt.Errorf("expected first argument to be a string, got %T", params.Arguments[0])) + l.logf(log.LevelMessage, "expected first argument to be a string, got %T", params.Arguments[0]) break } path, ok := params.Arguments[1].(string) if !ok { - l.logError(fmt.Errorf("expected second argument to be a string, got %T", params.Arguments[1])) + l.logf(log.LevelMessage, "expected second argument to be a string, got %T", params.Arguments[1]) break } line, ok := params.Arguments[2].(float64) if !ok { - l.logError(fmt.Errorf("expected third argument to be a number, got %T", params.Arguments[2])) + l.logf(log.LevelMessage, "expected third argument to be a number, got %T", params.Arguments[2]) break } currentModule, ok := l.cache.GetModule(file) if !ok { - l.logError(fmt.Errorf("failed to get module for file %q", file)) + l.logf(log.LevelMessage, "failed to get module for file %q", file) break } currentContents, ok := l.cache.GetFileContents(file) if !ok { - l.logError(fmt.Errorf("failed to get contents for file %q", file)) + l.logf(log.LevelMessage, "failed to get contents for file %q", file) break } allRuleHeadLocations, err := rego.AllRuleHeadLocations(ctx, filepath.Base(file), currentContents, currentModule) if err != nil { - l.logError(fmt.Errorf("failed to get rule head locations: %w", err)) + l.logf(log.LevelMessage, "failed to get rule head locations: %s", err) break } @@ -809,14 +828,14 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai if len(currentModule.Comments) > 0 && regalEvalUseAsInputComment.Match(currentModule.Comments[0].Text) { inputMap, err := rparse.PrepareAST(file, currentContents, currentModule) if err != nil { - l.logError(fmt.Errorf("failed to prepare module: %w", err)) + l.logf(log.LevelMessage, "failed to prepare module: %s", err) break } bs, err := encoding.JSON().Marshal(inputMap) if err != nil { - l.logError(fmt.Errorf("failed to marshal module: %w", err)) + l.logf(log.LevelMessage, "failed to marshal module: %s", err) break } @@ -837,7 +856,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai Type: 1, // error Message: cleanedMessage, }); err != nil { - l.logError(fmt.Errorf("failed to notify client of eval error: %w", err)) + l.logf(log.LevelMessage, "failed to notify client of eval error: %s", err) } break @@ -863,7 +882,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai responseResult := map[string]any{} if err = l.conn.Call(ctx, "regal/showEvalResult", responseParams, &responseResult); err != nil { - l.logError(fmt.Errorf("regal/showEvalResult failed: %v", err.Error())) + l.logf(log.LevelMessage, "regal/showEvalResult failed: %v", err.Error()) } } else { output := filepath.Join(l.workspacePath(), "output.json") @@ -896,13 +915,13 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai } if err != nil { - l.logError(err) + l.logf(log.LevelMessage, "command failed: %s", err) if err := l.conn.Notify(ctx, "window/showMessage", types.ShowMessageParams{ Type: 1, // error Message: err.Error(), }); err != nil { - l.logError(fmt.Errorf("failed to notify client of command error: %w", err)) + l.logf(log.LevelMessage, "failed to notify client of command error: %s", err) } break @@ -910,7 +929,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai if fixed { if err = l.conn.Call(ctx, methodWorkspaceApplyEdit, editParams, nil); err != nil { - l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) + l.logf(log.LevelMessage, "failed %s notify: %v", methodWorkspaceApplyEdit, err.Error()) } } } @@ -943,7 +962,7 @@ func (l *LanguageServer) StartWorkspaceStateWorker(ctx context.Context) { // then send the diagnostics message based on the cleared cache if err = l.sendFileDiagnostics(ctx, fileURI); err != nil { - l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + l.logf(log.LevelMessage, "failed to send diagnostic: %s", err) } } @@ -958,7 +977,7 @@ func (l *LanguageServer) StartWorkspaceStateWorker(ctx context.Context) { // on are not loaded from disk during editing. newURIs, err := l.loadWorkspaceContents(ctx, true) if err != nil { - l.logError(fmt.Errorf("failed to refresh workspace contents: %w", err)) + l.logf(log.LevelMessage, "failed to refresh workspace contents: %s", err) continue } @@ -990,7 +1009,7 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { // determine the new contents for the file, if permitted newContents, err := l.templateContentsForFile(job.URI) if err != nil { - l.logError(fmt.Errorf("failed to template new file: %w", err)) + l.logf(log.LevelMessage, "failed to template new file: %s", err) continue } @@ -1014,7 +1033,7 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { job.URI, ) if err != nil { - l.logError(fmt.Errorf("failed to fix directory package mismatch: %w", err)) + l.logf(log.LevelMessage, "failed to fix directory package mismatch: %s", err) continue } @@ -1038,7 +1057,7 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { }, ) if err != nil { - l.logError(fmt.Errorf("failed to delete empty directories: %w", err)) + l.logf(log.LevelMessage, "failed to delete empty directories: %s", err) continue } @@ -1066,7 +1085,7 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { "documentChanges": edits, }, }, nil); err != nil { - l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) + l.logf(log.LevelMessage, "failed %s notify: %v", methodWorkspaceApplyEdit, err.Error()) } // finally, trigger a diagnostics run for the new file @@ -1328,9 +1347,17 @@ func (l *LanguageServer) processHoverContentUpdate(ctx context.Context, fileURI return nil } -func (l *LanguageServer) logError(err error) { - if l.errorLog != nil { - fmt.Fprintf(l.errorLog, "ERROR: %s\n", err) +func (l *LanguageServer) logf(level log.Level, format string, args ...any) { + l.log(level, fmt.Sprintf(format, args...)) +} + +func (l *LanguageServer) log(level log.Level, message string) { + if !l.logLevel.ShouldLog(level) { + return + } + + if l.logWriter != nil { + fmt.Fprintln(l.logWriter, message) } } @@ -1400,7 +1427,7 @@ func (l *LanguageServer) handleTextDocumentHover( // when no builtins are found, we can't return a useful hover response. // log the error, but return an empty struct to avoid an error being shown in the client. if !ok { - l.logError(fmt.Errorf("could not get builtins for uri %q", params.TextDocument.URI)) + l.logf(log.LevelMessage, "could not get builtins for uri %q", params.TextDocument.URI) // return "null" as per the spec return nil, nil @@ -1619,7 +1646,7 @@ func (l *LanguageServer) handleTextDocumentInlayHint( module, ok := l.cache.GetModule(params.TextDocument.URI) if !ok { - l.logError(fmt.Errorf("failed to get inlay hint: no parsed module for uri %q", params.TextDocument.URI)) + l.logf(log.LevelMessage, "failed to get inlay hint: no parsed module for uri %q", params.TextDocument.URI) return []types.InlayHint{}, nil } @@ -1822,7 +1849,7 @@ func (l *LanguageServer) handleTextDocumentDefinition( return nil, nil } - l.logError(fmt.Errorf("failed to find definition: %w", err)) + l.logf(log.LevelMessage, "failed to find definition: %s", err) // return "null" as per the spec return nil, nil @@ -1954,7 +1981,7 @@ func (l *LanguageServer) handleTextDocumentDidSave( enabled, err := linter.NewLinter().WithUserConfig(*cfg).DetermineEnabledRules(ctx) if err != nil { - l.logError(fmt.Errorf("failed to determine enabled rules: %w", err)) + l.logf(log.LevelMessage, "failed to determine enabled rules: %s", err) return struct{}{}, nil } @@ -1976,7 +2003,7 @@ func (l *LanguageServer) handleTextDocumentDidSave( } if err := l.conn.Notify(ctx, "window/showMessage", resp); err != nil { - l.logError(fmt.Errorf("failed to notify: %w", err)) + l.logf(log.LevelMessage, "failed to notify: %s", err) return struct{}{}, nil } @@ -2002,7 +2029,7 @@ func (l *LanguageServer) handleTextDocumentDocumentSymbol( contents, ok := l.cache.GetFileContents(params.TextDocument.URI) if !ok { - l.logError(fmt.Errorf("failed to get file contents for uri %q", params.TextDocument.URI)) + l.logf(log.LevelMessage, "failed to get file contents for uri %q", params.TextDocument.URI) return []types.DocumentSymbol{}, nil } @@ -2118,7 +2145,7 @@ func (l *LanguageServer) handleTextDocumentFormatting( }, ) if err != nil { - l.logError(fmt.Errorf("failed to format file: %w", err)) + l.logf(log.LevelMessage, "failed to format file: %s", err) // return "null" as per the spec return nil, nil @@ -2241,7 +2268,7 @@ func (l *LanguageServer) handleWorkspaceDidDeleteFiles( l.cache.Delete(deleteOp.URI) if err := l.sendFileDiagnostics(ctx, deleteOp.URI); err != nil { - l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + l.logf(log.LevelMessage, "failed to send diagnostic: %s", err) } } @@ -2281,7 +2308,7 @@ func (l *LanguageServer) handleWorkspaceDidRenameFiles( l.cache.Delete(renameOp.OldURI) if err := l.sendFileDiagnostics(ctx, renameOp.OldURI); err != nil { - l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) + l.logf(log.LevelMessage, "failed to send diagnostic: %s", err) } l.cache.SetFileContents(renameOp.NewURI, content) @@ -2348,9 +2375,10 @@ func (l *LanguageServer) handleInitialize( l.clientIdentifier = clients.DetermineClientIdentifier(params.ClientInfo.Name) if l.clientIdentifier == clients.IdentifierGeneric { - l.logError( - fmt.Errorf("unable to match client identifier for initializing client, using generic functionality: %s", - params.ClientInfo.Name), + l.logf( + log.LevelMessage, + "unable to match client identifier for initializing client, using generic functionality: %s", + params.ClientInfo.Name, ) } @@ -2441,7 +2469,7 @@ func (l *LanguageServer) handleInitialize( err = l.loadEnabledRulesFromConfig(ctx, defaultConfig) if err != nil { - l.logError(fmt.Errorf("failed to cache enabled rules: %w", err)) + l.logf(log.LevelMessage, "failed to cache enabled rules: %s", err) } if l.workspaceRootURI != "" { @@ -2449,7 +2477,7 @@ func (l *LanguageServer) handleInitialize( l.bundleCache = bundles.NewCache(&bundles.CacheOptions{ WorkspacePath: workspaceRootPath, - ErrorLog: l.errorLog, + ErrorLog: l.logWriter, }) configFile, err := config.FindConfig(workspaceRootPath) diff --git a/internal/lsp/server_builtins_test.go b/internal/lsp/server_builtins_test.go index 98584b09..cc2e52ca 100644 --- a/internal/lsp/server_builtins_test.go +++ b/internal/lsp/server_builtins_test.go @@ -3,15 +3,20 @@ package lsp import ( "context" "testing" + + "github.com/styrainc/regal/internal/lsp/log" ) // https://github.com/StyraInc/regal/issues/679 func TestProcessBuiltinUpdateExitsOnMissingFile(t *testing.T) { t.Parallel() - ls := NewLanguageServer(context.Background(), &LanguageServerOptions{ - ErrorLog: newTestLogger(t), - }) + logger := newTestLogger(t) + + ls := NewLanguageServer( + context.Background(), + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) if err := ls.processHoverContentUpdate(context.Background(), "file://missing.rego", "foo"); err != nil { t.Fatal(err) diff --git a/internal/lsp/server_rename_test.go b/internal/lsp/server_rename_test.go index 04b02107..76001502 100644 --- a/internal/lsp/server_rename_test.go +++ b/internal/lsp/server_rename_test.go @@ -9,6 +9,7 @@ import ( "github.com/styrainc/regal/internal/lsp/cache" "github.com/styrainc/regal/internal/lsp/clients" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/pkg/config" "github.com/styrainc/regal/pkg/fixer/fixes" ) @@ -24,7 +25,13 @@ func TestLanguageServerFixRenameParams(t *testing.T) { ctx := context.Background() - l := NewLanguageServer(ctx, &LanguageServerOptions{ErrorLog: newTestLogger(t)}) + logger := newTestLogger(t) + + ls := NewLanguageServer( + ctx, + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) + c := cache.NewCache() f := &fixes.DirectoryPackageMismatch{} @@ -32,10 +39,10 @@ func TestLanguageServerFixRenameParams(t *testing.T) { c.SetFileContents(fileURL, "package authz.main.rules") - l.clientIdentifier = clients.IdentifierVSCode - l.workspaceRootURI = fmt.Sprintf("file://%s/workspace", tmpDir) - l.cache = c - l.loadedConfig = &config.Config{ + ls.clientIdentifier = clients.IdentifierVSCode + ls.workspaceRootURI = fmt.Sprintf("file://%s/workspace", tmpDir) + ls.cache = c + ls.loadedConfig = &config.Config{ Rules: map[string]config.Category{ "idiomatic": { "directory-package-mismatch": config.Rule{ @@ -48,7 +55,7 @@ func TestLanguageServerFixRenameParams(t *testing.T) { }, } - params, err := l.fixRenameParams("fix my file!", f, fileURL) + params, err := ls.fixRenameParams("fix my file!", f, fileURL) if err != nil { t.Fatalf("failed to fix rename params: %s", err) } diff --git a/internal/lsp/server_template_test.go b/internal/lsp/server_template_test.go index 17969094..41697c64 100644 --- a/internal/lsp/server_template_test.go +++ b/internal/lsp/server_template_test.go @@ -13,6 +13,7 @@ import ( "github.com/sourcegraph/jsonrpc2" "github.com/styrainc/regal/internal/lsp/clients" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/internal/lsp/types" "github.com/styrainc/regal/internal/lsp/uri" ) @@ -90,16 +91,20 @@ func TestTemplateContentsForFile(t *testing.T) { } } - ctx := context.Background() + logger := newTestLogger(t) - s := NewLanguageServer(ctx, &LanguageServerOptions{ErrorLog: newTestLogger(t)}) - s.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) + ls := NewLanguageServer( + context.Background(), + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) + + ls.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) fileURI := uri.FromPath(clients.IdentifierGeneric, filepath.Join(td, tc.FileKey)) - s.cache.SetFileContents(fileURI, tc.CacheFileContents) + ls.cache.SetFileContents(fileURI, tc.CacheFileContents) - newContents, err := s.templateContentsForFile(fileURI) + newContents, err := ls.templateContentsForFile(fileURI) if tc.ExpectedError != "" { if !strings.Contains(err.Error(), tc.ExpectedError) { t.Fatalf("expected error to contain %q, got %q", tc.ExpectedError, err) @@ -130,16 +135,20 @@ func TestTemplateContentsForFileInWorkspaceRoot(t *testing.T) { t.Fatalf("failed to create file %s: %s", filepath.Join(td, ".regal"), err) } - ctx := context.Background() + logger := newTestLogger(t) + + ls := NewLanguageServer( + context.Background(), + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) - s := NewLanguageServer(ctx, &LanguageServerOptions{ErrorLog: newTestLogger(t)}) - s.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) + ls.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) fileURI := uri.FromPath(clients.IdentifierGeneric, filepath.Join(td, "foo.rego")) - s.cache.SetFileContents(fileURI, "") + ls.cache.SetFileContents(fileURI, "") - _, err = s.templateContentsForFile(fileURI) + _, err = ls.templateContentsForFile(fileURI) if err == nil { t.Fatalf("expected error") } @@ -154,10 +163,14 @@ func TestTemplateContentsForFileWithUnknownRoot(t *testing.T) { td := t.TempDir() - ctx := context.Background() + logger := newTestLogger(t) + + ls := NewLanguageServer( + context.Background(), + &LanguageServerOptions{LogWriter: logger, LogLevel: log.LevelDebug}, + ) - s := NewLanguageServer(ctx, &LanguageServerOptions{ErrorLog: newTestLogger(t)}) - s.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) + ls.workspaceRootURI = uri.FromPath(clients.IdentifierGeneric, td) err := os.MkdirAll(filepath.Join(td, "foo"), 0o755) if err != nil { @@ -166,9 +179,9 @@ func TestTemplateContentsForFileWithUnknownRoot(t *testing.T) { fileURI := uri.FromPath(clients.IdentifierGeneric, filepath.Join(td, "foo/bar.rego")) - s.cache.SetFileContents(fileURI, "") + ls.cache.SetFileContents(fileURI, "") - newContents, err := s.templateContentsForFile(fileURI) + newContents, err := ls.templateContentsForFile(fileURI) if err != nil { t.Fatalf("unexpected error: %s", err) } diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index 58fdfa00..210a1888 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -18,6 +18,7 @@ import ( "github.com/anderseknert/roast/pkg/encoding" "github.com/sourcegraph/jsonrpc2" + "github.com/styrainc/regal/internal/lsp/log" "github.com/styrainc/regal/internal/lsp/types" "github.com/styrainc/regal/internal/util" ) @@ -119,7 +120,8 @@ func createAndInitServer( // set up the server and client connections ls := NewLanguageServer(ctx, &LanguageServerOptions{ - ErrorLog: logger, + LogWriter: logger, + LogLevel: log.LevelDebug, WorkspaceDiagnosticsPoll: pollingInterval, }) @@ -181,7 +183,7 @@ func createClientHandler( return func(_ context.Context, _ *jsonrpc2.Conn, req *jsonrpc2.Request) (result any, err error) { if req.Method != "textDocument/publishDiagnostics" { - fmt.Fprintln(logger, "unexpected request method:", req.Method) + fmt.Fprintln(logger, "createClientHandler: unexpected request method:", req.Method) return struct{}{}, nil } @@ -201,7 +203,7 @@ func createClientHandler( slices.Sort(violations) fileBase := filepath.Base(requestData.URI) - fmt.Fprintln(logger, "queue", fileBase, len(messages[fileBase])) + fmt.Fprintln(logger, "createClientHandler: queue", fileBase, len(messages[fileBase])) select { case messages[fileBase] <- violations: