Repository: mgechev/revive (go)

Issue:
Expand `unused-parameter` to check anonymous functions
**Is your feature request related to a problem? Please describe.**

The `unused-parameter` is phenomenally useful, however it doesn't seem to include anonymous functions. Had this linter included anonymous functions, we would have been able to catch a bug before it made its way into a recent release of ours.

**Describe the solution you'd like**

Anonymous functions should be tracked just like normal functions for unused variable usages.

In [this example](https://go.dev/play/p/Kl3o91k9jl2), `a` is unused. There could be a suggestion to use `func(int)` (or `func(_ int)` rather than `func(a int)`.

**Describe alternatives you've considered**

Depending on how `unused-parameter` is implemented, it might make sense to have this be a separate rule entirely, like `unused-receiver`.

**Additional context**

Here is a [bug fix](https://github.com/ava-labs/coreth/pull/461/files#diff-4968a027180246a1a0b75376b875471a5d85755bac928434cea30b23b153240dL155-L158) that would have been prevented had this linter existed.


Pull request:
Check function literals in `unused-param`
Closes #965

See #965 for the motivation behind this PR.

- [X] Added tests
- [X] I believe this code follows the existing style, but if this could be improved I'm very open to any feedback
- [x] Passes CI

This does potentially significantly change the performance of this rule because we will now walk into the function bodies. If we are concerned about a performance regression here I think an alternative could be to either add a new rule or add a configuration to the rule to gate checking anonymous functions.


Patch:
diff --git a/rule/unused-param.go b/rule/unused-param.go
index 63eab37..4b04ee9 100644
--- a/rule/unused-param.go
+++ b/rule/unused-param.go
@@ -87,54 +87,64 @@ type lintUnusedParamRule struct {
 }
 
 func (w lintUnusedParamRule) Visit(node ast.Node) ast.Visitor {
+	var (
+		funcType *ast.FuncType
+		funcBody *ast.BlockStmt
+	)
 	switch n := node.(type) {
+	case *ast.FuncLit:
+		funcType = n.Type
+		funcBody = n.Body
 	case *ast.FuncDecl:
-		params := retrieveNamedParams(n.Type.Params)
-		if len(params) < 1 {
-			return nil // skip, func without parameters
-		}
-
 		if n.Body == nil {
 			return nil // skip, is a function prototype
 		}
 
-		// inspect the func body looking for references to parameters
-		fselect := func(n ast.Node) bool {
-			ident, isAnID := n.(*ast.Ident)
+		funcType = n.Type
+		funcBody = n.Body
+	default:
+		return w // skip, not a function
+	}
 
-			if !isAnID {
-				return false
-			}
+	params := retrieveNamedParams(funcType.Params)
+	if len(params) < 1 {
+		return w // skip, func without parameters
+	}
 
-			_, isAParam := params[ident.Obj]
-			if isAParam {
-				params[ident.Obj] = false // mark as used
-			}
+	// inspect the func body looking for references to parameters
+	fselect := func(n ast.Node) bool {
+		ident, isAnID := n.(*ast.Ident)
 
+		if !isAnID {
 			return false
 		}
-		_ = pick(n.Body, fselect)
-
-		for _, p := range n.Type.Params.List {
-			for _, n := range p.Names {
-				if w.allowRegex.FindStringIndex(n.Name) != nil {
-					continue
-				}
-				if params[n.Obj] {
-					w.onFailure(lint.Failure{
-						Confidence: 1,
-						Node:       n,
-						Category:   "bad practice",
-						Failure:    fmt.Sprintf(w.failureMsg, n.Name),
-					})
-				}
-			}
+
+		_, isAParam := params[ident.Obj]
+		if isAParam {
+			params[ident.Obj] = false // mark as used
 		}
 
-		return nil // full method body already inspected
+		return false
+	}
+	_ = pick(funcBody, fselect)
+
+	for _, p := range funcType.Params.List {
+		for _, n := range p.Names {
+			if w.allowRegex.FindStringIndex(n.Name) != nil {
+				continue
+			}
+			if params[n.Obj] {
+				w.onFailure(lint.Failure{
+					Confidence: 1,
+					Node:       n,
+					Category:   "bad practice",
+					Failure:    fmt.Sprintf(w.failureMsg, n.Name),
+				})
+			}
+		}
 	}
 
-	return w
+	return w // full method body was inspected
 }
 
 func retrieveNamedParams(params *ast.FieldList) map[*ast.Object]bool {


Test patch:
diff --git a/testdata/unused-param.go b/testdata/unused-param.go
index a7ae1e8..e03b2d5 100644
--- a/testdata/unused-param.go
+++ b/testdata/unused-param.go
@@ -166,3 +166,19 @@ func encodeFixed64Rpc(dAtA []byte, offset int, v uint64, i int) int {
 
 	return 8
 }
+
+func innerAnonymousFunctionWithoutUsage() {
+	innerFunc := func(a int) {} // MATCH /parameter 'a' seems to be unused, consider removing or renaming it as _/
+	innerFunc(1)
+}
+
+func innerAnonymousFunctionWithUsage() {
+	innerFunc := func(a int) {
+		a += 1
+	}
+	innerFunc(1)
+
+	return someFunc(func(values []int) float64 { // MATCH /parameter 'values' seems to be unused, consider removing or renaming it as _/
+		return 1.1
+	})
+}