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 <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 3da03e3..aeb6c5b 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -439,6 +439,7 @@
 // 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 e277bda..768ff8c 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -263,13 +263,16 @@
 	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 ee35d63..181a522 100644
--- a/internal/lsp/template/parse.go
+++ b/internal/lsp/template/parse.go
@@ -493,14 +493,8 @@
 }
 
 // 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] + "..."
 	}