Skip to content

Commit

Permalink
Remove util.TypeOf helper function (#242)
Browse files Browse the repository at this point in the history 
This PR simply removes the helper function `util.TypeOf`. Although it
shortens the call from `pass.TypesInfo.TypeOf`, it increases the
cognitive load such that we do not know which we should use in NilAway's
codebase and we don't know if `util.TypeOf` has any special handling (it
doesn't!). Moreover, we're using a mixture of these two
functionally-identical expressions in our codebase.

Removal of such a helper function reduces confusions and helps
maintainability.
  • Loading branch information
@yuxincs
yuxincs committed May 14, 2024
1 parent 3062237 commit a25d53b
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 37 deletions.
46 changes: 23 additions & 23 deletions assertion/affiliation/affiliation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
// note that other n-to-1 assignments (e.g. v, ok := m[k]) are handled by the loop below, since only the first LHS element is
// being directly assigned to in a way we care about
if len(node.Rhs) == 1 && len(node.Lhs) > 1 {
if rhsSig, ok := util.TypeOf(pass, node.Rhs[0]).(*types.Tuple); ok && rhsSig.Len() == len(node.Lhs) {
if rhsSig, ok := pass.TypesInfo.TypeOf(node.Rhs[0]).(*types.Tuple); ok && rhsSig.Len() == len(node.Lhs) {
for i := range node.Lhs {
lhsType := util.TypeOf(pass, node.Lhs[i])
lhsType := pass.TypesInfo.TypeOf(node.Lhs[i])
rhsType := rhsSig.At(i).Type()
appendTypeToTypeTriggers(lhsType, rhsType)
}
Expand All @@ -121,15 +121,15 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
}
// e.g., var i I, var s *S, i = s, or more generally, i1, i2, i3 = s1, s2, s3
for i := 0; i < len(node.Lhs) && i < len(node.Rhs); i++ {
lhsType := util.TypeOf(pass, node.Lhs[i])
rhsType := util.TypeOf(pass, node.Rhs[i])
lhsType := pass.TypesInfo.TypeOf(node.Lhs[i])
rhsType := pass.TypesInfo.TypeOf(node.Rhs[i])
appendTypeToTypeTriggers(lhsType, rhsType)
}
case *ast.ValueSpec:
// e.g., var i I = &S{}
for i := 0; i < len(node.Values); i++ {
lhsType := util.TypeOf(pass, node.Type)
rhsType := util.TypeOf(pass, node.Values[i])
lhsType := pass.TypesInfo.TypeOf(node.Type)
rhsType := pass.TypesInfo.TypeOf(node.Values[i])
appendTypeToTypeTriggers(lhsType, rhsType)
}
case *ast.CallExpr:
Expand All @@ -139,8 +139,8 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
if fdecl, ok := declObj.(*types.Func); ok {
fsig := fdecl.Type().(*types.Signature)
for i := 0; i < fsig.Params().Len() && i < len(node.Args); i++ {
lhsType := fsig.Params().At(i).Type() // receiver param of method declaration
rhsType := util.TypeOf(pass, node.Args[i]) // caller param
lhsType := fsig.Params().At(i).Type() // receiver param of method declaration
rhsType := pass.TypesInfo.TypeOf(node.Args[i]) // caller param
appendTypeToTypeTriggers(lhsType, rhsType)
}
}
Expand All @@ -151,15 +151,15 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
if sliceType, ok := util.IsSliceAppendCall(node, pass); ok {
for i := 1; i < len(node.Args); i++ {
lhsType := sliceType.Elem()
rhsType := util.TypeOf(pass, node.Args[i])
rhsType := pass.TypesInfo.TypeOf(node.Args[i])
appendTypeToTypeTriggers(lhsType, rhsType)
}
}

case *ast.TypeAssertExpr:
// e.g., v, ok := i.(*S)
lhsType := util.TypeOf(pass, node.X)
rhsType := util.TypeOf(pass, node.Type)
lhsType := pass.TypesInfo.TypeOf(node.X)
rhsType := pass.TypesInfo.TypeOf(node.Type)
appendTypeToTypeTriggers(lhsType, rhsType)

case *ast.ReturnStmt:
Expand All @@ -170,8 +170,8 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
funcSigResultsList := results.List
for i := range node.Results {
if i < len(funcSigResultsList) {
lhsType := util.TypeOf(pass, funcSigResultsList[i].Type)
rhsType := util.TypeOf(pass, node.Results[i])
lhsType := pass.TypesInfo.TypeOf(funcSigResultsList[i].Type)
rhsType := pass.TypesInfo.TypeOf(node.Results[i])
appendTypeToTypeTriggers(lhsType, rhsType)
}
}
Expand All @@ -184,19 +184,19 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
// e.g., _ = []I{&S{}}
// TODO: currently, nested composite literal for ArrayType is not supported (e.g., _ = [][]I{{&A1{}}}).
// Tracked in issue #46.
lhsType := util.TypeOf(pass, nodeType.Elt)
lhsType := pass.TypesInfo.TypeOf(nodeType.Elt)
for _, elt := range node.Elts {
appendTypeToTypeTriggers(lhsType, util.TypeOf(pass, elt))
appendTypeToTypeTriggers(lhsType, pass.TypesInfo.TypeOf(elt))
}
case *ast.MapType:
// Key, value, or both of a map declared of type interface, and initialized with a struct
// e.g., _ = map[int]I{0: &S{}}
keyType := util.TypeOf(pass, nodeType.Key)
valueType := util.TypeOf(pass, nodeType.Value)
keyType := pass.TypesInfo.TypeOf(nodeType.Key)
valueType := pass.TypesInfo.TypeOf(nodeType.Value)
for _, elt := range node.Elts {
if kv, ok := elt.(*ast.KeyValueExpr); ok {
appendTypeToTypeTriggers(keyType, util.TypeOf(pass, kv.Key))
appendTypeToTypeTriggers(valueType, util.TypeOf(pass, kv.Value))
appendTypeToTypeTriggers(keyType, pass.TypesInfo.TypeOf(kv.Key))
appendTypeToTypeTriggers(valueType, pass.TypesInfo.TypeOf(kv.Value))
}
}
case *ast.Ident:
Expand All @@ -208,13 +208,13 @@ func (a *Affiliation) computeTriggersForCastingSites(pass *analysis.Pass, upstre
var lhsType, rhsType types.Type
if kv, ok := elt.(*ast.KeyValueExpr); ok {
// In this case the initialization is key-value based. E.g. s = &S{t: &T{}}
lhsType = util.TypeOf(pass, kv.Key)
rhsType = util.TypeOf(pass, kv.Value)
lhsType = pass.TypesInfo.TypeOf(kv.Key)
rhsType = pass.TypesInfo.TypeOf(kv.Value)
} else {
// In this case the initialization is serial. E.g. s = &S{&T{}}
if sObj := util.TypeAsDeeplyStruct(util.TypeOf(pass, node)); sObj != nil {
if sObj := util.TypeAsDeeplyStruct(pass.TypesInfo.TypeOf(node)); sObj != nil {
lhsType = sObj.Field(i).Type()
rhsType = util.TypeOf(pass, elt)
rhsType = pass.TypesInfo.TypeOf(elt)
}
}
if lhsType != nil && rhsType != nil {
Expand Down
4 changes: 2 additions & 2 deletions assertion/function/assertiontree/backprop_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func typeIsString(t types.Type) bool {
// nilable(result 0)
func exprAsConsumedByAssignment(rootNode *RootAssertionNode, expr ast.Node) *annotation.ConsumeTrigger {
if exprType, ok := expr.(*ast.IndexExpr); ok {
t := util.TypeOf(rootNode.Pass(), exprType.X)
t := rootNode.Pass().TypesInfo.TypeOf(exprType.X)
if util.TypeIsDeeplyMap(t) {
return &annotation.ConsumeTrigger{
Annotation: &annotation.MapWrittenTo{ConsumeTriggerTautology: &annotation.ConsumeTriggerTautology{}},
Expand Down Expand Up @@ -828,7 +828,7 @@ func addReturnConsumers(rootNode *RootAssertionNode, node *ast.ReturnStmt, expr
// return s // <-- track shallow and deep nilability of `s` here
// }
// ```
if util.TypeIsDeep(util.TypeOf(rootNode.Pass(), expr)) {
if util.TypeIsDeep(rootNode.Pass().TypesInfo.TypeOf(expr)) {
producer := &annotation.ProduceTrigger{
Annotation: exprAsDeepProducer(rootNode, expr),
Expr: expr,
Expand Down
3 changes: 1 addition & 2 deletions assertion/function/assertiontree/parse_expr_producer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,7 @@ func (r *RootAssertionNode) ParseExprAsProducer(expr ast.Expr, doNotTrack bool)
}
if expr.Op == token.AND {
// we treat a struct object pointer (e.g., &A{}) and struct object (e.g., A{}) identically for creating field producers
t := util.TypeOf(r.Pass(), expr.X)
if s := util.TypeAsDeeplyStruct(t); s != nil {
if s := util.TypeAsDeeplyStruct(r.Pass().TypesInfo.TypeOf(expr.X)); s != nil {
return r.ParseExprAsProducer(expr.X, doNotTrack)
}
}
Expand Down
5 changes: 2 additions & 3 deletions assertion/function/assertiontree/root_assertion_node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ func (r *RootAssertionNode) AddComputation(expr ast.Expr) {
// foo(s) // <-- track shallow and deep nilability of `s` here
// }
// ```
if util.TypeIsDeep(util.TypeOf(r.Pass(), arg)) {
if util.TypeIsDeep(r.Pass().TypesInfo.TypeOf(arg)) {
deepProducer := &annotation.ProduceTrigger{
Annotation: exprAsDeepProducer(r, arg),
Expr: arg,
Expand Down Expand Up @@ -802,9 +802,8 @@ func (r *RootAssertionNode) AddComputation(expr ast.Expr) {
if util.TypeIsDeeplyPtr(recv.Type()) { // Check 2: receiver is a pointer receiver
conf := r.Pass().ResultOf[config.Analyzer].(*config.Config)
if conf.IsPkgInScope(funcObj.Pkg()) { // Check 3: invoked method is in scope
t := util.TypeOf(r.Pass(), expr.X)
// Here, `t` can only be of type interface, struct, or named, of which we only support for struct and named types.
if !util.TypeIsDeeplyInterface(t) { // Check 4: invoking expression (caller) is of a non-interface type (e.g., struct or named)
if !util.TypeIsDeeplyInterface(r.Pass().TypesInfo.TypeOf(expr.X)) { // Check 4: invoking expression (caller) is of a non-interface type (e.g., struct or named)
allowNilable = true
// We are in the special case of supporting nilable receivers! Can be nilable depending on declaration annotation/inferred nilability.
r.AddConsumption(&annotation.ConsumeTrigger{
Expand Down
2 changes: 1 addition & 1 deletion assertion/global/globalvarinit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func getGlobalConsumers(pass *analysis.Pass, valspec *ast.ValueSpec) []*annotati

for i, name := range valspec.Names {
// Types that are not nilable are eliminated here
if !util.TypeBarsNilness(util.TypeOf(pass, name)) && !util.IsEmptyExpr(name) {
if !util.TypeBarsNilness(pass.TypesInfo.TypeOf(name)) && !util.IsEmptyExpr(name) {
v := pass.TypesInfo.ObjectOf(name).(*types.Var)
consumers[i] = &annotation.ConsumeTrigger{
Annotation: &annotation.GlobalVarAssign{
Expand Down
7 changes: 1 addition & 6 deletions util/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,6 @@ func UnwrapPtr(t types.Type) types.Type {
return t
}

// TypeOf returns the type of the passed AST expression
func TypeOf(pass *analysis.Pass, expr ast.Expr) types.Type {
return pass.TypesInfo.TypeOf(expr)
}

// FuncIdentFromCallExpr return a function identified from a call expression, nil otherwise
// nilable(result 0)
func FuncIdentFromCallExpr(expr *ast.CallExpr) *ast.Ident {
Expand Down Expand Up @@ -242,7 +237,7 @@ func IsSliceAppendCall(node *ast.CallExpr, pass *analysis.Pass) (*types.Slice, b
if funcName, ok := node.Fun.(*ast.Ident); ok {
if declObj := pass.TypesInfo.Uses[funcName]; declObj != nil {
if declObj.String() == "builtin append" {
if sliceType, ok := TypeOf(pass, node.Args[0]).(*types.Slice); ok {
if sliceType, ok := pass.TypesInfo.TypeOf(node.Args[0]).(*types.Slice); ok {
return sliceType, true
}
}
Expand Down

0 comments on commit a25d53b

Please sign in to comment.