From 6c10975b72dcad6a00720f002f908e146ac6737c Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 31 Aug 2022 15:41:08 -0400 Subject: [PATCH] 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 Auto-Submit: Alan Donovan gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley --- internal/lsp/cache/analysis.go | 6 +++--- .../testdata/rundespiteerrors/rundespiteerrors.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 internal/lsp/testdata/rundespiteerrors/rundespiteerrors.go diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go index 5a29feffa7f..144ef3ce4ea 100644 --- 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 new file mode 100644 index 00000000000..783e9a55f17 --- /dev/null +++ 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") + + } +}