Fix #10 by hard de-dupping documentSymbols

handler/handler.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
@@ -26,24 +27,23 @@ func Setup(cliPath string, _enableLogging bool) {
 	enableLogging = _enableLogging
 }
 
+// CLangdStarter starts clangd and returns its stdin/out/err
+type CLangdStarter func() (stdin io.WriteCloser, stdout io.ReadCloser, stderr io.ReadCloser)
+
 // NewInoHandler creates and configures an InoHandler.
-func NewInoHandler(stdin io.ReadCloser, stdout io.WriteCloser, stdinLog, stdoutLog io.Writer,
-	startClangd func() (io.WriteCloser, io.ReadCloser, io.ReadCloser),
-	clangdinLog, clangdoutLog, clangderrLog io.Writer, board Board) *InoHandler {
+func NewInoHandler(stdin io.ReadCloser, stdout io.WriteCloser, logStreams *StreamLogger, startClangd CLangdStarter, board Board) *InoHandler {
 	handler := &InoHandler{
 		clangdProc: ClangdProc{
-			Start:  startClangd,
-			inLog:  clangdinLog,
-			outLog: clangdoutLog,
-			errLog: clangderrLog,
+			Start: startClangd,
+			Logs:  logStreams,
 		},
 		data: make(map[lsp.DocumentURI]*FileData),
 		config: BoardConfig{
 			SelectedBoard: board,
 		},
 	}
-	handler.StartClangd()
-	stdStream := jsonrpc2.NewBufferedStream(StreamReadWrite{stdin, stdout, stdinLog, stdoutLog}, jsonrpc2.VSCodeObjectCodec{})
+	handler.startClangd()
+	stdStream := jsonrpc2.NewBufferedStream(logStreams.AttachStdInOut(stdin, stdout), jsonrpc2.VSCodeObjectCodec{})
 	stdHandler := jsonrpc2.AsyncHandler(jsonrpc2.HandlerWithError(handler.FromStdio))
 	handler.StdioConn = jsonrpc2.NewConn(context.Background(), stdStream, stdHandler)
 	if enableLogging {
@@ -63,9 +63,9 @@ type InoHandler struct {
 
 // ClangdProc contains the process input / output streams for clangd.
 type ClangdProc struct {
-	Start                 func() (io.WriteCloser, io.ReadCloser, io.ReadCloser)
-	inLog, outLog, errLog io.Writer
-	initParams            lsp.InitializeParams
+	Start      func() (io.WriteCloser, io.ReadCloser, io.ReadCloser)
+	Logs       *StreamLogger
+	initParams lsp.InitializeParams
 }
 
 // FileData gathers information on a .ino source file.
@@ -79,12 +79,12 @@ type FileData struct {
 }
 
 // StartClangd starts the clangd process and connects its input / output streams.
-func (handler *InoHandler) StartClangd() {
+func (handler *InoHandler) startClangd() {
 	clangdWrite, clangdRead, clangdErr := handler.clangdProc.Start()
 	if enableLogging {
-		go io.Copy(handler.clangdProc.errLog, clangdErr)
+		go io.Copy(handler.clangdProc.Logs.ClangdErr, clangdErr)
 	}
-	srw := StreamReadWrite{clangdRead, clangdWrite, handler.clangdProc.inLog, handler.clangdProc.outLog}
+	srw := handler.clangdProc.Logs.AttachClangdInOut(clangdRead, clangdWrite)
 	clangdStream := jsonrpc2.NewBufferedStream(srw, jsonrpc2.VSCodeObjectCodec{})
 	clangdHandler := jsonrpc2.AsyncHandler(jsonrpc2.HandlerWithError(handler.FromClangd))
 	handler.ClangdConn = jsonrpc2.NewConn(context.Background(), clangdStream, clangdHandler)
@@ -178,7 +178,7 @@ func (handler *InoHandler) changeBoardConfig(ctx context.Context, config *BoardC
 	}
 
 	// Restart the clangd process, initialize it and reopen the files
-	handler.StartClangd()
+	handler.startClangd()
 	initResult := new(lsp.InitializeResult)
 	err := handler.ClangdConn.Call(ctx, "initialize", &handler.clangdProc.initParams, initResult)
 	if err != nil {
@@ -588,29 +588,32 @@ func (handler *InoHandler) transformClangdResult(method string, uri lsp.Document
 			handler.cpp2inoTextEdit(&(*r)[index], uri)
 		}
 	case "textDocument/documentSymbol":
-		r := result.(*[]*documentSymbolOrSymbolInformation)
+		r, ok := result.(*[]*documentSymbolOrSymbolInformation)
+
+		if !ok || len(*r) == 0 {
+			return result
+		}
+
 		slice := *r
-		if len(slice) > 0 && slice[0].DocumentSymbol != nil {
+		if slice[0].DocumentSymbol != nil {
 			// Treat the input as []DocumentSymbol
 			symbols := make([]DocumentSymbol, len(slice))
 			for index := range slice {
 				symbols[index] = *slice[index].DocumentSymbol
 			}
-			result = handler.cpp2inoDocumentSymbols(symbols, uri)
-		} else if len(slice) > 0 && slice[0].SymbolInformation != nil {
+			return handler.cpp2inoDocumentSymbols(symbols, uri)
+		}
+		if slice[0].SymbolInformation != nil {
 			// Treat the input as []SymbolInformation
-			symbols := make([]lsp.SymbolInformation, len(slice))
-			for index := range slice {
-				symbols[index] = *slice[index].SymbolInformation
+			symbols := make([]*lsp.SymbolInformation, len(slice))
+			for i, s := range slice {
+				symbols[i] = s.SymbolInformation
 			}
-			for index := range symbols {
-				handler.cpp2inoLocation(&symbols[index].Location)
-			}
-			result = symbols
+			return handler.cpp2inoSymbolInformation(symbols)
 		}
 	case "textDocument/rename":
 		r := result.(*lsp.WorkspaceEdit)
-		result = handler.cpp2inoWorkspaceEdit(r)
+		return handler.cpp2inoWorkspaceEdit(r)
 	case "workspace/symbol":
 		r := result.(*[]lsp.SymbolInformation)
 		for index := range *r {
@@ -712,29 +715,61 @@ func (handler *InoHandler) cpp2inoDocumentSymbols(origSymbols []DocumentSymbol,
 	if !ok || len(origSymbols) == 0 {
 		return origSymbols
 	}
-	newSymbols := make([]DocumentSymbol, len(origSymbols))
-	j := 0
+
+	symbolIdx := make(map[string]*DocumentSymbol)
 	for i := 0; i < len(origSymbols); i++ {
 		symbol := &origSymbols[i]
 		symbol.Range.Start.Line = data.sourceLineMap[symbol.Range.Start.Line]
 		symbol.Range.End.Line = data.sourceLineMap[symbol.Range.End.Line]
 
 		duplicate := false
-		for k := 0; k < j; k++ {
-			if symbol.Name == newSymbols[k].Name && symbol.Range.Start.Line == newSymbols[k].Range.Start.Line {
-				duplicate = true
-				break
+		other, duplicate := symbolIdx[symbol.Name]
+		if duplicate {
+			// we prefer symbols later in the file due to the function header generation. E.g. if one has a function `void foo() {}` somehwre in the code
+			// the code generation will add a `void foo();` header at the beginning of the cpp file. We care about the function body later in the file, not
+			// the header early on.
+			if other.Range.Start.Line < symbol.Range.Start.Line {
+				continue
 			}
 		}
-		if !duplicate {
-			symbol.SelectionRange.Start.Line = data.sourceLineMap[symbol.SelectionRange.Start.Line]
-			symbol.SelectionRange.End.Line = data.sourceLineMap[symbol.SelectionRange.End.Line]
-			symbol.Children = handler.cpp2inoDocumentSymbols(symbol.Children, uri)
-			newSymbols[j] = *symbol
-			j++
+
+		symbol.SelectionRange.Start.Line = data.sourceLineMap[symbol.SelectionRange.Start.Line]
+		symbol.SelectionRange.End.Line = data.sourceLineMap[symbol.SelectionRange.End.Line]
+		symbol.Children = handler.cpp2inoDocumentSymbols(symbol.Children, uri)
+		symbolIdx[symbol.Name] = symbol
+	}
+
+	newSymbols := make([]DocumentSymbol, len(symbolIdx))
+	j := 0
+	for _, s := range symbolIdx {
+		newSymbols[j] = *s
+		j++
+	}
+	return newSymbols
+}
+
+func (handler *InoHandler) cpp2inoSymbolInformation(syms []*lsp.SymbolInformation) []lsp.SymbolInformation {
+	// much like in cpp2inoDocumentSymbols we de-duplicate symbols based on file in-file location.
+	idx := make(map[string]*lsp.SymbolInformation)
+	for _, sym := range syms {
+		handler.cpp2inoLocation(&sym.Location)
+
+		nme := fmt.Sprintf("%s::%s", sym.ContainerName, sym.Name)
+		other, duplicate := idx[nme]
+		if duplicate && other.Location.Range.Start.Line < sym.Location.Range.Start.Line {
+			continue
 		}
+
+		idx[nme] = sym
+	}
+
+	var j int
+	symbols := make([]lsp.SymbolInformation, len(idx))
+	for _, sym := range idx {
+		symbols[j] = *sym
+		j++
 	}
-	return newSymbols[:j]
+	return symbols
 }
 
 // FromClangd handles a message received from clangd.

handler/streamlog.go

@@ -0,0 +1,147 @@
+package handler
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// StreamLogger maintains log files for all streams involved in the language server
+type StreamLogger struct {
+	Default   io.WriteCloser
+	Stdin     io.WriteCloser
+	Stdout    io.WriteCloser
+	ClangdIn  io.WriteCloser
+	ClangdOut io.WriteCloser
+	ClangdErr io.WriteCloser
+}
+
+// Close closes all logging streams
+func (s *StreamLogger) Close() (err error) {
+	var errs []string
+	for _, c := range []io.Closer{s.Default, s.Stdin, s.Stdout, s.ClangdIn, s.ClangdOut, s.ClangdErr} {
+		if c == nil {
+			continue
+		}
+
+		err = c.Close()
+		if err != nil {
+			errs = append(errs, err.Error())
+		}
+	}
+	if len(errs) != 0 {
+		return fmt.Errorf(strings.Join(errs, ", "))
+	}
+
+	return nil
+}
+
+// AttachStdInOut attaches the stdin, stdout logger to the in/out channels
+func (s *StreamLogger) AttachStdInOut(in io.ReadCloser, out io.WriteCloser) io.ReadWriteCloser {
+	return &streamDuplex{
+		io.TeeReader(in, s.Stdin),
+		in,
+		io.MultiWriter(out, s.Stdout),
+		out,
+	}
+}
+
+// AttachClangdInOut attaches the clangd in, out logger to the in/out channels
+func (s *StreamLogger) AttachClangdInOut(in io.ReadCloser, out io.WriteCloser) io.ReadWriteCloser {
+	return &streamDuplex{
+		io.TeeReader(in, s.ClangdIn),
+		in,
+		io.MultiWriter(out, s.ClangdOut),
+		out,
+	}
+}
+
+type streamDuplex struct {
+	in   io.Reader
+	inc  io.Closer
+	out  io.Writer
+	outc io.Closer
+}
+
+func (sd *streamDuplex) Read(p []byte) (int, error) {
+	return sd.in.Read(p)
+}
+
+func (sd *streamDuplex) Write(p []byte) (int, error) {
+	return sd.out.Write(p)
+}
+
+func (sd *streamDuplex) Close() error {
+	ierr := sd.inc.Close()
+	oerr := sd.outc.Close()
+
+	if ierr != nil {
+		return ierr
+	}
+	if oerr != nil {
+		return oerr
+	}
+	return nil
+}
+
+// NewStreamLogger creates files for all stream logs. Returns an error if opening a single stream fails.
+func NewStreamLogger(basepath string) (res *StreamLogger, err error) {
+	res = &StreamLogger{}
+
+	res.Default, err = os.OpenFile(filepath.Join(basepath, "inols.log"), os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.Stdin, err = os.OpenFile(filepath.Join(basepath, "inols-stdin.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.Stdout, err = os.OpenFile(filepath.Join(basepath, "inols-stdout.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdIn, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-in.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdOut, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-out.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdErr, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-err.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+
+	return
+}
+
+// NewNoopLogger creates a logger that does nothing
+func NewNoopLogger() (res *StreamLogger) {
+	noop := noopCloser{ioutil.Discard}
+	return &StreamLogger{
+		Default:   noop,
+		Stdin:     noop,
+		Stdout:    noop,
+		ClangdIn:  noop,
+		ClangdOut: noop,
+		ClangdErr: noop,
+	}
+}
+
+type noopCloser struct {
+	io.Writer
+}
+
+func (noopCloser) Close() error {
+	return nil
+}