diff --git a/annotation/consume_trigger.go b/annotation/consume_trigger.go index c8ec3a26..161eb51c 100644 --- a/annotation/consume_trigger.go +++ b/annotation/consume_trigger.go @@ -22,6 +22,7 @@ import ( "strings" "go.uber.org/nilaway/util" + "go.uber.org/nilaway/util/orderedmap" ) // A ConsumingAnnotationTrigger indicated a possible reason that a nil flow to this site would indicate @@ -53,6 +54,12 @@ type ConsumingAnnotationTrigger interface { // Copy returns a deep copy of this ConsumingAnnotationTrigger Copy() ConsumingAnnotationTrigger + + // AddAssignment adds an assignment to the trigger for tracking and printing informative error message. + // NilAway's `backpropAcrossOneToOneAssignment()` lifts consumer triggers from the RHS of an assignment to the LHS. + // This implies loss of information about the assignment. This method is used to track such assignments and print + // a more informative error message. + AddAssignment(Assignment) } // customPos has the below default implementations, in which case ConsumeTrigger.Pos() will return a default value. @@ -68,9 +75,85 @@ type Prestring interface { String() string } +// Assignment is a struct that represents an assignment to an expression +type Assignment struct { + LHSExprStr string + RHSExprStr string + Position token.Position +} + +func (a *Assignment) String() string { + return fmt.Sprintf("`%s` to `%s` at %s", a.RHSExprStr, a.LHSExprStr, a.Position) +} + +// assignmentFlow is a struct that represents a flow of assignments. +// Note that we implement a copy method for this struct, since we want to deep copy the assignments map when we copy +// ConsumerTriggers. However, we don't implement an `equals` method for this struct, since it would incur a performance +// penalty in situations where multiple nilable flows reach a dereference site by creating more full triggers and possibly +// more rounds through backpropagation fix point. Consider the following example: +// +// func f(m map[int]*int) { +// var v *int +// var ok1, ok2 bool +// if cond { +// v, ok1 = m[0] // nilable flow 1, ok1 is false +// } else { +// v, ok2 = m[1] // nilable flow 2, ok2 is false +// } +// _, _ = ok1, ok2 +// _ = *v // nil panic! +// } +// +// Here `v` can be potentiall nilable from two flows: ok1 or ok2 is false. We would like to print only one error message +// for this situation with one representative flow printed in the error message. However, with an `equals` method, we would +// report multiple error messages, one for each flow, by creating multiple full triggers, thereby affecting performance. +type assignmentFlow struct { + // We use ordered map for `assignments` to maintain the order of assignments in the flow, and also to avoid + // duplicates that can get introduced due to fix point convergence in backpropagation. + assignments *orderedmap.OrderedMap[Assignment, bool] +} + +func (a *assignmentFlow) addEntry(entry Assignment) { + if a.assignments == nil { + a.assignments = orderedmap.New[Assignment, bool]() + } + a.assignments.Store(entry, true) +} + +func (a *assignmentFlow) copy() assignmentFlow { + if a.assignments == nil { + return assignmentFlow{} + } + assignments := orderedmap.New[Assignment, bool]() + for _, p := range a.assignments.Pairs { + assignments.Store(p.Key, true) + } + return assignmentFlow{assignments: assignments} +} + +func (a *assignmentFlow) String() string { + if a.assignments == nil || len(a.assignments.Pairs) == 0 { + return "" + } + + // backprop algorithm populates assignment entries in backward order. Reverse entries to get forward order of + // assignments, and store in `strs` slice. + strs := make([]string, 0, len(a.assignments.Pairs)) + for i := len(a.assignments.Pairs) - 1; i >= 0; i-- { + strs = append(strs, a.assignments.Pairs[i].Key.String()) + } + + // build the informative print string tracking the assignments + var sb strings.Builder + sb.WriteString(" via the assignment(s):\n\t\t-> ") + sb.WriteString(strings.Join(strs, ",\n\t\t-> ")) + return sb.String() +} + // TriggerIfNonNil is triggered if the contained Annotation is non-nil type TriggerIfNonNil struct { Ann Key + assignmentFlow } // Kind returns Conditional. @@ -97,24 +180,38 @@ func (t *TriggerIfNonNil) equals(other ConsumingAnnotationTrigger) bool { func (t *TriggerIfNonNil) Copy() ConsumingAnnotationTrigger { copyConsumer := *t copyConsumer.Ann = t.Ann.copy() + copyConsumer.assignmentFlow = t.assignmentFlow.copy() return ©Consumer } +// AddAssignment adds an assignment to the trigger. +func (t *TriggerIfNonNil) AddAssignment(e Assignment) { + t.assignmentFlow.addEntry(e) +} + // Prestring returns this Prestring as a Prestring -func (*TriggerIfNonNil) Prestring() Prestring { - return TriggerIfNonNilPrestring{} +func (t *TriggerIfNonNil) Prestring() Prestring { + return TriggerIfNonNilPrestring{ + AssignmentStr: t.assignmentFlow.String(), + } } // TriggerIfNonNilPrestring is a Prestring storing the needed information to compactly encode a TriggerIfNonNil -type TriggerIfNonNilPrestring struct{} +type TriggerIfNonNilPrestring struct { + AssignmentStr string +} -func (TriggerIfNonNilPrestring) String() string { - return "nonnil value" +func (t TriggerIfNonNilPrestring) String() string { + var sb strings.Builder + sb.WriteString("nonnil value") + sb.WriteString(t.AssignmentStr) + return sb.String() } // TriggerIfDeepNonNil is triggered if the contained Annotation is deeply non-nil type TriggerIfDeepNonNil struct { Ann Key + assignmentFlow } // Kind returns DeepConditional. @@ -141,23 +238,38 @@ func (t *TriggerIfDeepNonNil) equals(other ConsumingAnnotationTrigger) bool { func (t *TriggerIfDeepNonNil) Copy() ConsumingAnnotationTrigger { copyConsumer := *t copyConsumer.Ann = t.Ann.copy() + copyConsumer.assignmentFlow = t.assignmentFlow.copy() return ©Consumer } +// AddAssignment adds an assignment to the trigger. +func (t *TriggerIfDeepNonNil) AddAssignment(e Assignment) { + t.assignmentFlow.addEntry(e) +} + // Prestring returns this Prestring as a Prestring -func (*TriggerIfDeepNonNil) Prestring() Prestring { - return TriggerIfDeepNonNilPrestring{} +func (t *TriggerIfDeepNonNil) Prestring() Prestring { + return TriggerIfDeepNonNilPrestring{ + AssignmentStr: t.assignmentFlow.String(), + } } // TriggerIfDeepNonNilPrestring is a Prestring storing the needed information to compactly encode a TriggerIfDeepNonNil -type TriggerIfDeepNonNilPrestring struct{} +type TriggerIfDeepNonNilPrestring struct { + AssignmentStr string +} -func (TriggerIfDeepNonNilPrestring) String() string { - return "deeply nonnil value" +func (t TriggerIfDeepNonNilPrestring) String() string { + var sb strings.Builder + sb.WriteString("deeply nonnil value") + sb.WriteString(t.AssignmentStr) + return sb.String() } // ConsumeTriggerTautology is used at consumption sites were consuming nil is always an error -type ConsumeTriggerTautology struct{} +type ConsumeTriggerTautology struct { + assignmentFlow +} // Kind returns Always. func (*ConsumeTriggerTautology) Kind() TriggerKind { return Always } @@ -177,19 +289,32 @@ func (*ConsumeTriggerTautology) equals(other ConsumingAnnotationTrigger) bool { // Copy returns a deep copy of this ConsumingAnnotationTrigger func (t *ConsumeTriggerTautology) Copy() ConsumingAnnotationTrigger { copyConsumer := *t + copyConsumer.assignmentFlow = t.assignmentFlow.copy() return ©Consumer } +// AddAssignment adds an assignment to the trigger. +func (t *ConsumeTriggerTautology) AddAssignment(e Assignment) { + t.assignmentFlow.addEntry(e) +} + // Prestring returns this Prestring as a Prestring -func (*ConsumeTriggerTautology) Prestring() Prestring { - return ConsumeTriggerTautologyPrestring{} +func (t *ConsumeTriggerTautology) Prestring() Prestring { + return ConsumeTriggerTautologyPrestring{ + AssignmentStr: t.assignmentFlow.String(), + } } // ConsumeTriggerTautologyPrestring is a Prestring storing the needed information to compactly encode a ConsumeTriggerTautology -type ConsumeTriggerTautologyPrestring struct{} +type ConsumeTriggerTautologyPrestring struct { + AssignmentStr string +} -func (ConsumeTriggerTautologyPrestring) String() string { - return "must be nonnil" +func (c ConsumeTriggerTautologyPrestring) String() string { + var sb strings.Builder + sb.WriteString("must be nonnil") + sb.WriteString(c.AssignmentStr) + return sb.String() } // PtrLoad is when a value flows to a point where it is loaded as a pointer @@ -214,14 +339,21 @@ func (p *PtrLoad) Copy() ConsumingAnnotationTrigger { // Prestring returns this PtrLoad as a Prestring func (p *PtrLoad) Prestring() Prestring { - return PtrLoadPrestring{} + return PtrLoadPrestring{ + AssignmentStr: p.assignmentFlow.String(), + } } // PtrLoadPrestring is a Prestring storing the needed information to compactly encode a PtrLoad -type PtrLoadPrestring struct{} +type PtrLoadPrestring struct { + AssignmentStr string +} -func (PtrLoadPrestring) String() string { - return "dereferenced" +func (p PtrLoadPrestring) String() string { + var sb strings.Builder + sb.WriteString("dereferenced") + sb.WriteString(p.AssignmentStr) + return sb.String() } // MapAccess is when a map value flows to a point where it is indexed, and thus must be non-nil @@ -248,14 +380,21 @@ func (i *MapAccess) Copy() ConsumingAnnotationTrigger { // Prestring returns this MapAccess as a Prestring func (i *MapAccess) Prestring() Prestring { - return MapAccessPrestring{} + return MapAccessPrestring{ + AssignmentStr: i.assignmentFlow.String(), + } } // MapAccessPrestring is a Prestring storing the needed information to compactly encode a MapAccess -type MapAccessPrestring struct{} +type MapAccessPrestring struct { + AssignmentStr string +} -func (MapAccessPrestring) String() string { - return "keyed into" +func (i MapAccessPrestring) String() string { + var sb strings.Builder + sb.WriteString("keyed into") + sb.WriteString(i.AssignmentStr) + return sb.String() } // MapWrittenTo is when a map value flows to a point where one of its indices is written to, and thus @@ -281,14 +420,21 @@ func (m *MapWrittenTo) Copy() ConsumingAnnotationTrigger { // Prestring returns this MapWrittenTo as a Prestring func (m *MapWrittenTo) Prestring() Prestring { - return MapWrittenToPrestring{} + return MapWrittenToPrestring{ + AssignmentStr: m.assignmentFlow.String(), + } } // MapWrittenToPrestring is a Prestring storing the needed information to compactly encode a MapWrittenTo -type MapWrittenToPrestring struct{} +type MapWrittenToPrestring struct { + AssignmentStr string +} -func (MapWrittenToPrestring) String() string { - return "written to at an index" +func (m MapWrittenToPrestring) String() string { + var sb strings.Builder + sb.WriteString("written to at an index") + sb.WriteString(m.AssignmentStr) + return sb.String() } // SliceAccess is when a slice value flows to a point where it is sliced, and thus must be non-nil @@ -313,14 +459,21 @@ func (s *SliceAccess) Copy() ConsumingAnnotationTrigger { // Prestring returns this SliceAccess as a Prestring func (s *SliceAccess) Prestring() Prestring { - return SliceAccessPrestring{} + return SliceAccessPrestring{ + AssignmentStr: s.assignmentFlow.String(), + } } // SliceAccessPrestring is a Prestring storing the needed information to compactly encode a SliceAccess -type SliceAccessPrestring struct{} +type SliceAccessPrestring struct { + AssignmentStr string +} -func (SliceAccessPrestring) String() string { - return "sliced into" +func (s SliceAccessPrestring) String() string { + var sb strings.Builder + sb.WriteString("sliced into") + sb.WriteString(s.AssignmentStr) + return sb.String() } // FldAccess is when a value flows to a point where a field of it is accessed, and so it must be non-nil @@ -358,22 +511,28 @@ func (f *FldAccess) Prestring() Prestring { } return FldAccessPrestring{ - FieldName: fieldName, - MethodName: methodName, + FieldName: fieldName, + MethodName: methodName, + AssignmentStr: f.assignmentFlow.String(), } } // FldAccessPrestring is a Prestring storing the needed information to compactly encode a FldAccess type FldAccessPrestring struct { - FieldName string - MethodName string + FieldName string + MethodName string + AssignmentStr string } func (f FldAccessPrestring) String() string { + var sb strings.Builder if f.MethodName != "" { - return fmt.Sprintf("called `%s()`", f.MethodName) + sb.WriteString(fmt.Sprintf("called `%s()`", f.MethodName)) + } else { + sb.WriteString(fmt.Sprintf("accessed field `%s`", f.FieldName)) } - return fmt.Sprintf("accessed field `%s`", f.FieldName) + sb.WriteString(f.AssignmentStr) + return sb.String() } // UseAsErrorResult is when a value flows to the error result of a function, where it is expected to be non-nil @@ -409,6 +568,7 @@ func (u *UseAsErrorResult) Prestring() Prestring { ReturningFuncStr: retAnn.FuncDecl.Name(), IsNamedReturn: u.IsNamedReturn, RetName: retAnn.FuncDecl.Type().(*types.Signature).Results().At(retAnn.RetNum).Name(), + AssignmentStr: u.assignmentFlow.String(), } } @@ -418,13 +578,18 @@ type UseAsErrorResultPrestring struct { ReturningFuncStr string IsNamedReturn bool RetName string + AssignmentStr string } func (u UseAsErrorResultPrestring) String() string { + var sb strings.Builder if u.IsNamedReturn { - return fmt.Sprintf("returned as named error result `%s` of `%s()`", u.RetName, u.ReturningFuncStr) + sb.WriteString(fmt.Sprintf("returned as named error result `%s` of `%s()`", u.RetName, u.ReturningFuncStr)) + } else { + sb.WriteString(fmt.Sprintf("returned as error result %d of `%s()`", u.Pos, u.ReturningFuncStr)) } - return fmt.Sprintf("returned as error result %d of `%s()`", u.Pos, u.ReturningFuncStr) + sb.WriteString(u.AssignmentStr) + return sb.String() } // overriding position value to point to the raw return statement, which is the source of the potential error @@ -459,17 +624,22 @@ func (f *FldAssign) Copy() ConsumingAnnotationTrigger { func (f *FldAssign) Prestring() Prestring { fldAnn := f.Ann.(*FieldAnnotationKey) return FldAssignPrestring{ - FieldName: fldAnn.FieldDecl.Name(), + FieldName: fldAnn.FieldDecl.Name(), + AssignmentStr: f.assignmentFlow.String(), } } // FldAssignPrestring is a Prestring storing the needed information to compactly encode a FldAssign type FldAssignPrestring struct { - FieldName string + FieldName string + AssignmentStr string } func (f FldAssignPrestring) String() string { - return fmt.Sprintf("assigned into field `%s`", f.FieldName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into field `%s`", f.FieldName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // ArgFldPass is when a struct field value (A.f) flows to a point where it is passed to a function with a param of @@ -503,33 +673,40 @@ func (f *ArgFldPass) Prestring() Prestring { } return ArgFldPassPrestring{ - FieldName: ann.FieldDecl.Name(), - FuncName: ann.FuncDecl.Name(), - ParamNum: ann.ParamNum, - RecvName: recvName, - IsPassed: f.IsPassed, + FieldName: ann.FieldDecl.Name(), + FuncName: ann.FuncDecl.Name(), + ParamNum: ann.ParamNum, + RecvName: recvName, + IsPassed: f.IsPassed, + AssignmentStr: f.assignmentFlow.String(), } } // ArgFldPassPrestring is a Prestring storing the needed information to compactly encode a ArgFldPass type ArgFldPassPrestring struct { - FieldName string - FuncName string - ParamNum int - RecvName string - IsPassed bool + FieldName string + FuncName string + ParamNum int + RecvName string + IsPassed bool + AssignmentStr string } func (f ArgFldPassPrestring) String() string { + var sb strings.Builder prefix := "" if f.IsPassed { prefix = "assigned to " } if len(f.RecvName) > 0 { - return fmt.Sprintf("%sfield `%s` of method receiver `%s`", prefix, f.FieldName, f.RecvName) + sb.WriteString(fmt.Sprintf("%sfield `%s` of method receiver `%s`", prefix, f.FieldName, f.RecvName)) + } else { + sb.WriteString(fmt.Sprintf("%sfield `%s` of argument %d to `%s()`", prefix, f.FieldName, f.ParamNum, f.FuncName)) } - return fmt.Sprintf("%sfield `%s` of argument %d to `%s()`", prefix, f.FieldName, f.ParamNum, f.FuncName) + + sb.WriteString(f.AssignmentStr) + return sb.String() } // GlobalVarAssign is when a value flows to a point where it is assigned into a global variable @@ -556,17 +733,22 @@ func (g *GlobalVarAssign) Copy() ConsumingAnnotationTrigger { func (g *GlobalVarAssign) Prestring() Prestring { varAnn := g.Ann.(*GlobalVarAnnotationKey) return GlobalVarAssignPrestring{ - VarName: varAnn.VarDecl.Name(), + VarName: varAnn.VarDecl.Name(), + AssignmentStr: g.assignmentFlow.String(), } } // GlobalVarAssignPrestring is a Prestring storing the needed information to compactly encode a GlobalVarAssign type GlobalVarAssignPrestring struct { - VarName string + VarName string + AssignmentStr string } func (g GlobalVarAssignPrestring) String() string { - return fmt.Sprintf("assigned into global variable `%s`", g.VarName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into global variable `%s`", g.VarName)) + sb.WriteString(g.AssignmentStr) + return sb.String() } // ArgPass is when a value flows to a point where it is passed as an argument to a function. This @@ -599,15 +781,17 @@ func (a *ArgPass) Prestring() Prestring { switch key := a.Ann.(type) { case *ParamAnnotationKey: return ArgPassPrestring{ - ParamName: key.MinimalString(), - FuncName: key.FuncDecl.Name(), - Location: "", + ParamName: key.MinimalString(), + FuncName: key.FuncDecl.Name(), + Location: "", + AssignmentStr: a.assignmentFlow.String(), } case *CallSiteParamAnnotationKey: return ArgPassPrestring{ - ParamName: key.MinimalString(), - FuncName: key.FuncDecl.Name(), - Location: key.Location.String(), + ParamName: key.MinimalString(), + FuncName: key.FuncDecl.Name(), + Location: key.Location.String(), + AssignmentStr: a.assignmentFlow.String(), } default: panic(fmt.Sprintf( @@ -621,7 +805,8 @@ type ArgPassPrestring struct { FuncName string // Location points to the code location of the argument pass at the call site for a ArgPass // enclosing CallSiteParamAnnotationKey; Location is empty for a ArgPass enclosing ParamAnnotationKey. - Location string + Location string + AssignmentStr string } func (a ArgPassPrestring) String() string { @@ -630,6 +815,7 @@ func (a ArgPassPrestring) String() string { if a.Location != "" { sb.WriteString(fmt.Sprintf(" at %s", a.Location)) } + sb.WriteString(a.AssignmentStr) return sb.String() } @@ -658,17 +844,22 @@ func (a *RecvPass) Copy() ConsumingAnnotationTrigger { func (a *RecvPass) Prestring() Prestring { recvAnn := a.Ann.(*RecvAnnotationKey) return RecvPassPrestring{ - FuncName: recvAnn.FuncDecl.Name(), + FuncName: recvAnn.FuncDecl.Name(), + AssignmentStr: a.assignmentFlow.String(), } } // RecvPassPrestring is a Prestring storing the needed information to compactly encode a RecvPass type RecvPassPrestring struct { - FuncName string + FuncName string + AssignmentStr string } func (a RecvPassPrestring) String() string { - return fmt.Sprintf("used as receiver to call `%s()`", a.FuncName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("used as receiver to call `%s()`", a.FuncName)) + sb.WriteString(a.AssignmentStr) + return sb.String() } // InterfaceResultFromImplementation is when a result is determined to flow from a concrete method to an interface method via implementation @@ -701,19 +892,24 @@ func (i *InterfaceResultFromImplementation) Prestring() Prestring { retAnn.RetNum, util.PartiallyQualifiedFuncName(retAnn.FuncDecl), util.PartiallyQualifiedFuncName(i.ImplementingMethod), + i.assignmentFlow.String(), } } // InterfaceResultFromImplementationPrestring is a Prestring storing the needed information to compactly encode a InterfaceResultFromImplementation type InterfaceResultFromImplementationPrestring struct { - RetNum int - IntName string - ImplName string + RetNum int + IntName string + ImplName string + AssignmentStr string } func (i InterfaceResultFromImplementationPrestring) String() string { - return fmt.Sprintf("returned as result %d from interface method `%s()` (implemented by `%s()`)", - i.RetNum, i.IntName, i.ImplName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("returned as result %d from interface method `%s()` (implemented by `%s()`)", + i.RetNum, i.IntName, i.ImplName)) + sb.WriteString(i.AssignmentStr) + return sb.String() } // MethodParamFromInterface is when a param flows from an interface method to a concrete method via implementation @@ -746,19 +942,24 @@ func (m *MethodParamFromInterface) Prestring() Prestring { paramAnn.ParamNameString(), util.PartiallyQualifiedFuncName(paramAnn.FuncDecl), util.PartiallyQualifiedFuncName(m.InterfaceMethod), + m.assignmentFlow.String(), } } // MethodParamFromInterfacePrestring is a Prestring storing the needed information to compactly encode a MethodParamFromInterface type MethodParamFromInterfacePrestring struct { - ParamName string - ImplName string - IntName string + ParamName string + ImplName string + IntName string + AssignmentStr string } func (m MethodParamFromInterfacePrestring) String() string { - return fmt.Sprintf("passed as parameter `%s` to `%s()` (implementing `%s()`)", - m.ParamName, m.ImplName, m.IntName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("passed as parameter `%s` to `%s()` (implementing `%s()`)", + m.ParamName, m.ImplName, m.IntName)) + sb.WriteString(m.AssignmentStr) + return sb.String() } // DuplicateReturnConsumer duplicates a given consume trigger, assuming the given consumer trigger @@ -817,6 +1018,7 @@ func (u *UseAsReturn) Prestring() Prestring { u.IsNamedReturn, key.FuncDecl.Type().(*types.Signature).Results().At(key.RetNum).Name(), "", + u.assignmentFlow.String(), } case *CallSiteRetAnnotationKey: return UseAsReturnPrestring{ @@ -825,6 +1027,7 @@ func (u *UseAsReturn) Prestring() Prestring { u.IsNamedReturn, key.FuncDecl.Type().(*types.Signature).Results().At(key.RetNum).Name(), key.Location.String(), + u.assignmentFlow.String(), } default: panic(fmt.Sprintf("Expected RetAnnotationKey or CallSiteRetAnnotationKey but got: %T", key)) @@ -840,7 +1043,8 @@ type UseAsReturnPrestring struct { // Location is empty for a UseAsReturn enclosing RetAnnotationKey. Location points to the // location of the result at the call site for a UseAsReturn enclosing // CallSiteRetAnnotationKey. - Location string + Location string + AssignmentStr string } func (u UseAsReturnPrestring) String() string { @@ -854,6 +1058,7 @@ func (u UseAsReturnPrestring) String() string { if u.Location != "" { sb.WriteString(fmt.Sprintf(" at %s", u.Location)) } + sb.WriteString(u.AssignmentStr) return sb.String() } @@ -893,18 +1098,23 @@ func (u *UseAsFldOfReturn) Prestring() Prestring { retAnn.FuncDecl.Name(), retAnn.FieldDecl.Name(), retAnn.RetNum, + u.assignmentFlow.String(), } } // UseAsFldOfReturnPrestring is a Prestring storing the needed information to compactly encode a UseAsFldOfReturn type UseAsFldOfReturnPrestring struct { - FuncName string - FieldName string - RetNum int + FuncName string + FieldName string + RetNum int + AssignmentStr string } func (u UseAsFldOfReturnPrestring) String() string { - return fmt.Sprintf("field `%s` returned by result %d of `%s()`", u.FieldName, u.RetNum, u.FuncName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("field `%s` returned by result %d of `%s()`", u.FieldName, u.RetNum, u.FuncName)) + sb.WriteString(u.AssignmentStr) + return sb.String() } // GetRetFldConsumer returns the UseAsFldOfReturn consume trigger with given retKey and expr @@ -968,16 +1178,21 @@ func (f *SliceAssign) Prestring() Prestring { fldAnn := f.Ann.(*TypeNameAnnotationKey) return SliceAssignPrestring{ fldAnn.TypeDecl.Name(), + f.assignmentFlow.String(), } } // SliceAssignPrestring is a Prestring storing the needed information to compactly encode a SliceAssign type SliceAssignPrestring struct { - TypeName string + TypeName string + AssignmentStr string } func (f SliceAssignPrestring) String() string { - return fmt.Sprintf("assigned into a slice of deeply nonnil type `%s`", f.TypeName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into a slice of deeply nonnil type `%s`", f.TypeName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // ArrayAssign is when a value flows to a point where it is assigned into an array @@ -1005,16 +1220,21 @@ func (a *ArrayAssign) Prestring() Prestring { fldAnn := a.Ann.(*TypeNameAnnotationKey) return ArrayAssignPrestring{ fldAnn.TypeDecl.Name(), + a.assignmentFlow.String(), } } // ArrayAssignPrestring is a Prestring storing the needed information to compactly encode a SliceAssign type ArrayAssignPrestring struct { - TypeName string + TypeName string + AssignmentStr string } func (a ArrayAssignPrestring) String() string { - return fmt.Sprintf("assigned into an array of deeply nonnil type `%s`", a.TypeName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into an array of deeply nonnil type `%s`", a.TypeName)) + sb.WriteString(a.AssignmentStr) + return sb.String() } // PtrAssign is when a value flows to a point where it is assigned into a pointer @@ -1042,16 +1262,21 @@ func (f *PtrAssign) Prestring() Prestring { fldAnn := f.Ann.(*TypeNameAnnotationKey) return PtrAssignPrestring{ fldAnn.TypeDecl.Name(), + f.assignmentFlow.String(), } } // PtrAssignPrestring is a Prestring storing the needed information to compactly encode a PtrAssign type PtrAssignPrestring struct { - TypeName string + TypeName string + AssignmentStr string } func (f PtrAssignPrestring) String() string { - return fmt.Sprintf("assigned into a pointer of deeply nonnil type `%s`", f.TypeName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into a pointer of deeply nonnil type `%s`", f.TypeName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // MapAssign is when a value flows to a point where it is assigned into an annotated map @@ -1079,16 +1304,21 @@ func (f *MapAssign) Prestring() Prestring { fldAnn := f.Ann.(*TypeNameAnnotationKey) return MapAssignPrestring{ fldAnn.TypeDecl.Name(), + f.assignmentFlow.String(), } } // MapAssignPrestring is a Prestring storing the needed information to compactly encode a MapAssign type MapAssignPrestring struct { - TypeName string + TypeName string + AssignmentStr string } func (f MapAssignPrestring) String() string { - return fmt.Sprintf("assigned into a map of deeply nonnil type `%s`", f.TypeName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned into a map of deeply nonnil type `%s`", f.TypeName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // DeepAssignPrimitive is when a value flows to a point where it is assigned @@ -1113,15 +1343,22 @@ func (d *DeepAssignPrimitive) Copy() ConsumingAnnotationTrigger { } // Prestring returns this Prestring as a Prestring -func (*DeepAssignPrimitive) Prestring() Prestring { - return DeepAssignPrimitivePrestring{} +func (d *DeepAssignPrimitive) Prestring() Prestring { + return DeepAssignPrimitivePrestring{ + AssignmentStr: d.assignmentFlow.String(), + } } // DeepAssignPrimitivePrestring is a Prestring storing the needed information to compactly encode a DeepAssignPrimitive -type DeepAssignPrimitivePrestring struct{} +type DeepAssignPrimitivePrestring struct { + AssignmentStr string +} -func (DeepAssignPrimitivePrestring) String() string { - return "assigned into a deep type expecting nonnil element type" +func (d DeepAssignPrimitivePrestring) String() string { + var sb strings.Builder + sb.WriteString("assigned into a deep type expecting nonnil element type") + sb.WriteString(d.AssignmentStr) + return sb.String() } // ParamAssignDeep is when a value flows to a point where it is assigned deeply into a function parameter @@ -1146,16 +1383,23 @@ func (p *ParamAssignDeep) Copy() ConsumingAnnotationTrigger { // Prestring returns this ParamAssignDeep as a Prestring func (p *ParamAssignDeep) Prestring() Prestring { - return ParamAssignDeepPrestring{p.Ann.(*ParamAnnotationKey).MinimalString()} + return ParamAssignDeepPrestring{ + p.Ann.(*ParamAnnotationKey).MinimalString(), + p.assignmentFlow.String(), + } } // ParamAssignDeepPrestring is a Prestring storing the needed information to compactly encode a ParamAssignDeep type ParamAssignDeepPrestring struct { - ParamName string + ParamName string + AssignmentStr string } func (p ParamAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into parameter %s", p.ParamName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into parameter %s", p.ParamName)) + sb.WriteString(p.AssignmentStr) + return sb.String() } // FuncRetAssignDeep is when a value flows to a point where it is assigned deeply into a function return @@ -1184,17 +1428,22 @@ func (f *FuncRetAssignDeep) Prestring() Prestring { return FuncRetAssignDeepPrestring{ retAnn.FuncDecl.Name(), retAnn.RetNum, + f.assignmentFlow.String(), } } // FuncRetAssignDeepPrestring is a Prestring storing the needed information to compactly encode a FuncRetAssignDeep type FuncRetAssignDeepPrestring struct { - FuncName string - RetNum int + FuncName string + RetNum int + AssignmentStr string } func (f FuncRetAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into the result %d of `%s()`", f.RetNum, f.FuncName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into the result %d of `%s()`", f.RetNum, f.FuncName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // VariadicParamAssignDeep is when a value flows to a point where it is assigned deeply into a variadic @@ -1222,17 +1471,22 @@ func (v *VariadicParamAssignDeep) Copy() ConsumingAnnotationTrigger { func (v *VariadicParamAssignDeep) Prestring() Prestring { paramAnn := v.Ann.(*ParamAnnotationKey) return VariadicParamAssignDeepPrestring{ - ParamName: paramAnn.MinimalString(), + ParamName: paramAnn.MinimalString(), + AssignmentStr: v.assignmentFlow.String(), } } // VariadicParamAssignDeepPrestring is a Prestring storing the needed information to compactly encode a VariadicParamAssignDeep type VariadicParamAssignDeepPrestring struct { - ParamName string + ParamName string + AssignmentStr string } func (v VariadicParamAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into variadic parameter `%s`", v.ParamName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into variadic parameter `%s`", v.ParamName)) + sb.WriteString(v.AssignmentStr) + return sb.String() } // FieldAssignDeep is when a value flows to a point where it is assigned deeply into a field @@ -1258,16 +1512,23 @@ func (f *FieldAssignDeep) Copy() ConsumingAnnotationTrigger { // Prestring returns this FieldAssignDeep as a Prestring func (f *FieldAssignDeep) Prestring() Prestring { fldAnn := f.Ann.(*FieldAnnotationKey) - return FieldAssignDeepPrestring{fldAnn.FieldDecl.Name()} + return FieldAssignDeepPrestring{ + fldAnn.FieldDecl.Name(), + f.assignmentFlow.String(), + } } // FieldAssignDeepPrestring is a Prestring storing the needed information to compactly encode a FieldAssignDeep type FieldAssignDeepPrestring struct { - FldName string + FldName string + AssignmentStr string } func (f FieldAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into field `%s`", f.FldName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into field `%s`", f.FldName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // GlobalVarAssignDeep is when a value flows to a point where it is assigned deeply into a global variable @@ -1293,16 +1554,23 @@ func (g *GlobalVarAssignDeep) Copy() ConsumingAnnotationTrigger { // Prestring returns this GlobalVarAssignDeep as a Prestring func (g *GlobalVarAssignDeep) Prestring() Prestring { varAnn := g.Ann.(*GlobalVarAnnotationKey) - return GlobalVarAssignDeepPrestring{varAnn.VarDecl.Name()} + return GlobalVarAssignDeepPrestring{ + varAnn.VarDecl.Name(), + g.assignmentFlow.String(), + } } // GlobalVarAssignDeepPrestring is a Prestring storing the needed information to compactly encode a GlobalVarAssignDeep type GlobalVarAssignDeepPrestring struct { - VarName string + VarName string + AssignmentStr string } func (g GlobalVarAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into global variable `%s`", g.VarName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into global variable `%s`", g.VarName)) + sb.WriteString(g.AssignmentStr) + return sb.String() } // ChanAccess is when a channel is accessed for sending, and thus must be non-nil @@ -1327,14 +1595,21 @@ func (c *ChanAccess) Copy() ConsumingAnnotationTrigger { // Prestring returns this MapWrittenTo as a Prestring func (c *ChanAccess) Prestring() Prestring { - return ChanAccessPrestring{} + return ChanAccessPrestring{ + AssignmentStr: c.assignmentFlow.String(), + } } // ChanAccessPrestring is a Prestring storing the needed information to compactly encode a ChanAccess -type ChanAccessPrestring struct{} +type ChanAccessPrestring struct { + AssignmentStr string +} -func (ChanAccessPrestring) String() string { - return "uninitialized; nil channel accessed" +func (c ChanAccessPrestring) String() string { + var sb strings.Builder + sb.WriteString("uninitialized; nil channel accessed") + sb.WriteString(c.AssignmentStr) + return sb.String() } // LocalVarAssignDeep is when a value flows to a point where it is assigned deeply into a local variable of deeply nonnil type @@ -1360,16 +1635,23 @@ func (l *LocalVarAssignDeep) Copy() ConsumingAnnotationTrigger { // Prestring returns this LocalVarAssignDeep as a Prestring func (l *LocalVarAssignDeep) Prestring() Prestring { - return LocalVarAssignDeepPrestring{VarName: l.LocalVar.Name()} + return LocalVarAssignDeepPrestring{ + VarName: l.LocalVar.Name(), + AssignmentStr: l.assignmentFlow.String(), + } } // LocalVarAssignDeepPrestring is a Prestring storing the needed information to compactly encode a LocalVarAssignDeep type LocalVarAssignDeepPrestring struct { - VarName string + VarName string + AssignmentStr string } func (l LocalVarAssignDeepPrestring) String() string { - return fmt.Sprintf("assigned deeply into local variable `%s`", l.VarName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("assigned deeply into local variable `%s`", l.VarName)) + sb.WriteString(l.AssignmentStr) + return sb.String() } // ChanSend is when a value flows to a point where it is sent to a channel @@ -1395,16 +1677,23 @@ func (c *ChanSend) Copy() ConsumingAnnotationTrigger { // Prestring returns this ChanSend as a Prestring func (c *ChanSend) Prestring() Prestring { typeAnn := c.Ann.(*TypeNameAnnotationKey) - return ChanSendPrestring{typeAnn.TypeDecl.Name()} + return ChanSendPrestring{ + typeAnn.TypeDecl.Name(), + c.assignmentFlow.String(), + } } // ChanSendPrestring is a Prestring storing the needed information to compactly encode a ChanSend type ChanSendPrestring struct { - TypeName string + TypeName string + AssignmentStr string } func (c ChanSendPrestring) String() string { - return fmt.Sprintf("sent to channel of deeply nonnil type `%s`", c.TypeName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("sent to channel of deeply nonnil type `%s`", c.TypeName)) + sb.WriteString(c.AssignmentStr) + return sb.String() } // FldEscape is when a nilable value flows through a field of a struct that escapes. @@ -1438,17 +1727,22 @@ func (f *FldEscape) Copy() ConsumingAnnotationTrigger { func (f *FldEscape) Prestring() Prestring { ann := f.Ann.(*EscapeFieldAnnotationKey) return FldEscapePrestring{ - FieldName: ann.FieldDecl.Name(), + FieldName: ann.FieldDecl.Name(), + AssignmentStr: f.assignmentFlow.String(), } } // FldEscapePrestring is a Prestring storing the needed information to compactly encode a FldEscape type FldEscapePrestring struct { - FieldName string + FieldName string + AssignmentStr string } func (f FldEscapePrestring) String() string { - return fmt.Sprintf("field `%s` escaped out of our analysis scope (presumed nilable)", f.FieldName) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("field `%s` escaped out of our analysis scope (presumed nilable)", f.FieldName)) + sb.WriteString(f.AssignmentStr) + return sb.String() } // UseAsNonErrorRetDependentOnErrorRetNilability is when a value flows to a point where it is returned from an error returning function @@ -1485,6 +1779,7 @@ func (u *UseAsNonErrorRetDependentOnErrorRetNilability) Prestring() Prestring { retAnn.FuncDecl.Type().(*types.Signature).Results().At(retAnn.RetNum).Name(), retAnn.FuncDecl.Type().(*types.Signature).Results().Len() - 1, u.IsNamedReturn, + u.assignmentFlow.String(), } } @@ -1495,6 +1790,7 @@ type UseAsNonErrorRetDependentOnErrorRetNilabilityPrestring struct { RetName string ErrRetNum int IsNamedReturn bool + AssignmentStr string } func (u UseAsNonErrorRetDependentOnErrorRetNilabilityPrestring) String() string { @@ -1503,8 +1799,11 @@ func (u UseAsNonErrorRetDependentOnErrorRetNilabilityPrestring) String() string via = fmt.Sprintf(" via named return `%s`", u.RetName) } - return fmt.Sprintf("returned from `%s()`%s in position %d when the error return in position %d is not guaranteed to be non-nil through all paths", - u.FuncName, via, u.RetNum, u.ErrRetNum) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("returned from `%s()`%s in position %d when the error return in position %d is not guaranteed to be non-nil through all paths", + u.FuncName, via, u.RetNum, u.ErrRetNum)) + sb.WriteString(u.AssignmentStr) + return sb.String() } // overriding position value to point to the raw return statement, which is the source of the potential error @@ -1548,6 +1847,7 @@ func (u *UseAsErrorRetWithNilabilityUnknown) Prestring() Prestring { retAnn.RetNum, u.IsNamedReturn, retAnn.FuncDecl.Type().(*types.Signature).Results().At(retAnn.RetNum).Name(), + u.assignmentFlow.String(), } } @@ -1557,13 +1857,18 @@ type UseAsErrorRetWithNilabilityUnknownPrestring struct { RetNum int IsNamedReturn bool RetName string + AssignmentStr string } func (u UseAsErrorRetWithNilabilityUnknownPrestring) String() string { + var sb strings.Builder if u.IsNamedReturn { - return fmt.Sprintf("found in at least one path of `%s()` for named return `%s` in position %d", u.FuncName, u.RetName, u.RetNum) + sb.WriteString(fmt.Sprintf("found in at least one path of `%s()` for named return `%s` in position %d", u.FuncName, u.RetName, u.RetNum)) + } else { + sb.WriteString(fmt.Sprintf("found in at least one path of `%s()` for return in position %d", u.FuncName, u.RetNum)) } - return fmt.Sprintf("found in at least one path of `%s()` for return in position %d", u.FuncName, u.RetNum) + sb.WriteString(u.AssignmentStr) + return sb.String() } // overriding position value to point to the raw return statement, which is the source of the potential error diff --git a/assertion/function/assertiontree/backprop.go b/assertion/function/assertiontree/backprop.go index c30bb80f..9383d866 100644 --- a/assertion/function/assertiontree/backprop.go +++ b/assertion/function/assertiontree/backprop.go @@ -27,6 +27,7 @@ import ( "go.uber.org/nilaway/annotation" "go.uber.org/nilaway/config" "go.uber.org/nilaway/util" + "go.uber.org/nilaway/util/asthelper" "golang.org/x/exp/slices" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/cfg" @@ -585,7 +586,27 @@ buildShadowMask: } lhsNode, ok := rootNode.LiftFromPath(lpath) - if ok { + // TODO: below check for `lhsNode != nil` should not be needed when NilAway supports Ok form for + // used-defined functions (tracked issue #77) + if ok && lhsNode != nil { + // Add assignment entries to the consumers of lhsNode for informative printing of errors + for _, c := range lhsNode.ConsumeTriggers() { + var lhsExprStr, rhsExprStr string + var err error + if lhsExprStr, err = asthelper.PrintExpr(lhsVal, rootNode.Pass(), true /* isShortenExpr */); err != nil { + return err + } + if rhsExprStr, err = asthelper.PrintExpr(rhsVal, rootNode.Pass(), true /* isShortenExpr */); err != nil { + return err + } + + c.Annotation.AddAssignment(annotation.Assignment{ + LHSExprStr: lhsExprStr, + RHSExprStr: rhsExprStr, + Position: util.TruncatePosition(util.PosToLocation(lhsVal.Pos(), rootNode.Pass())), + }) + } + // If the lhsVal path is not only trackable but tracked, we add it as // a deferred landing landings = append(landings, deferredLanding{ @@ -609,10 +630,36 @@ buildShadowMask: rootNode.addProductionsForAssignmentFields(fieldProducers, lhsVal) } + // beforeTriggersLastIndex is used to find the newly added triggers on the next line + beforeTriggersLastIndex := len(rootNode.triggers) + rootNode.AddProduction(&annotation.ProduceTrigger{ Annotation: rproducers[0].GetShallow().Annotation, Expr: lhsVal, }, rproducers[0].GetDeepSlice()...) + + // Update consumers of newly added triggers with assignment entries for informative printing of errors + // TODO: the below check `len(rootNode.triggers) == 0` should not be needed, however, it is added to + // satisfy NilAway's analysis + if len(rootNode.triggers) == 0 { + continue + } + for _, t := range rootNode.triggers[beforeTriggersLastIndex:len(rootNode.triggers)] { + var lhsExprStr, rhsExprStr string + var err error + if lhsExprStr, err = asthelper.PrintExpr(lhsVal, rootNode.Pass(), true /* isShortenExpr */); err != nil { + return err + } + if rhsExprStr, err = asthelper.PrintExpr(rhsVal, rootNode.Pass(), true /* isShortenExpr */); err != nil { + return err + } + + t.Consumer.Annotation.AddAssignment(annotation.Assignment{ + LHSExprStr: lhsExprStr, + RHSExprStr: rhsExprStr, + Position: util.TruncatePosition(util.PosToLocation(lhsVal.Pos(), rootNode.Pass())), + }) + } default: return errors.New("rhs expression in a 1-1 assignment was multiply returning - " + "this certainly indicates an error in control flow") diff --git a/assertion/function/assertiontree/root_assertion_node.go b/assertion/function/assertiontree/root_assertion_node.go index 6f8cc4b2..a3fdcde2 100644 --- a/assertion/function/assertiontree/root_assertion_node.go +++ b/assertion/function/assertiontree/root_assertion_node.go @@ -896,8 +896,6 @@ func getFuncLitFromAssignment(ident *ast.Ident) *ast.FuncLit { // } // // ``` -// -// nilable(path, result 0) func (r *RootAssertionNode) LiftFromPath(path TrackableExpr) (AssertionNode, bool) { if path != nil { node, whichChild := r.lookupPath(path) diff --git a/nilaway_test.go b/nilaway_test.go index 1e253d33..3dce8adb 100644 --- a/nilaway_test.go +++ b/nilaway_test.go @@ -232,6 +232,13 @@ func TestConstants(t *testing.T) { analysistest.Run(t, testdata, Analyzer, "go.uber.org/consts") } +func TestErrorMessage(t *testing.T) { + t.Parallel() + + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "go.uber.org/errormessage") +} + func TestPrettyPrint(t *testing.T) { //nolint:paralleltest // We specifically do not set this test to be parallel such that this test is run separately // from the parallel tests. This makes it possible to set the pretty-print flag to true for diff --git a/testdata/src/go.uber.org/errormessage/errormessage.go b/testdata/src/go.uber.org/errormessage/errormessage.go new file mode 100644 index 00000000..d424d7f0 --- /dev/null +++ b/testdata/src/go.uber.org/errormessage/errormessage.go @@ -0,0 +1,264 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This package tests _single_ package inference. Due to limitations of `analysistest` framework, +// multi-package inference is tested by our integration test suites. Please see +// `testdata/README.md` for more details. + +// +package errormessage + +import "errors" + +var dummy bool + +func test1(x *int) { + x = nil + print(*x) //want "`nil` to `x`" +} + +func test2(x *int) { + x = nil + y := x + z := y + print(*z) //want "`y` to `z`" +} + +func test3(x *int) { + if dummy { + x = nil + } else { + x = new(int) + } + y := x + z := y + print(*z) //want "`nil` to `x`" +} + +// nilable(f) +type S struct { + f *int +} + +func test4(x *int) { + s := &S{} + x = nil + y := x + z := y + s.f = z + print(*s.f) //want "`z` to `s.f`" +} + +func test5() { + x := new(int) + for i := 0; i < 10; i++ { + print(*x) //want "`nil` to `y`" + var y *int = nil + z := y + x = z + } +} + +func test6() *int { + var x *int = nil + y := x + z := y + return z //want "`nil` to `x`" +} + +func test7() { + var x *int + if dummy { + y := new(int) + x = y + } + print(*x) //want "unassigned variable `x` dereferenced" +} + +func test8() { + x := new(int) + if dummy { + var y *int + x = y + } + print(*x) //want "`y` to `x`" +} + +func test9(m map[int]*int) { + x, _ := m[0] + y := x + print(*y) //want "`m\\[0\\]` to `x`" +} + +func test10(ch chan *int) { + x := <-ch //want "nil channel accessed" + y := x + print(*y) +} + +func callTest10() { + var ch chan *int + test10(ch) +} + +func test11(s []*int) { + x := s[0] //want "`s` sliced into" + y := x + print(*y) +} + +func callTest11() { + var s []*int + test11(s) +} + +func test12(mp map[int]S, i int) { + x := mp[i] // unrelated assignment, should not be printed in the error message + _ = x + + y := mp[i] // unrelated assignment, should not be printed in the error message + _ = y + + s := mp[i] // relevant assignment, should be printed in the error message + consumeS(&s) //want "`mp\\[i\\]` to `s`" +} + +func consumeS(s *S) { + print(s.f) +} + +func retErr() error { + return errors.New("error") +} + +func test13() *int { + if err := retErr(); err != nil { // unrelated assignment, should not be printed in the error message + return nil //want "literal `nil` returned" + } + return new(int) +} + +// below tests check shortening of expressions in assignment messages + +// nilable(s, result 0) +func (s *S) bar(i int) *int { + return nil +} + +// nilable(result 0) +func (s *S) foo(a int, b *int, c string, d bool) *S { + return nil +} + +func test14(x *int, i int) { + s := &S{} + x = s.foo(1, + new(int), + "abc", + true).bar(i) + y := x + print(*y) //want "`s.foo\\(...\\).bar\\(i\\)` to `x`" +} + +func test15(x *int) { + var longVarName, anotherLongVarName, yetAnotherLongName int + s := &S{} + x = s.foo(longVarName, &anotherLongVarName, "abc", true).bar(yetAnotherLongName) + y := x + print(*y) //want "`s.foo\\(...\\).bar\\(...\\)` to `x`" +} + +func test16(mp map[int]*int) { + var aVeryVeryVeryLongIndexVar int + x := mp[aVeryVeryVeryLongIndexVar] + y := x + print(*y) //want "`mp\\[...\\]` to `x`" +} + +func test17(x *int, mp map[int]*int) { + var aVeryVeryVeryLongIndexVar int + s := &S{} + + x = s.foo(1, mp[aVeryVeryVeryLongIndexVar], "abc", true).bar(2) //want "deep read" + y := x + print(*y) //want "`s.foo\\(...\\).bar\\(2\\)` to `x`" +} + +func test18(x *int, mp map[int]*int) { + s := &S{} + x = mp[*(s.foo(1, new(int), "abc", true).bar(2))] //want "dereferenced" + y := x + print(*y) //want "`mp\\[...\\]` to `x`" +} + +func test19() { + mp := make(map[string]*string) + x := mp["("] + y := x + print(*y) //want "`mp\\[\"\\(\"\\]` to `x`" + + x = mp[")"] + y = x + print(*y) //want "`mp\\[\"\\)\"\\]` to `x`" + + x = mp["))"] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + x = mp["(("] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + x = mp[")))((("] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + x = mp[")))((("] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + x = mp["(((()"] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + x = mp["())))"] + y = x + print(*y) //want "`mp\\[...\\]` to `x`" + + s := &S{} + i := 0 + a := s.foo(1, + new(int), + "({[", + true).bar(i) + b := a + print(*b) //want "`s.foo\\(...\\).bar\\(i\\)` to `a`" +} + +func test20() { + mp := make(map[rune]*rune) + x := mp['('] + y := x + print(*y) //want "`mp\\['\\('\\]` to `x`" + + x = mp[')'] + y = x + print(*y) //want "`mp\\['\\)'\\]` to `x`" +} + +// below test checks that NilAway can handle non-English (non-ASCII) identifiers +func test21() { + var 世界 *int = nil + print(*世界) //want "`nil` to `世界`" +} diff --git a/util/asthelper/asthelper.go b/util/asthelper/asthelper.go new file mode 100644 index 00000000..7d757e57 --- /dev/null +++ b/util/asthelper/asthelper.go @@ -0,0 +1,104 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package asthelper implements utility functions for AST. +package asthelper + +import ( + "go/ast" + "go/printer" + "go/token" + "io" + "strings" + + "golang.org/x/tools/go/analysis" +) + +// PrintExpr converts AST expression to string, and shortens long expressions if isShortenExpr is true +func PrintExpr(e ast.Expr, pass *analysis.Pass, isShortenExpr bool) (string, error) { + builder := &strings.Builder{} + var err error + + if !isShortenExpr { + err = printer.Fprint(builder, pass.Fset, e) + } else { + // traverse over the AST expression's subtree and shorten long expressions + // (e.g., s.foo(longVarName, anotherLongVarName, someOtherLongVarName) --> s.foo(...)) + err = printExpr(builder, pass.Fset, e) + } + + return builder.String(), err +} + +func printExpr(writer io.Writer, fset *token.FileSet, e ast.Expr) (err error) { + // _shortenExprLen is the maximum length of an expression to be printed in full. The value is set to 3 to account for + // the length of the ellipsis ("..."), which is used to shorten long expressions. + const _shortenExprLen = 3 + + // fullExpr returns true if the expression is short enough (<= _shortenExprLen) to be printed in full + fullExpr := func(node ast.Node) (string, bool) { + switch n := node.(type) { + case *ast.Ident: + if len(n.Name) <= _shortenExprLen { + return n.Name, true + } + case *ast.BasicLit: + if len(n.Value) <= _shortenExprLen { + return n.Value, true + } + } + return "", false + } + + switch node := e.(type) { + case *ast.Ident: + _, err = io.WriteString(writer, node.Name) + + case *ast.SelectorExpr: + if err = printExpr(writer, fset, node.X); err != nil { + return + } + _, err = io.WriteString(writer, "."+node.Sel.Name) + + case *ast.CallExpr: + if err = printExpr(writer, fset, node.Fun); err != nil { + return + } + var argStr string + if len(node.Args) > 0 { + argStr = "..." + if len(node.Args) == 1 { + if a, ok := fullExpr(node.Args[0]); ok { + argStr = a + } + } + } + _, err = io.WriteString(writer, "("+argStr+")") + + case *ast.IndexExpr: + if err = printExpr(writer, fset, node.X); err != nil { + return + } + + indexExpr := "..." + if v, ok := fullExpr(node.Index); ok { + indexExpr = v + } + _, err = io.WriteString(writer, "["+indexExpr+"]") + + default: + err = printer.Fprint(writer, fset, e) + } + return +}