Merge pull request #57 from bcmi-labs/fix-publish-diagnostic

Fix publish diagnostic

Author: cmaglie

Diff for: handler/handler.go

@@ -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
 }
@@ -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,
 		},
@@ -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" {
@@ -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)
@@ -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
@@ -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)
 	}
@@ -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)
@@ -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)
@@ -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
@@ -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
@@ -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 {
@@ -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:

Diff for: handler/sourcemapper/ino.go

@@ -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
 }
 

Diff for: lsp/uri.go

@@ -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

Diff for: lsp/uri_test.go

@@ -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) {
@@ -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)
@@ -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 {