From 917abfb0e7d0299b5b87d8f4e7316da6e130cf9c Mon Sep 17 00:00:00 2001
From: Dominik Honnef <dominik@honnef.co>
Date: Mon, 10 May 2021 07:18:16 +0200
Subject: [PATCH] gopls: propagate Staticcheck's diagnostic severities

Each analyzer in Staticcheck is annotated with the appropriate
severity to use for its diagnostics. For example, most checks in SA*
produce warnings, but some produce errors (e.g. when passing an
invalid regular expression to regexp.Compile).

This will be especially important for a follow-up CL that enables
Staticcheck's new quickfix category, which contains optional
refactorings that shouldn't be flagged as warnings.

Change-Id: I6235303a3bb188ef79f52952c01e9585301a3270
Reviewed-on: https://go-review.googlesource.com/c/tools/+/322491
Trust: Dominik Honnef <dominik@honnef.co>
Run-TryBot: Dominik Honnef <dominik@honnef.co>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
---
 gopls/internal/hooks/analysis.go | 21 ++++++++++++++++++++-
 internal/lsp/cache/errors.go     |  7 ++++++-
 internal/lsp/source/options.go   |  8 ++++++--
 internal/lsp/source/view.go      |  4 ++++
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/gopls/internal/hooks/analysis.go b/gopls/internal/hooks/analysis.go
index f9feded80b..25eed8dcf1 100644
--- a/gopls/internal/hooks/analysis.go
+++ b/gopls/internal/hooks/analysis.go
@@ -8,6 +8,7 @@
 package hooks
 
 import (
+	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"honnef.co/go/tools/analysis/lint"
 	"honnef.co/go/tools/simple"
@@ -16,6 +17,24 @@ import (
 )
 
 func updateAnalyzers(options *source.Options) {
+	mapSeverity := func(severity lint.Severity) protocol.DiagnosticSeverity {
+		switch severity {
+		case lint.SeverityError:
+			return protocol.SeverityError
+		case lint.SeverityDeprecated:
+			// TODO(dh): in LSP, deprecated is a tag, not a severity.
+			//   We'll want to support this once we enable SA5011.
+			return protocol.SeverityWarning
+		case lint.SeverityWarning:
+			return protocol.SeverityWarning
+		case lint.SeverityInfo:
+			return protocol.SeverityInformation
+		case lint.SeverityHint:
+			return protocol.SeverityHint
+		default:
+			return protocol.SeverityWarning
+		}
+	}
 	add := func(analyzers []*lint.Analyzer, skip map[string]struct{}) {
 		for _, a := range analyzers {
 			if _, ok := skip[a.Analyzer.Name]; ok {
@@ -23,7 +42,7 @@ func updateAnalyzers(options *source.Options) {
 			}
 
 			enabled := !a.Doc.NonDefault
-			options.AddStaticcheckAnalyzer(a.Analyzer, enabled)
+			options.AddStaticcheckAnalyzer(a.Analyzer, enabled, mapSeverity(a.Doc.Severity))
 		}
 	}
 
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index 42fafae812..6cc3e4592b 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -208,10 +208,15 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana
 	if err != nil {
 		return nil, err
 	}
+
+	severity := srcAnalyzer.Severity
+	if severity == 0 {
+		severity = protocol.SeverityWarning
+	}
 	diag := &source.Diagnostic{
 		URI:            spn.URI(),
 		Range:          rng,
-		Severity:       protocol.SeverityWarning,
+		Severity:       severity,
 		Source:         source.AnalyzerErrorKind(e.Category),
 		Message:        e.Message,
 		Related:        related,
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index dfa631eef0..1f81c9eb37 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -716,8 +716,12 @@ func (o *Options) Clone() *Options {
 	return result
 }
 
-func (o *Options) AddStaticcheckAnalyzer(a *analysis.Analyzer, enabled bool) {
-	o.StaticcheckAnalyzers[a.Name] = &Analyzer{Analyzer: a, Enabled: enabled}
+func (o *Options) AddStaticcheckAnalyzer(a *analysis.Analyzer, enabled bool, severity protocol.DiagnosticSeverity) {
+	o.StaticcheckAnalyzers[a.Name] = &Analyzer{
+		Analyzer: a,
+		Enabled:  enabled,
+		Severity: severity,
+	}
 }
 
 // EnableAllExperiments turns on all of the experimental "off-by-default"
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 6612d53481..a139c50249 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -534,6 +534,10 @@ type Analyzer struct {
 	// ActionKind is the kind of code action this analyzer produces. If
 	// unspecified the type defaults to quickfix.
 	ActionKind []protocol.CodeActionKind
+
+	// Severity is the severity set for diagnostics reported by this
+	// analyzer. If left unset it defaults to Warning.
+	Severity protocol.DiagnosticSeverity
 }
 
 func (a Analyzer) IsEnabled(view View) bool {