Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: parse out word for errors and clear them #1163

Merged
merged 1 commit into from
Apr 3, 2024
Merged

fix: parse out word for errors and clear them #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions buildengine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ type schemaChange struct {
*schema.Module
}

type Listener interface {
OnBuildStarted(project Project)
}

type BuildStartedListenerFunc func(project Project)

func (b BuildStartedListenerFunc) OnBuildStarted(project Project) { b(project) }

// Engine for building a set of modules.
type Engine struct {
client ftlv1connect.ControllerServiceClient
Expand All @@ -39,6 +47,7 @@ type Engine struct {
schemaChanges *pubsub.Topic[schemaChange]
cancel func()
parallelism int
listener Listener
}

type Option func(o *Engine)
Expand All @@ -49,6 +58,13 @@ func Parallelism(n int) Option {
}
}

// WithListener sets the event listener for the Engine.
func WithListener(listener Listener) Option {
return func(o *Engine) {
o.listener = listener
}
}

// New constructs a new [Engine].
//
// Completely offline builds are possible if the full dependency graph is
Expand Down Expand Up @@ -457,6 +473,9 @@ func (e *Engine) build(ctx context.Context, key ProjectKey, builtModules map[str
}
sch := &schema.Schema{Modules: maps.Values(combined)}

if e.listener != nil {
e.listener.OnBuildStarted(project)
}
err := Build(ctx, sch, project)
if err != nil {
return err
Expand Down
10 changes: 9 additions & 1 deletion cmd/ftl/cmd_dev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"errors"

"time"

"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -60,14 +61,17 @@ func (d *devCmd) Run(ctx context.Context, projConfig projectconfig.Config) error
return err
}

opts := []buildengine.Option{buildengine.Parallelism(d.Parallelism)}
if d.RunLsp {
d.languageServer = lsp.NewServer(ctx)
opts = append(opts, buildengine.WithListener(buildengine.BuildStartedListenerFunc(d.OnBuildStarted)))
ctx = log.ContextWithLogger(ctx, log.FromContext(ctx).AddSink(lsp.NewLogSink(d.languageServer)))
g.Go(func() error {
return d.languageServer.Run()
})
}
engine, err := buildengine.New(ctx, client, d.Dirs, d.External, buildengine.Parallelism(d.Parallelism))

engine, err := buildengine.New(ctx, client, d.Dirs, d.External, opts...)
if err != nil {
return err
}
Expand All @@ -76,3 +80,7 @@ func (d *devCmd) Run(ctx context.Context, projConfig projectconfig.Config) error

return g.Wait()
}

func (d *devCmd) OnBuildStarted(project buildengine.Project) {
d.languageServer.BuildStarted(project.Config().Dir)
}
62 changes: 0 additions & 62 deletions lsp/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,71 +1,9 @@
package lsp

import (
"fmt"

"github.com/tliron/commonlog"

"github.com/TBD54566975/ftl/internal/log"
)

// GLSPLogger is a custom logger for the language server.
type GLSPLogger struct {
commonlog.Logger
}

func (l *GLSPLogger) Log(entry log.Entry) {
l.Logger.Log(toGLSPLevel(entry.Level), 10, entry.Message, entry.Attributes)
}

func (l *GLSPLogger) Logf(level log.Level, format string, args ...interface{}) {
l.Log(log.Entry{Level: level, Message: fmt.Sprintf(format, args...)})
}
func (l *GLSPLogger) Tracef(format string, args ...interface{}) {
l.Log(log.Entry{Level: log.Trace, Message: fmt.Sprintf(format, args...)})
}

func (l *GLSPLogger) Debugf(format string, args ...interface{}) {
l.Log(log.Entry{Level: log.Debug, Message: fmt.Sprintf(format, args...)})
}

func (l *GLSPLogger) Infof(format string, args ...interface{}) {
l.Log(log.Entry{Level: log.Info, Message: fmt.Sprintf(format, args...)})
}

func (l *GLSPLogger) Warnf(format string, args ...interface{}) {
l.Log(log.Entry{Level: log.Warn, Message: fmt.Sprintf(format, args...)})
}

func (l *GLSPLogger) Errorf(err error, format string, args ...interface{}) {
if err == nil {
return
}
l.Log(log.Entry{Level: log.Error, Message: fmt.Sprintf(format, args...) + ": " + err.Error(), Error: err})
}

var _ log.Interface = (*GLSPLogger)(nil)

func NewGLSPLogger(log commonlog.Logger) *GLSPLogger {
return &GLSPLogger{log}
}

func toGLSPLevel(l log.Level) commonlog.Level {
switch l {
case log.Trace:
return commonlog.Debug
case log.Debug:
return commonlog.Debug
case log.Info:
return commonlog.Info
case log.Warn:
return commonlog.Warning
case log.Error:
return commonlog.Error
default:
return commonlog.Debug
}
}

type LogSink struct {
server *Server
}
Expand Down
97 changes: 77 additions & 20 deletions lsp/lsp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package lsp

import (
"bufio"
"context"
"errors"
"fmt"
"os"
"strings"
"unicode"

"github.com/puzpuzpuz/xsync/v3"
_ "github.com/tliron/commonlog/simple"
"github.com/tliron/glsp"
protocol "github.com/tliron/glsp/protocol_3_16"
Expand All @@ -22,10 +26,10 @@ const lsName = "ftl-language-server"
// Server is a language server.
type Server struct {
server *glspServer.Server
glspLogger *GLSPLogger
glspContext *glsp.Context
handler protocol.Handler
logger log.Logger
diagnostics *xsync.MapOf[protocol.DocumentUri, []protocol.Diagnostic]
}

// NewServer creates a new language server.
Expand All @@ -38,9 +42,9 @@ func NewServer(ctx context.Context) *Server {
}
s := glspServer.NewServer(&handler, lsName, false)
server := &Server{
server: s,
glspLogger: NewGLSPLogger(s.Log),
logger: *log.FromContext(ctx),
server: s,
logger: *log.FromContext(ctx).Scope("lsp"),
diagnostics: xsync.NewMapOf[protocol.DocumentUri, []protocol.Diagnostic](),
}
handler.Initialize = server.initialize()
return server
Expand All @@ -56,6 +60,19 @@ func (s *Server) Run() error {

type errSet map[string]schema.Error

// BuildStarted clears diagnostics for the given directory. New errors will arrive later if they still exist.
func (s *Server) BuildStarted(dir string) {
dirURI := "file://" + dir

s.diagnostics.Range(func(uri protocol.DocumentUri, diagnostics []protocol.Diagnostic) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha!

if strings.HasPrefix(uri, dirURI) {
s.diagnostics.Delete(uri)
s.publishDiagnostics(uri, []protocol.Diagnostic{})
}
return true
})
}

// Post sends diagnostics to the client. err must be joined schema.Errors.
func (s *Server) post(err error) {
errByFilename := make(map[string]errSet)
Expand Down Expand Up @@ -83,26 +100,41 @@ func publishErrors(errByFilename map[string]errSet, s *Server) {
pp := e.Pos
sourceName := "ftl"
severity := protocol.DiagnosticSeverityError

length, err := getLineOrWordLength(filename, pp.Line, pp.Column, false)
if err != nil {
s.logger.Errorf(err, "Failed to get line or word length")
continue
}
endColumn := pp.Column + length

diagnostics = append(diagnostics, protocol.Diagnostic{
Range: protocol.Range{
Start: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(pp.Column - 1)},
End: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(pp.Column + 10 - 1)},
End: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(endColumn - 1)},
},
Severity: &severity,
Source: &sourceName,
Message: e.Msg,
})
}

if s.glspContext == nil {
return
}
uri := "file://" + filename
s.diagnostics.Store(uri, diagnostics)
s.publishDiagnostics(uri, diagnostics)
}
}

go s.glspContext.Notify(protocol.ServerTextDocumentPublishDiagnostics, protocol.PublishDiagnosticsParams{
URI: "file://" + filename,
Diagnostics: diagnostics,
})
func (s *Server) publishDiagnostics(uri protocol.DocumentUri, diagnostics []protocol.Diagnostic) {
s.logger.Debugf("Publishing diagnostics for %s\n", uri)
if s.glspContext == nil {
return
}

go s.glspContext.Notify(protocol.ServerTextDocumentPublishDiagnostics, protocol.PublishDiagnosticsParams{
URI: uri,
Diagnostics: diagnostics,
})
}

func (s *Server) initialize() protocol.InitializeFunc {
Expand All @@ -123,14 +155,6 @@ func (s *Server) initialize() protocol.InitializeFunc {
}, nil
}
}
func (s *Server) clearDiagnosticsOfDocument(uri protocol.DocumentUri) {
go func() {
go s.glspContext.Notify(protocol.ServerTextDocumentPublishDiagnostics, protocol.PublishDiagnosticsParams{
URI: uri,
Diagnostics: []protocol.Diagnostic{},
})
}()
}

func initialized(context *glsp.Context, params *protocol.InitializedParams) error {
return nil
Expand All @@ -149,3 +173,36 @@ func setTrace(context *glsp.Context, params *protocol.SetTraceParams) error {
protocol.SetTraceValue(params.Value)
return nil
}

// getLineOrWordLength returns the length of the line or the length of the word starting at the given column.
// If wholeLine is true, it returns the length of the entire line.
// If wholeLine is false, it returns the length of the word starting at the column.
func getLineOrWordLength(filePath string, lineNum, column int, wholeLine bool) (int, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is probably okay as a stop-gap, we should really be extending the error type to include range information so we can include it when the errors are created, as this is incredibly inefficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started down this path and got pretty hung up on parser stuff (I think that was the source of the issue). It was a bit of a time suck so I just put this in for now to keep moving forward.

I suspect it had something to do with how our schema.Position doesn't exactly match participle.Position but I'm fully just guessing. Once I added EndColumn to schema.Position I lost all position information. I do have a branch with that work though we can pick up when you have a sec :)

Copy link
Collaborator

@alecthomas alecthomas Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your assumption is correct: the Position type is a mirror of participles so you'll need to either add the end location to the error rather than the position, or include it in our position type and write a function to do the mapping.

file, err := os.Open(filePath)
if err != nil {
return 0, err
}
defer file.Close()

scanner := bufio.NewScanner(file)
currentLine := 1
for scanner.Scan() {
if currentLine == lineNum {
lineText := scanner.Text()
if wholeLine {
return len(lineText), nil
}
start := column - 1
end := start
for end < len(lineText) && !unicode.IsSpace(rune(lineText[end])) {
end++
}
return end - start, nil
}
currentLine++
}
if err := scanner.Err(); err != nil {
return 0, err
}
return 0, os.ErrNotExist
}