Skip to content

Fix publish diagnostic #57

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

Merged
merged 2 commits into from
Jan 28, 2021
Merged

Fix publish diagnostic #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21d9a7c
Use canonical paths everywhere
cmaglie Jan 28, 2021
37f3b38
Better handling of publishDiagnostics
cmaglie Jan 28, 2021
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
131 changes: 57 additions & 74 deletions handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type InoHandler struct {
sketchMapper *sourcemapper.InoMapper
sketchTrackedFilesCount int
docs map[string]*lsp.TextDocumentItem
inoDocsWithDiagnostics map[string]bool
inoDocsWithDiagnostics map[lsp.DocumentURI]bool

config lsp.BoardConfig
}
Expand Down Expand Up @@ -106,7 +106,7 @@ func (handler *InoHandler) waitClangdStart(msg string) {
func NewInoHandler(stdio io.ReadWriteCloser, board lsp.Board) *InoHandler {
handler := &InoHandler{
docs: map[string]*lsp.TextDocumentItem{},
inoDocsWithDiagnostics: map[string]bool{},
inoDocsWithDiagnostics: map[lsp.DocumentURI]bool{},
config: lsp.BoardConfig{
SelectedBoard: board,
},
Expand Down Expand Up @@ -731,7 +731,7 @@ func startClangd(compileCommandsDir, sketchCpp *paths.Path, compilers map[string
func (handler *InoHandler) didOpen(inoDidOpen *lsp.DidOpenTextDocumentParams) (*lsp.DidOpenTextDocumentParams, error) {
// Add the TextDocumentItem in the tracked files list
inoItem := inoDidOpen.TextDocument
handler.docs[inoItem.URI.Canonical()] = &inoItem
handler.docs[inoItem.URI.AsPath().String()] = &inoItem

// If we are tracking a .ino...
if inoItem.URI.Ext() == ".ino" {
Expand All @@ -752,8 +752,8 @@ func (handler *InoHandler) didOpen(inoDidOpen *lsp.DidOpenTextDocumentParams) (*

func (handler *InoHandler) didClose(inoDidClose *lsp.DidCloseTextDocumentParams) (*lsp.DidCloseTextDocumentParams, error) {
inoIdentifier := inoDidClose.TextDocument
if _, exist := handler.docs[inoIdentifier.URI.Canonical()]; exist {
delete(handler.docs, inoIdentifier.URI.Canonical())
if _, exist := handler.docs[inoIdentifier.URI.AsPath().String()]; exist {
delete(handler.docs, inoIdentifier.URI.AsPath().String())
} else {
log.Printf(" didClose of untracked document: %s", inoIdentifier.URI)
return nil, unknownURI(inoIdentifier.URI)
Expand Down Expand Up @@ -789,8 +789,9 @@ func (handler *InoHandler) ino2cppTextDocumentItem(inoItem lsp.TextDocumentItem)
cppItem.Version = handler.sketchMapper.CppText.Version
} else {
cppItem.LanguageID = inoItem.LanguageID
cppItem.Text = handler.docs[inoItem.URI.Canonical()].Text
cppItem.Version = handler.docs[inoItem.URI.Canonical()].Version
inoPath := inoItem.URI.AsPath().String()
cppItem.Text = handler.docs[inoPath].Text
cppItem.Version = handler.docs[inoPath].Version
}

return cppItem, nil
Expand All @@ -799,7 +800,7 @@ func (handler *InoHandler) ino2cppTextDocumentItem(inoItem lsp.TextDocumentItem)
func (handler *InoHandler) didChange(ctx context.Context, req *lsp.DidChangeTextDocumentParams) (*lsp.DidChangeTextDocumentParams, error) {
doc := req.TextDocument

trackedDoc, ok := handler.docs[doc.URI.Canonical()]
trackedDoc, ok := handler.docs[doc.URI.AsPath().String()]
if !ok {
return nil, unknownURI(doc.URI)
}
Expand Down Expand Up @@ -974,6 +975,10 @@ func (handler *InoHandler) inoDocumentURIFromInoPath(inoPath string) (lsp.Docume
}

func (handler *InoHandler) cpp2inoDocumentURI(cppURI lsp.DocumentURI, cppRange lsp.Range) (lsp.DocumentURI, lsp.Range, error) {
// TODO: Split this function into 2
// - Cpp2inoSketchDocumentURI: converts sketch (cppURI, cppRange) -> (inoURI, inoRange)
// - Cpp2inoDocumentURI : converts non-sketch (cppURI) -> (inoURI) [range is the same]

// Sketchbook/Sketch/Sketch.ino <- build-path/sketch/Sketch.ino.cpp
// Sketchbook/Sketch/AnotherTab.ino <- build-path/sketch/Sketch.ino.cpp (different section from above)
// Sketchbook/Sketch/AnotherFile.cpp <- build-path/sketch/AnotherFile.cpp (1:1)
Expand Down Expand Up @@ -1010,9 +1015,11 @@ func (handler *InoHandler) cpp2inoDocumentURI(cppURI lsp.DocumentURI, cppRange l

rel, err := handler.buildSketchRoot.RelTo(cppPath)
if err == nil {
inoPath := handler.sketchRoot.JoinPath(rel)
inoPath := handler.sketchRoot.JoinPath(rel).String()
log.Printf(" URI: '%s' -> '%s'", cppPath, inoPath)
return lsp.NewDocumentURIFromPath(inoPath), cppRange, nil
inoURI, err := handler.inoDocumentURIFromInoPath(inoPath)
log.Printf(" as URI: '%s'", inoURI)
return inoURI, cppRange, err
}

log.Printf(" could not determine rel-path of '%s' in '%s': %s", cppPath, handler.buildSketchRoot, err)
Expand Down Expand Up @@ -1405,7 +1412,7 @@ func (handler *InoHandler) cpp2inoTextEdit(cppURI lsp.DocumentURI, cppEdit lsp.T
}

func (handler *InoHandler) cpp2inoDocumentSymbols(cppSymbols []lsp.DocumentSymbol, inoRequestedURI lsp.DocumentURI) []lsp.DocumentSymbol {
inoRequested := inoRequestedURI.Canonical()
inoRequested := inoRequestedURI.AsPath().String()
log.Printf(" filtering for requested ino file: %s", inoRequested)
if inoRequestedURI.Ext() != ".ino" || len(cppSymbols) == 0 {
return cppSymbols
Expand Down Expand Up @@ -1474,46 +1481,68 @@ func (handler *InoHandler) cpp2inoSymbolInformation(syms []lsp.SymbolInformation
}

func (handler *InoHandler) cpp2inoDiagnostics(cppDiags *lsp.PublishDiagnosticsParams) ([]*lsp.PublishDiagnosticsParams, error) {
inoDiagsParam := map[lsp.DocumentURI]*lsp.PublishDiagnosticsParams{}

if len(cppDiags.Diagnostics) == 0 {
// If we receive the empty diagnostic on the preprocessed sketch,
// just return an empty diagnostic array.
if cppDiags.URI.AsPath().EquivalentTo(handler.buildSketchCpp) {
return []*lsp.PublishDiagnosticsParams{}, nil
}

inoURI, _, err := handler.cpp2inoDocumentURI(cppDiags.URI, lsp.NilRange)
return []*lsp.PublishDiagnosticsParams{
{
cppURI := cppDiags.URI
isSketch := cppURI.AsPath().EquivalentTo(handler.buildSketchCpp)
if isSketch {
for inoURI := range handler.inoDocsWithDiagnostics {
inoDiagsParam[inoURI] = &lsp.PublishDiagnosticsParams{
URI: inoURI,
Diagnostics: []lsp.Diagnostic{},
},
}, err
}
}
handler.inoDocsWithDiagnostics = map[lsp.DocumentURI]bool{}
} else {
inoURI, _, err := handler.cpp2inoDocumentURI(cppURI, lsp.NilRange)
if err != nil {
return nil, err
}
inoDiagsParam[inoURI] = &lsp.PublishDiagnosticsParams{
URI: inoURI,
Diagnostics: []lsp.Diagnostic{},
}
}

convertedDiagnostics := map[lsp.DocumentURI]*lsp.PublishDiagnosticsParams{}
for _, cppDiag := range cppDiags.Diagnostics {
inoURI, inoRange, err := handler.cpp2inoDocumentURI(cppDiags.URI, cppDiag.Range)
inoURI, inoRange, err := handler.cpp2inoDocumentURI(cppURI, cppDiag.Range)
if err != nil {
return nil, err
}
if inoURI.String() == sourcemapper.NotInoURI.String() {
continue
}

inoDiagParam, created := convertedDiagnostics[inoURI]
inoDiagParam, created := inoDiagsParam[inoURI]
if !created {
inoDiagParam = &lsp.PublishDiagnosticsParams{
URI: inoURI,
Diagnostics: []lsp.Diagnostic{},
}
convertedDiagnostics[inoURI] = inoDiagParam
inoDiagsParam[inoURI] = inoDiagParam
}

inoDiag := cppDiag
inoDiag.Range = inoRange
inoDiagParam.Diagnostics = append(inoDiagParam.Diagnostics, inoDiag)

if isSketch {
handler.inoDocsWithDiagnostics[inoURI] = true

// If we have an "undefined reference" in the .ino code trigger a
// check for newly created symbols (that in turn may trigger a
// new arduino-preprocessing of the sketch).
if inoDiag.Code == "undeclared_var_use_suggest" ||
inoDiag.Code == "undeclared_var_use" ||
inoDiag.Code == "ovl_no_viable_function_in_call" ||
inoDiag.Code == "pp_file_not_found" {
handler.buildSketchSymbolsCheck = true
}
}
}

inoDiagParams := []*lsp.PublishDiagnosticsParams{}
for _, v := range convertedDiagnostics {
for _, v := range inoDiagsParam {
inoDiagParams = append(inoDiagParams, v)
}
return inoDiagParams, nil
Expand Down Expand Up @@ -1602,34 +1631,9 @@ func (handler *InoHandler) FromClangd(ctx context.Context, connection *jsonrpc2.
if err != nil {
return nil, err
}
cleanUpInoDiagnostics := false
if len(inoDiagnostics) == 0 {
cleanUpInoDiagnostics = true
}

// Push back to IDE the converted diagnostics
inoDocsWithDiagnostics := map[string]bool{}
for _, inoDiag := range inoDiagnostics {
if inoDiag.URI.String() == sourcemapper.NotInoURI.String() {
cleanUpInoDiagnostics = true
continue
}

// If we have an "undefined reference" in the .ino code trigger a
// check for newly created symbols (that in turn may trigger a
// new arduino-preprocessing of the sketch).
if inoDiag.URI.Ext() == ".ino" {
inoDocsWithDiagnostics[inoDiag.URI.Canonical()] = true
cleanUpInoDiagnostics = true
for _, diag := range inoDiag.Diagnostics {
if diag.Code == "undeclared_var_use_suggest" ||
diag.Code == "undeclared_var_use" ||
diag.Code == "ovl_no_viable_function_in_call" ||
diag.Code == "pp_file_not_found" {
handler.buildSketchSymbolsCheck = true
}
}
}

log.Printf(prefix+"to IDE: publishDiagnostics(%s):", inoDiag.URI)
for _, diag := range inoDiag.Diagnostics {
Expand All @@ -1639,27 +1643,6 @@ func (handler *InoHandler) FromClangd(ctx context.Context, connection *jsonrpc2.
return nil, err
}
}

if cleanUpInoDiagnostics {
// Remove diagnostics from all .ino where there are no errors coming from clang
for sourcePath := range handler.inoDocsWithDiagnostics {
if inoDocsWithDiagnostics[sourcePath] {
// skip if we already sent updated diagnostics
continue
}
// otherwise clear previous diagnostics
msg := lsp.PublishDiagnosticsParams{
URI: lsp.NewDocumentURI(sourcePath),
Diagnostics: []lsp.Diagnostic{},
}
log.Printf(prefix+"to IDE: publishDiagnostics(%s):", msg.URI)
if err := handler.StdioConn.Notify(ctx, "textDocument/publishDiagnostics", msg); err != nil {
return nil, err
}
}

handler.inoDocsWithDiagnostics = inoDocsWithDiagnostics
}
return nil, err

case *lsp.ApplyWorkspaceEditParams:
Expand Down
4 changes: 2 additions & 2 deletions handler/sourcemapper/ino.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ type InoLine struct {

// InoToCppLine converts a source (.ino) line into a target (.cpp) line
func (s *InoMapper) InoToCppLine(sourceURI lsp.DocumentURI, line int) int {
return s.toCpp[InoLine{sourceURI.Canonical(), line}]
return s.toCpp[InoLine{sourceURI.AsPath().String(), line}]
}

// InoToCppLineOk converts a source (.ino) line into a target (.cpp) line
func (s *InoMapper) InoToCppLineOk(sourceURI lsp.DocumentURI, line int) (int, bool) {
res, ok := s.toCpp[InoLine{sourceURI.Canonical(), line}]
res, ok := s.toCpp[InoLine{sourceURI.AsPath().String(), line}]
return res, ok
}

Expand Down
13 changes: 4 additions & 9 deletions lsp/uri.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,25 @@ var expDriveID = regexp.MustCompile("^/[a-zA-Z]:")

// AsPath convert the DocumentURI to a paths.Path
func (uri DocumentURI) AsPath() *paths.Path {
return paths.New(uri.Unbox())
return paths.New(uri.unbox()).Canonical()
}

// Unbox convert the DocumentURI to a file path string
func (uri DocumentURI) Unbox() string {
// unbox convert the DocumentURI to a file path string
func (uri DocumentURI) unbox() string {
path := uri.url.Path
if expDriveID.MatchString(path) {
return path[1:]
}
return path
}

// Canonical returns the canonical path to the file pointed by the URI
func (uri DocumentURI) Canonical() string {
return uri.AsPath().Canonical().String()
}

func (uri DocumentURI) String() string {
return uri.url.String()
}

// Ext returns the extension of the file pointed by the URI
func (uri DocumentURI) Ext() string {
return filepath.Ext(uri.Unbox())
return filepath.Ext(uri.unbox())
}

// NewDocumentURIFromPath create a DocumentURI from the given Path object
Expand Down
16 changes: 8 additions & 8 deletions lsp/uri_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@ import (
func TestUriToPath(t *testing.T) {
d, err := NewDocumentURIFromURL("file:///C:/Users/test/Sketch.ino")
require.NoError(t, err)
require.Equal(t, "C:/Users/test/Sketch.ino", d.Unbox())
require.Equal(t, "C:/Users/test/Sketch.ino", d.unbox())

d, err = NewDocumentURIFromURL("file:///c%3A/Users/test/Sketch.ino")
require.NoError(t, err)
require.Equal(t, "c:/Users/test/Sketch.ino", d.Unbox())
require.Equal(t, "c:/Users/test/Sketch.ino", d.unbox())

d, err = NewDocumentURIFromURL("file:///Users/test/Sketch.ino")
require.NoError(t, err)
require.Equal(t, "/Users/test/Sketch.ino", d.Unbox())
require.Equal(t, "/Users/test/Sketch.ino", d.unbox())

d, err = NewDocumentURIFromURL("file:///c%3A/Users/USERNA~1/AppData/Local/Temp/.arduinoProIDE-unsaved202108-10416-j28c17.lru6k/sketch_jan8a/sketch_jan8a.ino")
require.NoError(t, err)
require.Equal(t, "c:/Users/USERNA~1/AppData/Local/Temp/.arduinoProIDE-unsaved202108-10416-j28c17.lru6k/sketch_jan8a/sketch_jan8a.ino", d.Unbox())
require.Equal(t, "c:/Users/USERNA~1/AppData/Local/Temp/.arduinoProIDE-unsaved202108-10416-j28c17.lru6k/sketch_jan8a/sketch_jan8a.ino", d.unbox())

d, err = NewDocumentURIFromURL("file:///%F0%9F%98%9B")
require.NoError(t, err)
require.Equal(t, "/\U0001F61B", d.Unbox())
require.Equal(t, "/\U0001F61B", d.unbox())
}

func TestPathToUri(t *testing.T) {
Expand All @@ -48,11 +48,11 @@ func TestJSONMarshalUnmarshal(t *testing.T) {
var d DocumentURI
err := json.Unmarshal([]byte(`"file:///Users/test/Sketch.ino"`), &d)
require.NoError(t, err)
require.Equal(t, "/Users/test/Sketch.ino", d.Unbox())
require.Equal(t, "/Users/test/Sketch.ino", d.unbox())

err = json.Unmarshal([]byte(`"file:///%F0%9F%98%9B"`), &d)
require.NoError(t, err)
require.Equal(t, "/\U0001F61B", d.Unbox())
require.Equal(t, "/\U0001F61B", d.unbox())

d = NewDocumentURI("C:\\Users\\test\\Sketch.ino")
data, err := json.Marshal(d)
Expand All @@ -79,7 +79,7 @@ func TestNotInoFromSourceMapper(t *testing.T) {
d, err := NewDocumentURIFromURL("file:///not-ino")
require.NoError(t, err)
fmt.Println(d.String())
fmt.Println(d.Unbox())
fmt.Println(d.unbox())
}

func windowsToSlash(path string) string {
Expand Down