From 93852cb65b2312da940f01dec1c7c342c2a5ea8d Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 10 May 2022 12:18:53 -0400 Subject: [PATCH] internal/lsp: fix source.CompareDiagnostic asymmetry In passing I noticed that this three-way comparison is not (anti)symmetric. Such comparisons should consist of a list of pairs of tests of this form: if x.key1 < y.key1 { return -1 } if x.key1 > y.key1 { return +1 } ...key2, etc... return 0 Also in passing: - simplify panic-proof debug string function. - augment doc comment of (*Server).beginFileRequest Change-Id: Idcd0844ea4e96fc2dee5bbc270f5a200b5c27aa0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/405480 Reviewed-by: Robert Findley TryBot-Result: Gopher Robot Run-TryBot: Robert Findley --- internal/lsp/general.go | 1 + internal/lsp/source/util.go | 9 ++++++--- internal/lsp/template/parse.go | 10 ++-------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 3da03e3fc3..aeb6c5b223 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -439,6 +439,7 @@ func (s *Server) handleOptionResults(ctx context.Context, results source.OptionR // it to a snapshot. // We don't want to return errors for benign conditions like wrong file type, // so callers should do if !ok { return err } rather than if err != nil. +// The returned cleanup function is non-nil even in case of false/error result. func (s *Server) beginFileRequest(ctx context.Context, pURI protocol.DocumentURI, expectKind source.FileKind) (source.Snapshot, source.VersionedFileHandle, bool, func(), error) { uri := pURI.SpanURI() if !uri.IsFile() { diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index e277bdab45..768ff8c636 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -263,13 +263,16 @@ func CompareDiagnostic(a, b *Diagnostic) int { if a.Source < b.Source { return -1 } + if a.Source > b.Source { + return +1 + } if a.Message < b.Message { return -1 } - if a.Message == b.Message { - return 0 + if a.Message > b.Message { + return +1 } - return 1 + return 0 } // FindPackageFromPos finds the first package containing pos in its diff --git a/internal/lsp/template/parse.go b/internal/lsp/template/parse.go index ee35d634d2..181a5228fd 100644 --- a/internal/lsp/template/parse.go +++ b/internal/lsp/template/parse.go @@ -493,14 +493,8 @@ func (wr wrNode) writeNode(n parse.Node, indent string) { } // short prints at most 40 bytes of node.String(), for debugging -func short(n parse.Node) (ret string) { - defer func() { - if x := recover(); x != nil { - // all because of typed nils - ret = "NIL" - } - }() - s := n.String() +func short(n parse.Node) string { + s := fmt.Sprint(n) // recovers from panic if len(s) > 40 { return s[:40] + "..." }