Skip to content

Commit

Permalink
x/tools: be defensive after types.Info.Types[expr] lookups
Browse files Browse the repository at this point in the history
This CL is the result of a quick audit
of x/tools for places that look in the Types map and do
not handle missing entries gracefully (unless dominated
by a check for welltypedness, such as RunDespiteErrors:false
in the analysis framework). In each case it either adds
a defensive check or documents the assumption.

See golang/go#69092 (comment)

Updates golang/go#69092

Change-Id: I3573512fd47ee4dca2e0b4bce2803b92424d7037
Reviewed-on: https://go-review.googlesource.com/c/tools/+/617416
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
  • Loading branch information
adonovan authored and gopherbot committed Oct 14, 2024
1 parent dec6bf1 commit 454be60
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 8 deletions.
4 changes: 3 additions & 1 deletion go/types/internal/play/play.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,10 @@ func handleSelectJSON(w http.ResponseWriter, req *http.Request) {
if tv.Value != nil {
fmt.Fprintf(out, ", and constant value %v", tv.Value)
}
fmt.Fprintf(out, "\n\n")
} else {
fmt.Fprintf(out, "%T has no type", innermostExpr)
}
fmt.Fprintf(out, "\n\n")
}

// selection x.f information (if cursor is over .f)
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/cache/testfuncs/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (b *indexBuilder) findSubtests(parent gobTest, typ *ast.FuncType, body *ast
continue // subtest has wrong signature
}

val := info.Types[call.Args[0]].Value
val := info.Types[call.Args[0]].Value // may be zero
if val == nil || val.Kind() != constant.String {
continue
}
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/completion/printf.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func printfArgKind(info *types.Info, call *ast.CallExpr, argIdx int) objKind {
}

// Format string must be a constant.
strArg := info.Types[call.Args[numParams-2]].Value
strArg := info.Types[call.Args[numParams-2]].Value // may be zero
if strArg == nil || strArg.Kind() != constant.String {
return kindAny
}
Expand Down
6 changes: 5 additions & 1 deletion gopls/internal/golang/freesymbols.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,11 @@ func freeRefs(pkg *types.Package, info *types.Info, file *ast.File, start, end t

if ref != nil {
ref.expr = n.(ast.Expr)
ref.typ = info.Types[n.(ast.Expr)].Type
if tv, ok := info.Types[ref.expr]; ok {
ref.typ = tv.Type
} else {
ref.typ = types.Typ[types.Invalid]
}
free = append(free, ref)
}

Expand Down
2 changes: 0 additions & 2 deletions gopls/internal/golang/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,6 @@ func highlightIdentifier(id *ast.Ident, file *ast.File, info *types.Info, result
highlightWriteInExpr(n.Chan)
case *ast.CompositeLit:
t := info.TypeOf(n)
// Every expression should have a type;
// work around https://github.com/golang/go/issues/69092.
if t == nil {
t = types.Typ[types.Invalid]
}
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Outer:
var typ types.Type
if assign, ok := ts.Assign.(*ast.AssignStmt); ok && len(assign.Rhs) == 1 {
if rhs := assign.Rhs[0].(*ast.TypeAssertExpr); ok {
typ = info.TypeOf(rhs.X)
typ = info.TypeOf(rhs.X) // may be nil
}
}
return objs, typ
Expand Down
3 changes: 3 additions & 0 deletions internal/refactor/inline/escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ func escape(info *types.Info, root ast.Node, f func(v *types.Var, escapes bool))
//
// We must traverse the normal terms and check
// whether any of them is an array.
//
// We assume TypeOf returns non-nil.
if _, ok := info.TypeOf(e.X).Underlying().(*types.Array); ok {
lvalue(e.X, escapes) // &a[i] on array
}
case *ast.SelectorExpr:
// We assume TypeOf returns non-nil.
if _, ok := info.TypeOf(e.X).Underlying().(*types.Struct); ok {
lvalue(e.X, escapes) // &s.f on struct
}
Expand Down
2 changes: 1 addition & 1 deletion internal/typeparams/genericfeatures/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func ForPackage(inspect *inspector.Inspector, info *types.Info) Features {
direct |= GenericFuncDecls
}
case *ast.InterfaceType:
tv := info.Types[n]
tv := info.Types[n] // may be zero
if iface, _ := tv.Type.(*types.Interface); iface != nil && !iface.IsMethodSet() {
direct |= EmbeddedTypeSets
}
Expand Down

0 comments on commit 454be60

Please sign in to comment.