internal/lsp/cache: honor RunDespiteErrors=false

This change prevents analysis from running on a package containing any kind of error unless the analyzer has indicated that it is robust. This is presumed to be the cause of several panics in production. And a test. Updates golang/go#54762 Change-Id: I9327e18ef8d7773c943ea45fc786991188358131 Reviewed-on: https://go-review.googlesource.com/c/tools/+/426803 Run-TryBot: Alan Donovan <[email protected]> Auto-Submit: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]>
golang · Sep 1, 2022 · 6c10975 · 6c10975
1 parent 49ab44d
commit 6c10975
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 3 deletions.
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
@@ -329,9 +329,9 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a
 	}
 	analysisinternal.SetTypeErrors(pass, pkg.typeErrors)
 
-	// TODO(adonovan): fix: don't run analyzers on packages
-	// containing type errors unless the analyzers have
-	// RunDespiteErrors=true.
+	if (pkg.HasListOrParseErrors() || pkg.HasTypeErrors()) && !analyzer.RunDespiteErrors {
+		return nil, fmt.Errorf("skipping analysis %s because package %s contains errors", analyzer.Name, pkg.ID())
+	}
 
 	// Recover from panics (only) within the analyzer logic.
 	// (Use an anonymous function to limit the recover scope.)

diff --git a/internal/lsp/testdata/rundespiteerrors/rundespiteerrors.go b/internal/lsp/testdata/rundespiteerrors/rundespiteerrors.go
@@ -0,0 +1,14 @@
+package rundespiteerrors
+
+// This test verifies that analyzers without RunDespiteErrors are not
+// executed on a package containing type errors (see issue #54762).
+func _() {
+	// A type error.
+	_ = 1 + "" //@diag("1", "compiler", "mismatched types|cannot convert", "error")
+
+	// A violation of an analyzer for which RunDespiteErrors=false:
+	// no diagnostic is produced; the diag comment is merely illustrative.
+	for _ = range "" { //diag("for _", "simplifyrange", "simplify range expression", "warning")
+
+	}
+}