Relates #367: update unparam

$ git cherry -v cc9d2fb52971 fbb59629db34
+ 7362051ae01a0e35956c077c3be5505c70edd200 testdata: add more regression
tests
+ a88ca0234e2c3732a53cd49514fb3877a5d9f1f5 properly record which methods
implement interfaces
+ b762b0b27fa23ebbdfc31df1af4097cfd89a17f6 work properly with method
wrapper functions
+ f59bb08c5c7429b88e0c6e2399b12e71e8d950db testdata: consistently use
tabs
+ 46a5101c55d03117b263b4c5161e5d01353311c1 replace more callgraph code
with plain SSA
+ 1679b9996abdc6431c2147a133e5223ebb86ea60 rewrite foo(bar()) code to
work without callgraph
+ 71b5df77c291f8c5ead5f9ff0a69e0eebf3ae5b2 rewrite "called in return"
code without callgraph
+ fc5b1c74f563d4f2f9ee7d91eda648fc7baebf67 check: replace last use of
callgraph
+ 229ad68a599e2622cf9d3bb87a385b158efe736f fix a minor false negative in
callExtract
+ fbb59629db34a0f69275bc336cc8c3a4dd9fbe5d fix up another false negative
in testdata

go.mod

@@ -61,7 +61,7 @@ require (
 	gopkg.in/yaml.v2 v2.2.1
 	mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed
 	mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b // indirect
-	mvdan.cc/unparam v0.0.0-20190124213536-cc9d2fb52971
+	mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34
 	sourcegraph.com/sourcegraph/go-diff v0.0.0-20171119081133-3f415a150aec
 	sourcegraph.com/sqs/pbtypes v0.0.0-20160107090929-4d1b9dc7ffc3 // indirect
 )

go.sum

@@ -173,8 +173,8 @@ mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed h1:WX1yoOaKQfddO/mLzdV4wp
 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed/go.mod h1:Xkxe497xwlCKkIaQYRfC7CSLworTXY9RMqwhhCm+8Nc=
 mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b h1:DxJ5nJdkhDlLok9K6qO+5290kphDJbHOQO1DFFFTeBo=
 mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b/go.mod h1:2odslEg/xrtNQqCYg2/jCoyKnw3vv5biOc3JnIcYfL4=
-mvdan.cc/unparam v0.0.0-20190124213536-cc9d2fb52971 h1:INXoeng6RAblgAds8fdt2cAu1Bs/bgr5zq1fdYcuxMI=
-mvdan.cc/unparam v0.0.0-20190124213536-cc9d2fb52971/go.mod h1:H6SUd1XjIs+qQCyskXg5OFSrilMRUkD8ePJpHKDPaeY=
+mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34 h1:B1LAOfRqg2QUyCdzfjf46quTSYUTAK5OCwbh6pljHbM=
+mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34/go.mod h1:H6SUd1XjIs+qQCyskXg5OFSrilMRUkD8ePJpHKDPaeY=
 sourcegraph.com/sourcegraph/go-diff v0.0.0-20171119081133-3f415a150aec h1:wAAdENPXC7bE1oxY4VqSDdhaA+XQ8TgQHsZMMnrXjEk=
 sourcegraph.com/sourcegraph/go-diff v0.0.0-20171119081133-3f415a150aec/go.mod h1:R09mWeb9JcPbO+A3cYDc11xjz0wp6r9+KnqdqROAoRU=
 sourcegraph.com/sqs/pbtypes v0.0.0-20160107090929-4d1b9dc7ffc3 h1:hXy8YsgVLDz5mlngKhNHQhAsAGrSp3dlXZN4b0/4UUI=

vendor/golang.org/x/tools/go/callgraph/callgraph.go

@@ -1,129 +0,0 @@
-// Copyright 2013 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-/*
-
-Package callgraph defines the call graph and various algorithms
-and utilities to operate on it.
-
-A call graph is a labelled directed graph whose nodes represent
-functions and whose edge labels represent syntactic function call
-sites.  The presence of a labelled edge (caller, site, callee)
-indicates that caller may call callee at the specified call site.
-
-A call graph is a multigraph: it may contain multiple edges (caller,
-*, callee) connecting the same pair of nodes, so long as the edges
-differ by label; this occurs when one function calls another function
-from multiple call sites.  Also, it may contain multiple edges
-(caller, site, *) that differ only by callee; this indicates a
-polymorphic call.
-
-A SOUND call graph is one that overapproximates the dynamic calling
-behaviors of the program in all possible executions.  One call graph
-is more PRECISE than another if it is a smaller overapproximation of
-the dynamic behavior.
-
-All call graphs have a synthetic root node which is responsible for
-calling main() and init().
-
-Calls to built-in functions (e.g. panic, println) are not represented
-in the call graph; they are treated like built-in operators of the
-language.
-
-*/
-package callgraph // import "golang.org/x/tools/go/callgraph"
-
-// TODO(adonovan): add a function to eliminate wrappers from the
-// callgraph, preserving topology.
-// More generally, we could eliminate "uninteresting" nodes such as
-// nodes from packages we don't care about.
-
-import (
-	"fmt"
-	"go/token"
-
-	"golang.org/x/tools/go/ssa"
-)
-
-// A Graph represents a call graph.
-//
-// A graph may contain nodes that are not reachable from the root.
-// If the call graph is sound, such nodes indicate unreachable
-// functions.
-//
-type Graph struct {
-	Root  *Node                   // the distinguished root node
-	Nodes map[*ssa.Function]*Node // all nodes by function
-}
-
-// New returns a new Graph with the specified root node.
-func New(root *ssa.Function) *Graph {
-	g := &Graph{Nodes: make(map[*ssa.Function]*Node)}
-	g.Root = g.CreateNode(root)
-	return g
-}
-
-// CreateNode returns the Node for fn, creating it if not present.
-func (g *Graph) CreateNode(fn *ssa.Function) *Node {
-	n, ok := g.Nodes[fn]
-	if !ok {
-		n = &Node{Func: fn, ID: len(g.Nodes)}
-		g.Nodes[fn] = n
-	}
-	return n
-}
-
-// A Node represents a node in a call graph.
-type Node struct {
-	Func *ssa.Function // the function this node represents
-	ID   int           // 0-based sequence number
-	In   []*Edge       // unordered set of incoming call edges (n.In[*].Callee == n)
-	Out  []*Edge       // unordered set of outgoing call edges (n.Out[*].Caller == n)
-}
-
-func (n *Node) String() string {
-	return fmt.Sprintf("n%d:%s", n.ID, n.Func)
-}
-
-// A Edge represents an edge in the call graph.
-//
-// Site is nil for edges originating in synthetic or intrinsic
-// functions, e.g. reflect.Call or the root of the call graph.
-type Edge struct {
-	Caller *Node
-	Site   ssa.CallInstruction
-	Callee *Node
-}
-
-func (e Edge) String() string {
-	return fmt.Sprintf("%s --> %s", e.Caller, e.Callee)
-}
-
-func (e Edge) Description() string {
-	var prefix string
-	switch e.Site.(type) {
-	case nil:
-		return "synthetic call"
-	case *ssa.Go:
-		prefix = "concurrent "
-	case *ssa.Defer:
-		prefix = "deferred "
-	}
-	return prefix + e.Site.Common().Description()
-}
-
-func (e Edge) Pos() token.Pos {
-	if e.Site == nil {
-		return token.NoPos
-	}
-	return e.Site.Pos()
-}
-
-// AddEdge adds the edge (caller, site, callee) to the call graph.
-// Elimination of duplicate edges is the caller's responsibility.
-func AddEdge(caller *Node, site ssa.CallInstruction, callee *Node) {
-	e := &Edge{caller, site, callee}
-	callee.In = append(callee.In, e)
-	caller.Out = append(caller.Out, e)
-}

vendor/golang.org/x/tools/go/callgraph/cha/cha.go

@@ -1,139 +0,0 @@
-// Copyright 2014 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package cha computes the call graph of a Go program using the Class
-// Hierarchy Analysis (CHA) algorithm.
-//
-// CHA was first described in "Optimization of Object-Oriented Programs
-// Using Static Class Hierarchy Analysis", Jeffrey Dean, David Grove,
-// and Craig Chambers, ECOOP'95.
-//
-// CHA is related to RTA (see go/callgraph/rta); the difference is that
-// CHA conservatively computes the entire "implements" relation between
-// interfaces and concrete types ahead of time, whereas RTA uses dynamic
-// programming to construct it on the fly as it encounters new functions
-// reachable from main.  CHA may thus include spurious call edges for
-// types that haven't been instantiated yet, or types that are never
-// instantiated.
-//
-// Since CHA conservatively assumes that all functions are address-taken
-// and all concrete types are put into interfaces, it is sound to run on
-// partial programs, such as libraries without a main or test function.
-//
-package cha // import "golang.org/x/tools/go/callgraph/cha"
-
-import (
-	"go/types"
-
-	"golang.org/x/tools/go/callgraph"
-	"golang.org/x/tools/go/ssa"
-	"golang.org/x/tools/go/ssa/ssautil"
-	"golang.org/x/tools/go/types/typeutil"
-)
-
-// CallGraph computes the call graph of the specified program using the
-// Class Hierarchy Analysis algorithm.
-//
-func CallGraph(prog *ssa.Program) *callgraph.Graph {
-	cg := callgraph.New(nil) // TODO(adonovan) eliminate concept of rooted callgraph
-
-	allFuncs := ssautil.AllFunctions(prog)
-
-	// funcsBySig contains all functions, keyed by signature.  It is
-	// the effective set of address-taken functions used to resolve
-	// a dynamic call of a particular signature.
-	var funcsBySig typeutil.Map // value is []*ssa.Function
-
-	// methodsByName contains all methods,
-	// grouped by name for efficient lookup.
-	// (methodsById would be better but not every SSA method has a go/types ID.)
-	methodsByName := make(map[string][]*ssa.Function)
-
-	// An imethod represents an interface method I.m.
-	// (There's no go/types object for it;
-	// a *types.Func may be shared by many interfaces due to interface embedding.)
-	type imethod struct {
-		I  *types.Interface
-		id string
-	}
-	// methodsMemo records, for every abstract method call I.m on
-	// interface type I, the set of concrete methods C.m of all
-	// types C that satisfy interface I.
-	//
-	// Abstract methods may be shared by several interfaces,
-	// hence we must pass I explicitly, not guess from m.
-	//
-	// methodsMemo is just a cache, so it needn't be a typeutil.Map.
-	methodsMemo := make(map[imethod][]*ssa.Function)
-	lookupMethods := func(I *types.Interface, m *types.Func) []*ssa.Function {
-		id := m.Id()
-		methods, ok := methodsMemo[imethod{I, id}]
-		if !ok {
-			for _, f := range methodsByName[m.Name()] {
-				C := f.Signature.Recv().Type() // named or *named
-				if types.Implements(C, I) {
-					methods = append(methods, f)
-				}
-			}
-			methodsMemo[imethod{I, id}] = methods
-		}
-		return methods
-	}
-
-	for f := range allFuncs {
-		if f.Signature.Recv() == nil {
-			// Package initializers can never be address-taken.
-			if f.Name() == "init" && f.Synthetic == "package initializer" {
-				continue
-			}
-			funcs, _ := funcsBySig.At(f.Signature).([]*ssa.Function)
-			funcs = append(funcs, f)
-			funcsBySig.Set(f.Signature, funcs)
-		} else {
-			methodsByName[f.Name()] = append(methodsByName[f.Name()], f)
-		}
-	}
-
-	addEdge := func(fnode *callgraph.Node, site ssa.CallInstruction, g *ssa.Function) {
-		gnode := cg.CreateNode(g)
-		callgraph.AddEdge(fnode, site, gnode)
-	}
-
-	addEdges := func(fnode *callgraph.Node, site ssa.CallInstruction, callees []*ssa.Function) {
-		// Because every call to a highly polymorphic and
-		// frequently used abstract method such as
-		// (io.Writer).Write is assumed to call every concrete
-		// Write method in the program, the call graph can
-		// contain a lot of duplication.
-		//
-		// TODO(adonovan): opt: consider factoring the callgraph
-		// API so that the Callers component of each edge is a
-		// slice of nodes, not a singleton.
-		for _, g := range callees {
-			addEdge(fnode, site, g)
-		}
-	}
-
-	for f := range allFuncs {
-		fnode := cg.CreateNode(f)
-		for _, b := range f.Blocks {
-			for _, instr := range b.Instrs {
-				if site, ok := instr.(ssa.CallInstruction); ok {
-					call := site.Common()
-					if call.IsInvoke() {
-						tiface := call.Value.Type().Underlying().(*types.Interface)
-						addEdges(fnode, site, lookupMethods(tiface, call.Method))
-					} else if g := call.StaticCallee(); g != nil {
-						addEdge(fnode, site, g)
-					} else if _, ok := call.Value.(*ssa.Builtin); !ok {
-						callees, _ := funcsBySig.At(call.Signature()).([]*ssa.Function)
-						addEdges(fnode, site, callees)
-					}
-				}
-			}
-		}
-	}
-
-	return cg
-}

vendor/golang.org/x/tools/go/callgraph/util.go

@@ -1,181 +0,0 @@
-// Copyright 2013 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package callgraph
-
-import "golang.org/x/tools/go/ssa"
-
-// This file provides various utilities over call graphs, such as
-// visitation and path search.
-
-// CalleesOf returns a new set containing all direct callees of the
-// caller node.
-//
-func CalleesOf(caller *Node) map[*Node]bool {
-	callees := make(map[*Node]bool)
-	for _, e := range caller.Out {
-		callees[e.Callee] = true
-	}
-	return callees
-}
-
-// GraphVisitEdges visits all the edges in graph g in depth-first order.
-// The edge function is called for each edge in postorder.  If it
-// returns non-nil, visitation stops and GraphVisitEdges returns that
-// value.
-//
-func GraphVisitEdges(g *Graph, edge func(*Edge) error) error {
-	seen := make(map[*Node]bool)
-	var visit func(n *Node) error
-	visit = func(n *Node) error {
-		if !seen[n] {
-			seen[n] = true
-			for _, e := range n.Out {
-				if err := visit(e.Callee); err != nil {
-					return err
-				}
-				if err := edge(e); err != nil {
-					return err
-				}
-			}
-		}
-		return nil
-	}
-	for _, n := range g.Nodes {
-		if err := visit(n); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// PathSearch finds an arbitrary path starting at node start and
-// ending at some node for which isEnd() returns true.  On success,
-// PathSearch returns the path as an ordered list of edges; on
-// failure, it returns nil.
-//
-func PathSearch(start *Node, isEnd func(*Node) bool) []*Edge {
-	stack := make([]*Edge, 0, 32)
-	seen := make(map[*Node]bool)
-	var search func(n *Node) []*Edge
-	search = func(n *Node) []*Edge {
-		if !seen[n] {
-			seen[n] = true
-			if isEnd(n) {
-				return stack
-			}
-			for _, e := range n.Out {
-				stack = append(stack, e) // push
-				if found := search(e.Callee); found != nil {
-					return found
-				}
-				stack = stack[:len(stack)-1] // pop
-			}
-		}
-		return nil
-	}
-	return search(start)
-}
-
-// DeleteSyntheticNodes removes from call graph g all nodes for
-// synthetic functions (except g.Root and package initializers),
-// preserving the topology.  In effect, calls to synthetic wrappers
-// are "inlined".
-//
-func (g *Graph) DeleteSyntheticNodes() {
-	// Measurements on the standard library and go.tools show that
-	// resulting graph has ~15% fewer nodes and 4-8% fewer edges
-	// than the input.
-	//
-	// Inlining a wrapper of in-degree m, out-degree n adds m*n
-	// and removes m+n edges.  Since most wrappers are monomorphic
-	// (n=1) this results in a slight reduction.  Polymorphic
-	// wrappers (n>1), e.g. from embedding an interface value
-	// inside a struct to satisfy some interface, cause an
-	// increase in the graph, but they seem to be uncommon.
-
-	// Hash all existing edges to avoid creating duplicates.
-	edges := make(map[Edge]bool)
-	for _, cgn := range g.Nodes {
-		for _, e := range cgn.Out {
-			edges[*e] = true
-		}
-	}
-	for fn, cgn := range g.Nodes {
-		if cgn == g.Root || fn.Synthetic == "" || isInit(cgn.Func) {
-			continue // keep
-		}
-		for _, eIn := range cgn.In {
-			for _, eOut := range cgn.Out {
-				newEdge := Edge{eIn.Caller, eIn.Site, eOut.Callee}
-				if edges[newEdge] {
-					continue // don't add duplicate
-				}
-				AddEdge(eIn.Caller, eIn.Site, eOut.Callee)
-				edges[newEdge] = true
-			}
-		}
-		g.DeleteNode(cgn)
-	}
-}
-
-func isInit(fn *ssa.Function) bool {
-	return fn.Pkg != nil && fn.Pkg.Func("init") == fn
-}
-
-// DeleteNode removes node n and its edges from the graph g.
-// (NB: not efficient for batch deletion.)
-func (g *Graph) DeleteNode(n *Node) {
-	n.deleteIns()
-	n.deleteOuts()
-	delete(g.Nodes, n.Func)
-}
-
-// deleteIns deletes all incoming edges to n.
-func (n *Node) deleteIns() {
-	for _, e := range n.In {
-		removeOutEdge(e)
-	}
-	n.In = nil
-}
-
-// deleteOuts deletes all outgoing edges from n.
-func (n *Node) deleteOuts() {
-	for _, e := range n.Out {
-		removeInEdge(e)
-	}
-	n.Out = nil
-}
-
-// removeOutEdge removes edge.Caller's outgoing edge 'edge'.
-func removeOutEdge(edge *Edge) {
-	caller := edge.Caller
-	n := len(caller.Out)
-	for i, e := range caller.Out {
-		if e == edge {
-			// Replace it with the final element and shrink the slice.
-			caller.Out[i] = caller.Out[n-1]
-			caller.Out[n-1] = nil // aid GC
-			caller.Out = caller.Out[:n-1]
-			return
-		}
-	}
-	panic("edge not found: " + edge.String())
-}
-
-// removeInEdge removes edge.Callee's incoming edge 'edge'.
-func removeInEdge(edge *Edge) {
-	caller := edge.Callee
-	n := len(caller.In)
-	for i, e := range caller.In {
-		if e == edge {
-			// Replace it with the final element and shrink the slice.
-			caller.In[i] = caller.In[n-1]
-			caller.In[n-1] = nil // aid GC
-			caller.In = caller.In[:n-1]
-			return
-		}
-	}
-	panic("edge not found: " + edge.String())
-}

vendor/modules.txt

@@ -207,8 +207,6 @@ golang.org/x/tools/go/internal/packagesdriver
 golang.org/x/tools/internal/gopathwalk
 golang.org/x/tools/internal/semver
 golang.org/x/tools/internal/module
-golang.org/x/tools/go/callgraph
-golang.org/x/tools/go/callgraph/cha
 golang.org/x/tools/go/internal/gcimporter
 golang.org/x/tools/internal/fastwalk
 # gopkg.in/yaml.v2 v2.2.1
@@ -217,7 +215,7 @@ gopkg.in/yaml.v2
 mvdan.cc/interfacer/check
 # mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b
 mvdan.cc/lint
-# mvdan.cc/unparam v0.0.0-20190124213536-cc9d2fb52971
+# mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34
 mvdan.cc/unparam/check
 # sourcegraph.com/sourcegraph/go-diff v0.0.0-20171119081133-3f415a150aec
 sourcegraph.com/sourcegraph/go-diff/diff

vendor/mvdan.cc/unparam/check/check.go

@@ -21,8 +21,6 @@ import (
 	"sort"
 	"strings"
 
-	"golang.org/x/tools/go/callgraph"
-	"golang.org/x/tools/go/callgraph/cha"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/ssa"
 	"golang.org/x/tools/go/ssa/ssautil"
@@ -52,7 +50,6 @@ func UnusedParams(tests bool, exported, debug bool, args ...string) ([]string, e
 type Checker struct {
 	pkgs []*packages.Package
 	prog *ssa.Program
-	graph *callgraph.Graph
 
 	wd string
 
@@ -71,11 +68,20 @@ type Checker struct {
 	// to the declaration, as that could be either a FuncDecl or FuncLit.
 	funcBodyByPos map[token.Pos]*ast.BlockStmt
 
-	typesImplementing map[*types.Named]bool
+	// typesImplementing records the method names that each named type needs
+	// to typecheck properly, as they're required to implement interfaces.
+	typesImplementing map[*types.Named][]string
 
-	// Funcs used as a struct field or a func call argument. These are very
-	// often signatures which cannot be changed.
-	funcUsedAs map[*ssa.Function]string
+	// localCallSites is a very simple form of a callgraph, only recording
+	// direct function calls within a single package.
+	localCallSites map[*ssa.Function][]ssa.CallInstruction
+
+	// These three maps record whether an entire func's signature cannot be
+	// changed, or only its list of parameters or results.
+
+	signRequiredBy    map[*ssa.Function]string
+	paramsRequiredBy  map[*ssa.Function]string
+	resultsRequiredBy map[*ssa.Function]string
 }
 
 var errorType = types.Universe.Lookup("error").Type()
@@ -182,7 +188,7 @@ func (c *Checker) Check() ([]Issue, error) {
 	c.cachedDeclCounts = make(map[string]map[string]int)
 	c.callByPos = make(map[token.Pos]*ast.CallExpr)
 	c.funcBodyByPos = make(map[token.Pos]*ast.BlockStmt)
-	c.typesImplementing = make(map[*types.Named]bool)
+	c.typesImplementing = make(map[*types.Named][]string)
 
 	wantPkg := make(map[*types.Package]*packages.Package)
 	genFiles := make(map[string]bool)
@@ -200,16 +206,13 @@ func (c *Checker) Check() ([]Issue, error) {
 						len(node.Names) != 1 || node.Names[0].Name != "_" {
 						break
 					}
-					_, ok := pkg.TypesInfo.TypeOf(node.Type).Underlying().(*types.Interface)
+					iface, ok := pkg.TypesInfo.TypeOf(node.Type).Underlying().(*types.Interface)
 					if !ok {
 						break
 					}
+					// var _ someIface = named
 					valTyp := pkg.TypesInfo.Types[node.Values[0]].Type
-					named := findNamed(valTyp)
-					if named == nil {
-						break
-					}
-					c.typesImplementing[named] = true
+					c.addImplementing(findNamed(valTyp), iface)
 				case *ast.CallExpr:
 					c.callByPos[node.Lparen] = node
 				// ssa.Function.Pos returns the declaring
@@ -224,40 +227,121 @@ func (c *Checker) Check() ([]Issue, error) {
 			})
 		}
 	}
-	c.graph = cha.CallGraph(c.prog)
-	c.graph.DeleteSyntheticNodes()
-
 	allFuncs := ssautil.AllFunctions(c.prog)
 
-	c.funcUsedAs = make(map[*ssa.Function]string)
-	for fn := range allFuncs {
-		for _, b := range fn.Blocks {
+	// map from *ssa.FreeVar to *ssa.Function, to find function literals
+	// behind closure vars in the simpler scenarios.
+	freeVars := map[*ssa.FreeVar]*ssa.Function{}
+	for curFunc := range allFuncs {
+		for _, b := range curFunc.Blocks {
 			for _, instr := range b.Instrs {
+				instr, ok := instr.(*ssa.MakeClosure)
+				if !ok {
+					continue
+				}
+				fn := instr.Fn.(*ssa.Function)
+				for i, fv := range fn.FreeVars {
+					binding := instr.Bindings[i]
+					alloc, ok := binding.(*ssa.Alloc)
+					if !ok {
+						continue
+					}
+					for _, ref := range *alloc.Referrers() {
+						store, ok := ref.(*ssa.Store)
+						if !ok {
+							continue
+						}
+						if fn, ok := store.Val.(*ssa.Function); ok {
+							freeVars[fv] = fn
+							break
+						}
+					}
+				}
+			}
+		}
+	}
+
+	c.signRequiredBy = make(map[*ssa.Function]string)
+	c.paramsRequiredBy = make(map[*ssa.Function]string)
+	c.resultsRequiredBy = make(map[*ssa.Function]string)
+	c.localCallSites = make(map[*ssa.Function][]ssa.CallInstruction)
+	for curFunc := range allFuncs {
+		if strings.HasPrefix(curFunc.Synthetic, "wrapper for func") {
+			// Synthetic func wrappers are uninteresting, and can
+			// lead to false negatives.
+			continue
+		}
+		for _, b := range curFunc.Blocks {
+			for _, instr := range b.Instrs {
+				if instr, ok := instr.(ssa.CallInstruction); ok {
+					if fn := findFunction(freeVars, instr.Common().Value); fn != nil {
+						c.localCallSites[fn] = append(c.localCallSites[fn], instr)
+					}
+					fn := receivesExtractedArgs(freeVars, instr)
+					if fn != nil {
+						// fn(someFunc()) fixes params
+						c.paramsRequiredBy[fn] = "forwarded call"
+					}
+				}
 				switch instr := instr.(type) {
 				case *ssa.Call:
 					for _, arg := range instr.Call.Args {
-						if fn := findFunction(arg); fn != nil {
+						if fn := findFunction(freeVars, arg); fn != nil {
 							// someFunc(fn)
-							c.funcUsedAs[fn] = "param"
+							c.signRequiredBy[fn] = "call"
+						}
+					}
+				case *ssa.Phi:
+					for _, val := range instr.Edges {
+						if fn := findFunction(freeVars, val); fn != nil {
+							// nonConstVar = fn
+							c.signRequiredBy[fn] = "phi"
+						}
+					}
+				case *ssa.Return:
+					for _, val := range instr.Results {
+						if fn := findFunction(freeVars, val); fn != nil {
+							// return fn
+							c.signRequiredBy[fn] = "result"
+						}
+					}
+					if call := callExtract(instr, instr.Results); call != nil {
+						if fn := findFunction(freeVars, call.Call.Value); fn != nil {
+							// return fn()
+							c.resultsRequiredBy[fn] = "return"
 						}
 					}
 				case *ssa.Store:
-					if _, ok := instr.Addr.(*ssa.FieldAddr); !ok {
-						break
-					}
-					if fn := findFunction(instr.Val); fn != nil {
+					as := ""
+					switch instr.Addr.(type) {
+					case *ssa.FieldAddr:
 						// x.someField = fn
-						c.funcUsedAs[fn] = "field"
+						as = "field"
+					case *ssa.IndexAddr:
+						// x[someIndex] = fn
+						as = "element"
+					case *ssa.Global:
+						// someGlobal = fn
+						as = "global"
+					default:
+						continue
+					}
+					if fn := findFunction(freeVars, instr.Val); fn != nil {
+						c.signRequiredBy[fn] = as
 					}
 				case *ssa.MakeInterface:
-					if fn := findFunction(instr.X); fn != nil {
+					// someIface(named)
+					iface := instr.Type().Underlying().(*types.Interface)
+					c.addImplementing(findNamed(instr.X.Type()), iface)
+
+					if fn := findFunction(freeVars, instr.X); fn != nil {
 						// emptyIface = fn
-						c.funcUsedAs[fn] = "interface"
+						c.signRequiredBy[fn] = "interface"
 					}
 				case *ssa.ChangeType:
-					if fn := findFunction(instr.X); fn != nil {
+					if fn := findFunction(freeVars, instr.X); fn != nil {
 						// someType(fn)
-						c.funcUsedAs[fn] = "type conversion"
+						c.signRequiredBy[fn] = "type conversion"
 					}
 				}
 			}
@@ -303,6 +387,29 @@ func (c *Checker) Check() ([]Issue, error) {
 	return c.issues, nil
 }
 
+func stringsContains(list []string, elem string) bool {
+	for _, e := range list {
+		if e == elem {
+			return true
+		}
+	}
+	return false
+}
+
+func (c *Checker) addImplementing(named *types.Named, iface *types.Interface) {
+	if named == nil || iface == nil {
+		return
+	}
+	list := c.typesImplementing[named]
+	for i := 0; i < iface.NumMethods(); i++ {
+		name := iface.Method(i).Name()
+		if !stringsContains(list, name) {
+			list = append(list, name)
+		}
+	}
+	c.typesImplementing[named] = list
+}
+
 func findNamed(typ types.Type) *types.Named {
 	switch typ := typ.(type) {
 	case *types.Pointer:
@@ -314,13 +421,36 @@ func findNamed(typ types.Type) *types.Named {
 }
 
 // findFunction returns the function that is behind a value, if any.
-func findFunction(value ssa.Value) *ssa.Function {
+func findFunction(freeVars map[*ssa.FreeVar]*ssa.Function, value ssa.Value) *ssa.Function {
 	switch value := value.(type) {
 	case *ssa.Function:
+		name := value.Name()
+		if strings.HasSuffix(name, "$thunk") || strings.HasSuffix(name, "$bound") {
+			// Method wrapper funcs contain a single block, which
+			// calls the function being wrapped, and returns. We
+			// want the function being wrapped.
+			for _, instr := range value.Blocks[0].Instrs {
+				call, ok := instr.(*ssa.Call)
+				if !ok {
+					continue
+				}
+				if callee := call.Call.StaticCallee(); callee != nil {
+					return callee
+				}
+			}
+			return nil // no static callee?
+		}
 		return value
 	case *ssa.MakeClosure:
 		// closure of a func
-		return value.Fn.(*ssa.Function)
+		return findFunction(freeVars, value.Fn)
+	case *ssa.UnOp:
+		if value.Op != token.MUL {
+			break
+		}
+		if fv, ok := value.X.(*ssa.FreeVar); ok {
+			return freeVars[fv]
+		}
 	}
 	return nil
 }
@@ -349,22 +479,14 @@ func (c *Checker) checkFunc(fn *ssa.Function, pkg *packages.Package) {
 		c.debug("  skip - dummy implementation\n")
 		return
 	}
-	var inboundCalls []*callgraph.Edge
-	if node := c.graph.Nodes[fn]; node != nil {
-		inboundCalls = node.In
-	}
-	if requiredViaCall(fn, inboundCalls) {
-		c.debug("  skip - type is required via call\n")
-		return
-	}
-	if usedAs := c.funcUsedAs[fn]; usedAs != "" {
-		c.debug("  skip - func is used as a %s\n", usedAs)
+	if by := c.signRequiredBy[fn]; by != "" {
+		c.debug("  skip - func signature required by %s\n", by)
 		return
 	}
 	if recv := fn.Signature.Recv(); recv != nil {
 		named := findNamed(recv.Type())
-		if c.typesImplementing[named] {
-			c.debug("  skip - receiver must implement an interface\n")
+		if stringsContains(c.typesImplementing[named], fn.Name()) {
+			c.debug("  skip - method required to implement an interface\n")
 			return
 		}
 	}
@@ -372,12 +494,18 @@ func (c *Checker) checkFunc(fn *ssa.Function, pkg *packages.Package) {
 		c.debug("  skip - multiple implementations via build tags\n")
 		return
 	}
+	paramsBy := c.paramsRequiredBy[fn]
+	resultsBy := c.resultsRequiredBy[fn]
+	callSites := c.localCallSites[fn]
 
 	results := fn.Signature.Results()
 	sameConsts := make([]*ssa.Const, results.Len())
 	numRets := 0
 	allRetsExtracting := true
 	for _, block := range fn.Blocks {
+		if resultsBy != "" {
+			continue // we can't change the returns
+		}
 		last := block.Instrs[len(block.Instrs)-1]
 		ret, ok := last.(*ssa.Return)
 		if !ok {
@@ -406,9 +534,6 @@ func (c *Checker) checkFunc(fn *ssa.Function, pkg *packages.Package) {
 			// false positives)
 			continue
 		}
-		if calledInReturn(inboundCalls) {
-			continue
-		}
 		res := results.At(i)
 		name := paramDesc(i, res)
 		c.addIssue(fn, res.Pos(), "result %s is always %s", name, constValueString(cnst))
@@ -416,6 +541,9 @@ func (c *Checker) checkFunc(fn *ssa.Function, pkg *packages.Package) {
 
 resLoop:
 	for i := 0; i < results.Len(); i++ {
+		if resultsBy != "" {
+			continue // we can't change the returns
+		}
 		if allRetsExtracting {
 			continue
 		}
@@ -426,8 +554,8 @@ resLoop:
 			continue
 		}
 		count := 0
-		for _, edge := range inboundCalls {
-			val := edge.Site.Value()
+		for _, site := range callSites {
+			val := site.Value()
 			if val == nil { // e.g. go statement
 				count++
 				continue
@@ -454,6 +582,9 @@ resLoop:
 	}
 
 	for i, par := range fn.Params {
+		if paramsBy != "" {
+			continue // we can't change the params
+		}
 		if i == 0 && fn.Signature.Recv() != nil { // receiver
 			continue
 		}
@@ -468,7 +599,7 @@ resLoop:
 			continue
 		}
 		reason := "is unused"
-		constStr := c.alwaysReceivedConst(inboundCalls, par, i)
+		constStr := c.alwaysReceivedConst(callSites, par, i)
 		if constStr != "" {
 			reason = fmt.Sprintf("always receives %s", constStr)
 		} else if c.anyRealUse(par, i, pkg) {
@@ -479,43 +610,6 @@ resLoop:
 	}
 }
 
-// calledInReturn reports whether any of a function's inbound calls happened
-// directly as a return statement. That is, if function "foo" was used via
-// "return foo()". This means that the result parameters of the function cannot
-// be changed without breaking other code.
-func calledInReturn(in []*callgraph.Edge) bool {
-	for _, edge := range in {
-		val := edge.Site.Value()
-		if val == nil { // e.g. go statement
-			continue
-		}
-		refs := *val.Referrers()
-		if len(refs) == 0 { // no use of return values
-			continue
-		}
-		allReturnExtracts := true
-		for _, instr := range refs {
-			switch x := instr.(type) {
-			case *ssa.Return:
-				return true
-			case *ssa.Extract:
-				refs := *x.Referrers()
-				if len(refs) != 1 {
-					allReturnExtracts = false
-					break
-				}
-				if _, ok := refs[0].(*ssa.Return); !ok {
-					allReturnExtracts = false
-				}
-			}
-		}
-		if allReturnExtracts {
-			return true
-		}
-	}
-	return false
-}
-
 // nodeStr stringifies a syntax tree node. It is only meant for simple nodes,
 // such as short value expressions.
 func nodeStr(node ast.Node) string {
@@ -534,8 +628,8 @@ func nodeStr(node ast.Node) string {
 // This function is used to recommend that the parameter be replaced by a direct
 // use of the constant. To avoid false positives, the function will return false
 // if the number of inbound calls is too low.
-func (c *Checker) alwaysReceivedConst(in []*callgraph.Edge, par *ssa.Parameter, pos int) string {
-	if len(in) < 4 {
+func (c *Checker) alwaysReceivedConst(callSites []ssa.CallInstruction, par *ssa.Parameter, pos int) string {
+	if len(callSites) < 4 {
 		// We can't possibly receive the same constant value enough
 		// times, hence a potential false positive.
 		return ""
@@ -552,8 +646,8 @@ func (c *Checker) alwaysReceivedConst(in []*callgraph.Edge, par *ssa.Parameter,
 		origPos--
 	}
 	seenOrig := ""
-	for _, edge := range in {
-		call := edge.Site.Common()
+	for _, site := range callSites {
+		call := site.Common()
 		if pos >= len(call.Args) {
 			// TODO: investigate? Weird crash in
 			// internal/x/net/http2/hpack/hpack_test.go, where we
@@ -612,8 +706,15 @@ func (c *Checker) anyRealUse(par *ssa.Parameter, pos int, pkg *packages.Package)
 			if any {
 				return false
 			}
-			if id, ok := node.(*ast.Ident); ok {
-				obj := pkg.TypesInfo.Uses[id]
+			asgn, ok := node.(*ast.AssignStmt)
+			if !ok || asgn.Tok != token.ASSIGN || len(asgn.Lhs) != 1 || len(asgn.Rhs) != 1 {
+				return true
+			}
+			if left, ok := asgn.Lhs[0].(*ast.Ident); !ok || left.Name != "_" {
+				return true
+			}
+			if right, ok := asgn.Rhs[0].(*ast.Ident); ok {
+				obj := pkg.TypesInfo.Uses[right]
 				if obj != nil && obj.Pos() == par.Pos() {
 					any = true
 				}
@@ -798,29 +899,72 @@ func (c *Checker) multipleImpls(pkg *packages.Package, fn *ssa.Function) bool {
 	return count[name] > 1
 }
 
-// receivesExtractedArgs reports whether a function call got all of its
-// arguments via another function call. That is, if a call to function "foo" was
-// of the form "foo(bar())".
-func receivesExtractedArgs(sign *types.Signature, call *ssa.Call) bool {
-	if call == nil {
-		return false
+// receivesExtractedArgs returns the statically called function, if its multiple
+// arguments were all received via another function call. That is, if a call to
+// function "foo" was of the form "foo(bar())". This often means that the
+// parameters in "foo" are difficult to remove, even if unused.
+func receivesExtractedArgs(freeVars map[*ssa.FreeVar]*ssa.Function, call ssa.CallInstruction) *ssa.Function {
+	comm := call.Common()
+	callee := findFunction(freeVars, comm.Value)
+	if callee == nil {
+		return nil
 	}
-	if sign.Params().Len() < 2 {
-		return false // extracting into one param is ok
+	if callee.Signature.Params().Len() < 2 {
+		// there aren't multiple parameters
+		return nil
 	}
-	args := call.Operands(nil)
-	for i, arg := range args {
-		if i == 0 {
-			continue // *ssa.Function, func itself
+	args := comm.Args
+	if callee.Signature.Recv() != nil {
+		// skip the receiver argument
+		args = args[1:]
 	}
-		if i == 1 && sign.Recv() != nil {
-			continue // method receiver
+	if c := callExtract(call, args); c != nil {
+		return callee
 	}
-		if _, ok := (*arg).(*ssa.Extract); !ok {
-			return false
+	return nil
+}
+
+// callExtract returns the call instruction fn(...) if it is used directly as
+// arguments to the parent instruction, such as fn2(fn(...)) or return fn(...).
+func callExtract(parent ssa.Instruction, values []ssa.Value) *ssa.Call {
+	if len(values) == 1 {
+		if call, ok := values[0].(*ssa.Call); ok {
+			return call
 		}
 	}
-	return true
+	var prev *ssa.Call
+	for i, val := range values {
+		ext, ok := val.(*ssa.Extract)
+		if !ok {
+			return nil
+		}
+		if ext.Index != i {
+			return nil // not extracted in the same order
+		}
+		call, ok := ext.Tuple.(*ssa.Call)
+		if !ok {
+			return nil // not a call
+		}
+		if prev == nil {
+			prev = call
+		} else if prev != call {
+			return nil // not the same calls
+		}
+	}
+	if prev == nil {
+		return nil
+	}
+	if prev.Call.Signature().Results().Len() != len(values) {
+		return nil // not extracting all the results
+	}
+	if prev.Pos() < parent.Pos() {
+		// Of the form:
+		//
+		//   a, b := fn()
+		//   fn2(a, b)
+		return nil
+	}
+	return prev
 }
 
 // paramDesc returns a string describing a parameter variable. If the parameter
@@ -833,42 +977,3 @@ func paramDesc(i int, v *types.Var) string {
 	}
 	return fmt.Sprintf("%d (%s)", i, v.Type().String())
 }
-
-// requiredViaCall reports whether a function has any inbound call that strongly
-// depends on the function's signature. For example, if the function is accessed
-// via a field, or if it gets its arguments from another function call. In these
-// cases, changing the function signature would mean a larger refactor.
-func requiredViaCall(fn *ssa.Function, calls []*callgraph.Edge) bool {
-	for _, edge := range calls {
-		call := edge.Site.Value()
-		if receivesExtractedArgs(fn.Signature, call) {
-			// called via fn(x())
-			return true
-		}
-		switch edge.Site.Common().Value.(type) {
-		case *ssa.Parameter:
-			// Func used as parameter; not safe.
-			return true
-		case *ssa.Call:
-			// Called through an interface; not safe.
-			return true
-		case *ssa.TypeAssert:
-			// Called through a type assertion; not safe.
-			return true
-		case *ssa.Const:
-			// Conservative "may implement" edge; not safe.
-			return true
-		case *ssa.UnOp:
-			// TODO: why is this not safe?
-			return true
-		case *ssa.Phi:
-			// We may run this function or another; not safe.
-			return true
-		case *ssa.Function:
-			// Called directly; the call site can easily be adapted.
-		default:
-			// Other instructions are safe or non-interesting.
-		}
-	}
-	return false
-}