Use canonical form for files when dealing with URI

This should avoid some weird cases on Windows when sometimes short path
in the DOS8.3 form are mixed with the longer counterpart.

Author: cmaglie

go.mod

@@ -4,7 +4,7 @@ go 1.12
 
 require (
 	github.com/arduino/arduino-cli v0.0.0-20201215104024-6a177ebf56f2
-	github.com/arduino/go-paths-helper v1.4.0
+	github.com/arduino/go-paths-helper v1.5.0
 	github.com/pkg/errors v0.9.1
 	github.com/sourcegraph/jsonrpc2 v0.0.0-20200429184054-15c2290dcb37
 	github.com/stretchr/testify v1.6.1

go.sum

@@ -17,6 +17,8 @@ github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3
 github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
 github.com/arduino/go-paths-helper v1.4.0 h1:ilnseAdxmN1bFnLxxXHRtcdmt9jBf3O4jtYfWfqule4=
 github.com/arduino/go-paths-helper v1.4.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
+github.com/arduino/go-paths-helper v1.5.0 h1:RVo189hD+GhUS1rQ3gixwK1nSbvVR8MGIGa7Gxv2bdM=
+github.com/arduino/go-paths-helper v1.5.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
 github.com/arduino/go-properties-orderedmap v1.3.0 h1:4No/vQopB36e7WUIk6H6TxiSEJPiMrVOCZylYmua39o=
 github.com/arduino/go-properties-orderedmap v1.3.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=

handler/builder.go

@@ -112,7 +112,7 @@ func (handler *InoHandler) generateBuildEnvironment() (*paths.Path, error) {
 	}
 	data := overridesFile{Overrides: map[string]string{}}
 	for uri, trackedFile := range handler.docs {
-		rel, err := uri.AsPath().RelFrom(handler.sketchRoot)
+		rel, err := paths.New(uri).RelFrom(handler.sketchRoot)
 		if err != nil {
 			return nil, errors.WithMessage(err, "dumping tracked files")
 		}

handler/handler.go

@@ -58,8 +58,8 @@ type InoHandler struct {
 	sketchName                 string
 	sketchMapper               *sourcemapper.InoMapper
 	sketchTrackedFilesCount    int
-	docs                       map[lsp.DocumentURI]*lsp.TextDocumentItem
-	inoDocsWithDiagnostics     map[lsp.DocumentURI]bool
+	docs                       map[string]*lsp.TextDocumentItem
+	inoDocsWithDiagnostics     map[string]bool
 
 	config       lsp.BoardConfig
 	synchronizer Synchronizer
@@ -68,8 +68,8 @@ type InoHandler struct {
 // NewInoHandler creates and configures an InoHandler.
 func NewInoHandler(stdio io.ReadWriteCloser, board lsp.Board) *InoHandler {
 	handler := &InoHandler{
-		docs:                   map[lsp.DocumentURI]*lsp.TextDocumentItem{},
-		inoDocsWithDiagnostics: map[lsp.DocumentURI]bool{},
+		docs:                   map[string]*lsp.TextDocumentItem{},
+		inoDocsWithDiagnostics: map[string]bool{},
 		config: lsp.BoardConfig{
 			SelectedBoard: board,
 		},
@@ -542,7 +542,7 @@ func startClangd(compileCommandsDir, sketchCpp *paths.Path) (io.WriteCloser, io.
 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] = &inoItem
+	handler.docs[inoItem.URI.Canonical()] = &inoItem
 
 	// If we are tracking a .ino...
 	if inoItem.URI.Ext() == ".ino" {
@@ -566,8 +566,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]; exist {
-		delete(handler.docs, inoIdentifier.URI)
+	if _, exist := handler.docs[inoIdentifier.URI.Canonical()]; exist {
+		delete(handler.docs, inoIdentifier.URI.Canonical())
 	} else {
 		log.Printf("    didClose of untracked document: %s", inoIdentifier.URI)
 		return nil, unknownURI(inoIdentifier.URI)
@@ -602,8 +602,8 @@ func (handler *InoHandler) ino2cppTextDocumentItem(inoItem lsp.TextDocumentItem)
 		cppItem.Text = handler.sketchMapper.CppText.Text
 		cppItem.Version = handler.sketchMapper.CppText.Version
 	} else {
-		cppItem.Text = handler.docs[inoItem.URI].Text
-		cppItem.Version = handler.docs[inoItem.URI].Version
+		cppItem.Text = handler.docs[inoItem.URI.Canonical()].Text
+		cppItem.Version = handler.docs[inoItem.URI.Canonical()].Version
 	}
 
 	return cppItem, nil
@@ -612,7 +612,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]
+	trackedDoc, ok := handler.docs[doc.URI.Canonical()]
 	if !ok {
 		return nil, unknownURI(doc.URI)
 	}
@@ -766,6 +766,23 @@ func (handler *InoHandler) ino2cppDocumentURI(inoURI lsp.DocumentURI) (lsp.Docum
 	return lsp.NilURI, err
 }
 
+func (handler *InoHandler) inoDocumentURIFromInoPath(inoPath string) (lsp.DocumentURI, error) {
+	if inoPath == sourcemapper.NotIno.File {
+		return sourcemapper.NotInoURI, nil
+	}
+	doc, ok := handler.docs[inoPath]
+	if !ok {
+		log.Printf("    !!! Unresolved .ino path: %s", inoPath)
+		log.Printf("    !!! Known doc paths are:")
+		for p := range handler.docs {
+			log.Printf("    !!! > %s", p)
+		}
+		uri := lsp.NewDocumentURI(inoPath)
+		return uri, unknownURI(uri)
+	}
+	return doc.URI, nil
+}
+
 func (handler *InoHandler) cpp2inoDocumentURI(cppURI lsp.DocumentURI, cppRange lsp.Range) (lsp.DocumentURI, lsp.Range, error) {
 	// Sketchbook/Sketch/Sketch.ino      <- build-path/sketch/Sketch.ino.cpp
 	// Sketchbook/Sketch/AnotherTab.ino  <- build-path/sketch/Sketch.ino.cpp  (different section from above)
@@ -784,16 +801,19 @@ func (handler *InoHandler) cpp2inoDocumentURI(cppURI lsp.DocumentURI, cppRange l
 		} else {
 			log.Printf("    URI: ERROR: %s", err)
 			handler.sketchMapper.DebugLogAll()
+			return lsp.NilURI, lsp.NilRange, err
 		}
-		return lsp.NewDocumentURI(inoPath), inoRange, err
+		inoURI, err := handler.inoDocumentURIFromInoPath(inoPath)
+		return inoURI, inoRange, err
 	}
 
 	inside, err := cppPath.IsInsideDir(handler.buildSketchRoot)
 	if err != nil {
 		log.Printf("    could not determine if '%s' is inside '%s'", cppPath, handler.buildSketchRoot)
-		return lsp.NilURI, lsp.Range{}, err
+		return lsp.NilURI, lsp.NilRange, err
 	}
 	if !inside {
+		log.Printf("    '%s' is not inside '%s'", cppPath, handler.buildSketchRoot)
 		log.Printf("    keep doc identifier to '%s' as-is", cppPath)
 		return cppURI, cppRange, nil
 	}
@@ -806,7 +826,7 @@ func (handler *InoHandler) cpp2inoDocumentURI(cppURI lsp.DocumentURI, cppRange l
 	}
 
 	log.Printf("    could not determine rel-path of '%s' in '%s': %s", cppPath, handler.buildSketchRoot, err)
-	return lsp.NilURI, lsp.Range{}, err
+	return lsp.NilURI, lsp.NilRange, err
 }
 
 func (handler *InoHandler) ino2cppTextDocumentPositionParams(inoParams *lsp.TextDocumentPositionParams) (*lsp.TextDocumentPositionParams, error) {
@@ -1111,36 +1131,41 @@ func (handler *InoHandler) Cpp2InoCommand(command *lsp.Command) *lsp.Command {
 	return inoCommand
 }
 
-func (handler *InoHandler) cpp2inoWorkspaceEdit(origWorkspaceEdit *lsp.WorkspaceEdit) *lsp.WorkspaceEdit {
-	if origWorkspaceEdit == nil {
+func (handler *InoHandler) cpp2inoWorkspaceEdit(cppWorkspaceEdit *lsp.WorkspaceEdit) *lsp.WorkspaceEdit {
+	if cppWorkspaceEdit == nil {
 		return nil
 	}
-	resWorkspaceEdit := &lsp.WorkspaceEdit{
+	inoWorkspaceEdit := &lsp.WorkspaceEdit{
 		Changes: map[lsp.DocumentURI][]lsp.TextEdit{},
 	}
-	for editURI, edits := range origWorkspaceEdit.Changes {
+	for editURI, edits := range cppWorkspaceEdit.Changes {
 		// if the edits are not relative to sketch file...
 		if !editURI.AsPath().EquivalentTo(handler.buildSketchCpp) {
 			// ...pass them through...
-			resWorkspaceEdit.Changes[editURI] = edits
+			inoWorkspaceEdit.Changes[editURI] = edits
 			continue
 		}
 
 		// ...otherwise convert edits to the sketch.ino.cpp into multilpe .ino edits
 		for _, edit := range edits {
-			cppRange := edit.Range
-			inoFile, inoRange := handler.sketchMapper.CppToInoRange(cppRange)
-			inoURI := lsp.NewDocumentURI(inoFile)
-			if _, have := resWorkspaceEdit.Changes[inoURI]; !have {
-				resWorkspaceEdit.Changes[inoURI] = []lsp.TextEdit{}
+			inoURI, inoRange, err := handler.cpp2inoDocumentURI(editURI, edit.Range)
+			if err != nil {
+				log.Printf("    error converting edit %s:%s: %s", editURI, edit.Range, err)
+				continue
+			}
+			//inoFile, inoRange := handler.sketchMapper.CppToInoRange(edit.Range)
+			//inoURI := lsp.NewDocumentURI(inoFile)
+			if _, have := inoWorkspaceEdit.Changes[inoURI]; !have {
+				inoWorkspaceEdit.Changes[inoURI] = []lsp.TextEdit{}
 			}
-			resWorkspaceEdit.Changes[inoURI] = append(resWorkspaceEdit.Changes[inoURI], lsp.TextEdit{
+			inoWorkspaceEdit.Changes[inoURI] = append(inoWorkspaceEdit.Changes[inoURI], lsp.TextEdit{
 				NewText: edit.NewText,
 				Range:   inoRange,
 			})
 		}
 	}
-	return resWorkspaceEdit
+	log.Printf("    done converting workspaceEdit")
+	return inoWorkspaceEdit
 }
 
 func (handler *InoHandler) cpp2inoLocation(cppLocation lsp.Location) (lsp.Location, error) {
@@ -1260,7 +1285,7 @@ func (handler *InoHandler) cpp2inoDiagnostics(cppDiags *lsp.PublishDiagnosticsPa
 			return []*lsp.PublishDiagnosticsParams{}, nil
 		}
 
-		inoURI, _, err := handler.cpp2inoDocumentURI(cppDiags.URI, lsp.Range{})
+		inoURI, _, err := handler.cpp2inoDocumentURI(cppDiags.URI, lsp.NilRange)
 		return []*lsp.PublishDiagnosticsParams{
 			{
 				URI:         inoURI,
@@ -1332,9 +1357,9 @@ func (handler *InoHandler) FromClangd(ctx context.Context, connection *jsonrpc2.
 		}
 
 		// Push back to IDE the converted diagnostics
-		inoDocsWithDiagnostics := map[lsp.DocumentURI]bool{}
+		inoDocsWithDiagnostics := map[string]bool{}
 		for _, inoDiag := range inoDiagnostics {
-			if inoDiag.URI == lsp.NewDocumentURI(sourcemapper.NotIno.File) {
+			if inoDiag.URI.String() == sourcemapper.NotInoURI.String() {
 				cleanUpInoDiagnostics = true
 				continue
 			}
@@ -1350,7 +1375,7 @@ func (handler *InoHandler) FromClangd(ctx context.Context, connection *jsonrpc2.
 			// 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] = true
+				inoDocsWithDiagnostics[inoDiag.URI.Canonical()] = true
 				cleanUpInoDiagnostics = true
 				for _, diag := range inoDiag.Diagnostics {
 					if diag.Code == "undeclared_var_use_suggest" || diag.Code == "undeclared_var_use" {
@@ -1366,14 +1391,14 @@ func (handler *InoHandler) FromClangd(ctx context.Context, connection *jsonrpc2.
 
 		if cleanUpInoDiagnostics {
 			// Remove diagnostics from all .ino where there are no errors coming from clang
-			for sourceURI := range handler.inoDocsWithDiagnostics {
-				if inoDocsWithDiagnostics[sourceURI] {
+			for sourcePath := range handler.inoDocsWithDiagnostics {
+				if inoDocsWithDiagnostics[sourcePath] {
 					// skip if we already sent updated diagnostics
 					continue
 				}
 				// otherwise clear previous diagnostics
 				msg := lsp.PublishDiagnosticsParams{
-					URI:         sourceURI,
+					URI:         lsp.NewDocumentURI(sourcePath),
 					Diagnostics: []lsp.Diagnostic{},
 				}
 				if enableLogging {

handler/sourcemapper/ino.go

@@ -9,14 +9,14 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/arduino/go-paths-helper"
 	"github.com/bcmi-labs/arduino-language-server/handler/textutils"
 	"github.com/bcmi-labs/arduino-language-server/lsp"
 	"github.com/pkg/errors"
 )
 
 // InoMapper is a mapping between the .ino sketch and the preprocessed .cpp file
 type InoMapper struct {
-	InoText         map[lsp.DocumentURI]*SourceRevision
 	CppText         *SourceRevision
 	toCpp           map[InoLine]int // Converts File.ino:line -> line
 	toIno           map[int]InoLine // Convers line -> File.ino:line
@@ -25,7 +25,10 @@ type InoMapper struct {
 }
 
 // NotIno are lines that do not belongs to an .ino file
-var NotIno = InoLine{"not-ino", 0}
+var NotIno = InoLine{"/not-ino", 0}
+
+// NotInoURI is the DocumentURI that do not belongs to an .ino file
+var NotInoURI, _ = lsp.NewDocumentURIFromURL("file:///not-ino")
 
 type SourceRevision struct {
 	Version int
@@ -40,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.Unbox(), line}]
+	return s.toCpp[InoLine{sourceURI.Canonical(), 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.Unbox(), line}]
+	res, ok := s.toCpp[InoLine{sourceURI.Canonical(), line}]
 	return res, ok
 }
 
@@ -166,7 +169,7 @@ func CreateInoMapper(targetFile []byte) *InoMapper {
 			if err == nil && l > 0 {
 				sourceLine = l - 1
 			}
-			sourceFile = unquoteCppString(tokens[2])
+			sourceFile = paths.New(unquoteCppString(tokens[2])).Canonical().String()
 			mapper.toIno[targetLine] = NotIno
 		} else if sourceFile != "" {
 			mapper.mapLine(sourceFile, sourceLine, targetLine)

lsp/structures.go

@@ -48,6 +48,8 @@ type Range struct {
 	End Position `json:"end"`
 }
 
+var NilRange = Range{}
+
 func (r Range) String() string {
 	return fmt.Sprintf("%s-%s", r.Start, r.End)
 }

lsp/uri.go

@@ -33,6 +33,11 @@ func (uri DocumentURI) Unbox() string {
 	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()
 }

lsp/uri_test.go

@@ -2,6 +2,7 @@ package lsp
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -74,6 +75,13 @@ func TestJSONMarshalUnmarshal(t *testing.T) {
 	require.Equal(t, `"file:///%F0%9F%98%9B"`, string(data))
 }
 
+func TestNotInoFromSourceMapper(t *testing.T) {
+	d, err := NewDocumentURIFromURL("file:///not-ino")
+	require.NoError(t, err)
+	fmt.Println(d.String())
+	fmt.Println(d.Unbox())
+}
+
 func windowsToSlash(path string) string {
 	return strings.ReplaceAll(path, `\`, "/")
 }