From 3eb8a8f04e850749214aad381b203dfc1f3a5916 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Wed, 31 Aug 2022 15:29:37 -0400
Subject: [PATCH] internal/lsp/cache: delete misleading Package.IllTyped method

This method does not in fact report whether the package is ill-typed.
It actually reports whether any of three fields of pkg are nil.
They never are.
This does rather explain why type-error analyzers are executed
on (genuinely) ill-typed packages, despite the !IllTyped()
condition. And why analyzers that aren't prepared to handle
ill-typed code (!RunDespiteErrors) sometimes crash in production.

Updates golang/go#54762

Change-Id: I95421584bec68fb849b5ed52cc4e6d9b6bb679be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426802
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
---
 internal/lsp/cache/analysis.go | 8 +++-----
 internal/lsp/cache/cache.go    | 9 ++-------
 internal/lsp/cache/pkg.go      | 4 ----
 internal/lsp/source/rename.go  | 5 +----
 internal/lsp/source/view.go    | 1 -
 5 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index fa10cc3d548..5a29feffa7f 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -329,11 +329,9 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a
 	}
 	analysisinternal.SetTypeErrors(pass, pkg.typeErrors)
 
-	// We never run analyzers on ill-typed code,
-	// even those marked RunDespiteErrors=true.
-	if pkg.IsIllTyped() {
-		return nil, fmt.Errorf("analysis skipped due to errors in package")
-	}
+	// TODO(adonovan): fix: don't run analyzers on packages
+	// containing type errors unless the analyzers have
+	// RunDespiteErrors=true.
 
 	// Recover from panics (only) within the analyzer logic.
 	// (Use an anonymous function to limit the recover scope.)
diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go
index c002850653b..eea302187d5 100644
--- a/internal/lsp/cache/cache.go
+++ b/internal/lsp/cache/cache.go
@@ -226,13 +226,8 @@ func (c *Cache) PackageStats(withNames bool) template.HTML {
 			if v.pkg == nil {
 				break
 			}
-			var typsCost, typInfoCost int64
-			if v.pkg.types != nil {
-				typsCost = typesCost(v.pkg.types.Scope())
-			}
-			if v.pkg.typesInfo != nil {
-				typInfoCost = typesInfoCost(v.pkg.typesInfo)
-			}
+			typsCost := typesCost(v.pkg.types.Scope())
+			typInfoCost := typesInfoCost(v.pkg.typesInfo)
 			stat := packageStat{
 				id:        v.pkg.m.ID,
 				mode:      v.pkg.mode,
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 1217ec29fd4..3478c58ad1f 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -93,10 +93,6 @@ func (p *pkg) GetTypesSizes() types.Sizes {
 	return p.typesSizes
 }
 
-func (p *pkg) IsIllTyped() bool {
-	return p.types == nil || p.typesInfo == nil || p.typesSizes == nil
-}
-
 func (p *pkg) ForTest() string {
 	return string(p.m.ForTest)
 }
diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 6bbe91afae6..85d7c12f945 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -158,7 +158,7 @@ func Rename(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position,
 		return nil, err
 	}
 
-	obj, pkg := qos[0].obj, qos[0].pkg
+	obj := qos[0].obj
 
 	if err := checkRenamable(obj); err != nil {
 		return nil, err
@@ -169,9 +169,6 @@ func Rename(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position,
 	if !isValidIdentifier(newName) {
 		return nil, fmt.Errorf("invalid identifier to rename: %q", newName)
 	}
-	if pkg == nil || pkg.IsIllTyped() {
-		return nil, fmt.Errorf("package for %s is ill typed", f.URI())
-	}
 	refs, err := references(ctx, s, qos, true, false, true)
 	if err != nil {
 		return nil, err
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index a188e3666b1..498299e1f7f 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -639,7 +639,6 @@ type Package interface {
 	GetTypes() *types.Package
 	GetTypesInfo() *types.Info
 	GetTypesSizes() types.Sizes
-	IsIllTyped() bool
 	ForTest() string
 	GetImport(pkgPath string) (Package, error)
 	MissingDependencies() []string