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 <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Alan Donovan <[email protected]>
golang · Aug 31, 2022 · 3eb8a8f · 3eb8a8f
1 parent cb91d6c
commit 3eb8a8f
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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