diff --git a/internal/analysis/nonmutating/nonmutating.go b/internal/analysis/nonmutating/nonmutating.go index 0f15fb0..0c9fd78 100644 --- a/internal/analysis/nonmutating/nonmutating.go +++ b/internal/analysis/nonmutating/nonmutating.go @@ -1,7 +1,6 @@ package nonmutating import ( - "fmt" "go/ast" "go/types" @@ -166,21 +165,6 @@ func (state *functionState) collectAssignAliases(stmt *ast.AssignStmt) bool { return changed } -func (state *functionState) containerStorageVisible(expr ast.Expr) (origin, bool) { - expr = unparen(expr) - switch expr := expr.(type) { - case *ast.Ident: - origin, ok := state.originOf(expr) - if !ok { - return origin, false - } - - return origin, isReferenceLike(state.typeOf(expr)) - default: - panic("unexpected expr type: " + fmt.Sprintf("%T", expr)) - } -} - func (state *functionState) exprRefersToCallerVisibleMemory(expr ast.Expr) (origin, bool) { expr = unparen(expr) switch expr := expr.(type) { @@ -192,18 +176,18 @@ func (state *functionState) exprRefersToCallerVisibleMemory(expr ast.Expr) (orig return origin, isReferenceLike(state.typeOf(expr)) case *ast.SelectorExpr: - origin, ok := state.originOf(expr.X) - if !ok { - return origin, false + if origin, ok := state.exprRefersToCallerVisibleMemory(expr.X); ok { + return origin, true } - if _, ok := state.containerStorageVisible(expr.X); ok { - return origin, true + origin, ok := state.rootOrigin(expr.X) + if !ok { + return origin, false } return origin, isReferenceLike(state.typeOf(expr)) case *ast.IndexExpr: - origin, ok := state.originOf(expr.X) + origin, ok := state.rootOrigin(expr.X) if !ok { return origin, false } @@ -296,6 +280,26 @@ func (state *functionState) reportMutations(body *ast.BlockStmt) { }) } +// rootOrigin returns the parameter or receiver that an expression starts at. +// The analyzer holds an origin for each identifier, so a chain that starts +// below an identifier, such as an element of a slice parameter, reports the +// identifier that the chain starts at. +func (state *functionState) rootOrigin(expr ast.Expr) (origin, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + return state.originOf(expr) + case *ast.StarExpr: + return state.rootOrigin(expr.X) + case *ast.SelectorExpr: + return state.rootOrigin(expr.X) + case *ast.IndexExpr: + return state.rootOrigin(expr.X) + } + + return origin{}, false +} + func (state *functionState) typeOf(expr ast.Expr) types.Type { return state.pass.TypesInfo.TypeOf(expr) } @@ -308,7 +312,7 @@ func (state *functionState) visibleMutationOrigin(expr ast.Expr) (origin, bool) case *ast.StarExpr: return state.exprRefersToCallerVisibleMemory(expr.X) case *ast.SelectorExpr: - return state.containerStorageVisible(expr.X) + return state.exprRefersToCallerVisibleMemory(expr.X) case *ast.IndexExpr: return state.exprRefersToCallerVisibleMemory(expr.X) } diff --git a/internal/analysis/nonmutating/testdata/src/a/a.go b/internal/analysis/nonmutating/testdata/src/a/a.go index 53c1ed6..aa500f4 100644 --- a/internal/analysis/nonmutating/testdata/src/a/a.go +++ b/internal/analysis/nonmutating/testdata/src/a/a.go @@ -122,3 +122,34 @@ func DeletesLocalMap() { func MutatesDoublePointer(pp **int) { **pp = 1 // want "//constable:nonmutating function mutates pointer parameter pp" } + +type Element struct{ n int } + +//constable:nonmutating +func MutatesFieldOfSliceElement(elements []Element) { + elements[0].n = 1 // want "//constable:nonmutating function mutates parameter elements" +} + +//constable:nonmutating +func MutatesNestedSliceElementField(ss [][]Element) { + ss[0][0].n = 1 // want "//constable:nonmutating function mutates parameter ss" +} + +type Inner struct{ data []byte } + +type Outer struct{ inner Inner } + +//constable:nonmutating +func MutatesFieldThroughDeref(p *Outer) { + (*p).inner.data[0] = 0 // want "//constable:nonmutating function mutates pointer parameter p" +} + +//constable:nonmutating +func (o Outer) ClearNestedField() { + o.inner.data[0] = 0 // want "//constable:nonmutating method mutates receiver o" +} + +//constable:nonmutating +func (o Outer) ReplacesNestedField() { + o.inner = Inner{} +}