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.

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

On parse failure, keep the old AST #66

Merged
merged 1 commit into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion pkg/server/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import (
type document struct {
// From DidOpen and DidChange
item protocol.TextDocumentItem
ast ast.Node

// Contains the last succesfully parsed AST. If doc.err is not nil, it's out of date.
ast ast.Node
linesChangedSinceAST map[int]bool

// From diagnostics
val string
Expand Down
6 changes: 5 additions & 1 deletion pkg/server/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,12 @@ func (s *server) definitionLink(ctx context.Context, params *protocol.Definition
return nil, utils.LogErrorf("Definition: %s: %w", errorRetrievingDocument, err)
}

// Only find definitions, if the the line we're trying to find a definition for hasn't changed since last succesful AST parse
if doc.ast == nil {
return nil, utils.LogErrorf("Definition: %s", errorParsingDocument)
return nil, utils.LogErrorf("Definition: document was never successfully parsed, can't find definitions")
}
if doc.linesChangedSinceAST[int(params.Position.Line)] {
return nil, utils.LogErrorf("Definition: document line %d was changed since last successful parse, can't find definitions", params.Position.Line)
}

vm, err := s.getVM(doc.item.URI.SpanURI().Filename())
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (s *server) Hover(ctx context.Context, params *protocol.HoverParams) (*prot
return nil, utils.LogErrorf("Hover: %s: %w", errorRetrievingDocument, err)
}

if doc.ast == nil {
if doc.err != nil {
// Hover triggers often. Throwing an error on each request is noisy
log.Errorf("Hover: %s", errorParsingDocument)
return nil, nil
Expand Down
31 changes: 24 additions & 7 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package server
import (
"context"
"path/filepath"
"strings"

"github.com/google/go-jsonnet"
"github.com/google/go-jsonnet/ast"
"github.com/grafana/jsonnet-language-server/pkg/stdlib"
"github.com/grafana/jsonnet-language-server/pkg/utils"
tankaJsonnet "github.com/grafana/tanka/pkg/jsonnet"
Expand Down Expand Up @@ -73,10 +75,28 @@ func (s *server) DidChange(ctx context.Context, params *protocol.DidChangeTextDo
}

if params.TextDocument.Version > doc.item.Version && len(params.ContentChanges) != 0 {
oldText := doc.item.Text
doc.item.Text = params.ContentChanges[len(params.ContentChanges)-1].Text
doc.ast, doc.err = jsonnet.SnippetToAST(doc.item.URI.SpanURI().Filename(), doc.item.Text)
if doc.err != nil {
return s.cache.put(doc)

var ast ast.Node
ast, doc.err = jsonnet.SnippetToAST(doc.item.URI.SpanURI().Filename(), doc.item.Text)

// If the AST parsed correctly, set it on the document
// Otherwise, keep the old AST, and find all the lines that have changed since last AST
if ast != nil {
doc.ast = ast
doc.linesChangedSinceAST = map[int]bool{}
} else {
splitOldText := strings.Split(oldText, "\n")
splitNewText := strings.Split(doc.item.Text, "\n")
for index, oldLine := range splitOldText {
if index >= len(splitNewText) {
doc.linesChangedSinceAST[index] = true
}
if oldLine != splitNewText[index] {
doc.linesChangedSinceAST[index] = true
}
}
}
}
return nil
Expand All @@ -85,12 +105,9 @@ func (s *server) DidChange(ctx context.Context, params *protocol.DidChangeTextDo
func (s *server) DidOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) (err error) {
defer s.queueDiagnostics(params.TextDocument.URI)

doc := &document{item: params.TextDocument}
doc := &document{item: params.TextDocument, linesChangedSinceAST: map[int]bool{}}
if params.TextDocument.Text != "" {
doc.ast, doc.err = jsonnet.SnippetToAST(params.TextDocument.URI.SpanURI().Filename(), params.TextDocument.Text)
if doc.err != nil {
return s.cache.put(doc)
}
}
return s.cache.put(doc)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/symbols.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (s *server) DocumentSymbol(ctx context.Context, params *protocol.DocumentSy
return nil, utils.LogErrorf("DocumentSymbol: %s: %w", errorRetrievingDocument, err)
}

if doc.ast == nil {
if doc.err != nil {
// Returning an error too often can lead to the client killing the language server
// Logging the errors is sufficient
log.Errorf("DocumentSymbol: %s", errorParsingDocument)
Expand Down