Skip to content

Commit cef11a9

Browse files
heschistamblerre
authored andcommitted
internal/lsp/cache: tolerate analysis panics better
Panics in type error analyzers shouldn't block diagnostics. Fixes golang/go#45075. Change-Id: I897f0949551ab65276371f7ec8140ccb689e5a7b Reviewed-on: https://go-review.googlesource.com/c/tools/+/302533 Trust: Heschi Kreinick <[email protected]> Run-TryBot: Heschi Kreinick <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]> (cherry picked from commit 9b614f5) Reviewed-on: https://go-review.googlesource.com/c/tools/+/302689 Trust: Rebecca Stambler <[email protected]> Run-TryBot: Rebecca Stambler <[email protected]>
1 parent b1fb54b commit cef11a9

File tree

3 files changed

+31
-2
lines changed

3 files changed

+31
-2
lines changed

gopls/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,5 @@ require (
1414
mvdan.cc/gofumpt v0.1.0
1515
mvdan.cc/xurls/v2 v2.2.0
1616
)
17+
18+
replace golang.org/x/tools => ../

gopls/internal/regtest/diagnostics/diagnostics_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,3 +1862,30 @@ package main
18621862
)
18631863
})
18641864
}
1865+
1866+
// Tests golang/go#45075, a panic in fillreturns breaks diagnostics.
1867+
func TestFillReturnsPanic(t *testing.T) {
1868+
// At tip, the panic no longer reproduces.
1869+
testenv.SkipAfterGo1Point(t, 16)
1870+
const files = `
1871+
-- go.mod --
1872+
module mod.com
1873+
1874+
go 1.16
1875+
-- main.go --
1876+
package main
1877+
1878+
1879+
func foo() int {
1880+
return x, nil
1881+
}
1882+
1883+
`
1884+
Run(t, files, func(t *testing.T, env *Env) {
1885+
env.OpenFile("main.go")
1886+
env.Await(
1887+
env.DiagnosticAtRegexpWithMessage("main.go", `return x`, "wrong number of return values"),
1888+
LogMatching(protocol.Error, `.*analysis fillreturns.*panicked.*`, 2),
1889+
)
1890+
})
1891+
}

internal/lsp/cache/analysis.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@ func runAnalysis(ctx context.Context, snapshot *snapshot, analyzer *analysis.Ana
212212
}
213213
defer func() {
214214
if r := recover(); r != nil {
215-
event.Log(ctx, fmt.Sprintf("analysis panicked: %s", r), tag.Package.Of(pkg.ID()))
216215
data.err = errors.Errorf("analysis %s for package %s panicked: %v", analyzer.Name, pkg.PkgPath(), r)
217216
}
218217
}()
@@ -408,7 +407,8 @@ func (s *snapshot) DiagnosePackage(ctx context.Context, spkg source.Package) (ma
408407
var err error
409408
errorAnalyzerDiag, err = s.Analyze(ctx, pkg.ID(), analyzers)
410409
if err != nil {
411-
return nil, err
410+
// Keep going: analysis failures should not block diagnostics.
411+
event.Error(ctx, "type error analysis failed", err, tag.Package.Of(pkg.ID()))
412412
}
413413
}
414414
diags := map[span.URI][]*source.Diagnostic{}

0 commit comments

Comments
 (0)