fix(lint): only count a direct call as an inlinable use

The reference counter treated every name or member expression typed as a
function as an inlinable use, so a helper referenced once as a value, whether
assigned to a function pointer, returned, or passed as a callback, was
reported even though there is no call site to inline into. Record whether a
reference is the callee of a call and report only when the single reference is
a direct call. Updates the pointer fixture to stay clean and adds the returned
and callback value positions, alongside a function actually called once that
still reports.

Author: 0xMars42

crates/lint/src/sol/info/internal_function_used_once.rs

@@ -78,13 +78,17 @@ impl<'ast> ProjectLintPass<'ast> for InternalFunctionUsedOnce {
             if name.as_str().starts_with('_') {
                 continue;
             }
-            // Exactly one reference: zero references is dead code, a different concern.
-            // Self-references do not count and mark the function recursive, which rules it
-            // out entirely: a recursive function cannot be inlined into its caller. A single
-            // reference that only enters through a reference cycle (mutually recursive
-            // helpers with no external caller) has no caller to inline into either.
+            // Exactly one reference, and it must be a direct call: zero references is dead
+            // code, a different concern, and a reference used as a value (assigned to a
+            // function pointer, returned, or passed as a callback) has no call site to inline
+            // into, so it is not an inlining candidate. Self-references do not count and mark
+            // the function recursive, which rules it out entirely: a recursive function cannot
+            // be inlined into its caller. A single reference that only enters through a
+            // reference cycle (mutually recursive helpers with no external caller) has no
+            // caller to inline into either.
             let Some(info) = counts.get(&function_id) else { continue };
             if info.count == 1
+                && info.value_count == 0
                 && !info.self_referencing
                 && !only_referenced_within_cycle(&counts, function_id)
             {
@@ -122,6 +126,7 @@ fn operator_bound_functions(hir: &hir::Hir<'_>) -> HashSet<hir::FunctionId> {
 #[derive(Default)]
 struct RefInfo {
     count: usize,
+    value_count: usize,
     self_referencing: bool,
     first_from: Option<hir::FunctionId>,
 }
@@ -194,23 +199,58 @@ impl<'gcx> hir::Visit<'gcx> for ReferenceCounter<'gcx> {
     }
 
     fn visit_expr(&mut self, expr: &'gcx hir::Expr<'gcx>) -> ControlFlow<Self::BreakValue> {
-        // A name or member expression typed as a function is one resolved reference. A
-        // self-reference marks the function recursive instead of counting.
-        if matches!(expr.kind, hir::ExprKind::Ident(..) | hir::ExprKind::Member(..))
-            && let Some(ty) = self.gcx.type_of_expr(expr.peel_parens().id)
-            && let TyKind::Fn(function_ty) = ty.kind
-            && let Some(function_id) = function_ty.function_id
-        {
-            let info = self.refs.entry(function_id).or_default();
-            if self.current == Some(function_id) {
-                info.self_referencing = true;
-            } else {
-                info.count += 1;
-                if info.count == 1 {
-                    info.first_from = self.current;
+        // The callee of a call is a direct, inlinable use: record it as a call and descend into
+        // the callee's own sub-expressions (a member receiver, a nested call) without also
+        // counting the callee node as a value reference. Every other name or member expression
+        // typed as a function is a value-position reference, such as a function pointer that is
+        // assigned, returned or passed as a callback, which has no call site to inline into.
+        if let hir::ExprKind::Call(callee, args, opts) = &expr.kind {
+            let peeled = callee.peel_parens();
+            match peeled.kind {
+                hir::ExprKind::Ident(_) => self.record_reference(peeled, true),
+                hir::ExprKind::Member(receiver, _) => {
+                    self.record_reference(peeled, true);
+                    let _ = self.visit_expr(receiver);
+                }
+                _ => {
+                    let _ = self.visit_expr(peeled);
+                }
+            }
+            if let Some(opts) = opts {
+                for arg in opts.args {
+                    let _ = self.visit_expr(&arg.value);
                 }
             }
+            return self.visit_call_args(args);
+        }
+        if matches!(expr.kind, hir::ExprKind::Ident(..) | hir::ExprKind::Member(..)) {
+            self.record_reference(expr, false);
         }
         self.walk_expr(expr)
     }
 }
+
+impl<'gcx> ReferenceCounter<'gcx> {
+    /// Records one resolved function reference from `expr`, marking whether it is a direct call
+    /// or a value-position use. `type_of_expr` gives the single declaration the type checker
+    /// selected, so overloads, the qualified and `using for` forms and import aliases are all
+    /// attributed to the right function. A self-reference marks the function recursive rather
+    /// than counting.
+    fn record_reference(&mut self, expr: &hir::Expr<'_>, is_call: bool) {
+        let Some(ty) = self.gcx.type_of_expr(expr.peel_parens().id) else { return };
+        let TyKind::Fn(function_ty) = ty.kind else { return };
+        let Some(function_id) = function_ty.function_id else { return };
+        let info = self.refs.entry(function_id).or_default();
+        if self.current == Some(function_id) {
+            info.self_referencing = true;
+        } else {
+            info.count += 1;
+            if info.count == 1 {
+                info.first_from = self.current;
+            }
+            if !is_call {
+                info.value_count += 1;
+            }
+        }
+    }
+}

crates/lint/testdata/InternalFunctionUsedOnce.sol

@@ -2,13 +2,15 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.18;
 
-// Tests for `internal-function-used-once`: an internal function referenced exactly once in the
-// whole unit can usually be inlined into its caller. References are resolved through the type
-// checker, calls and values alike. Out of scope: functions whose name starts with `_` (hook
-// convention), `virtual` functions and overrides (they exist for dynamic dispatch), functions
-// referenced zero times (dead code is another concern) or more than once, and functions bound
-// as user-defined operators (their operator uses are not name references, and the binding
-// requires a named function).
+// Tests for `internal-function-used-once`: an internal function called exactly once in the
+// whole unit can usually be inlined into its caller. The single reference must be a direct
+// call, resolved through the type checker; a reference used as a value (a function pointer
+// assigned, returned or passed as a callback) has no call site to inline into and is out of
+// scope. Also out of scope: functions whose name starts with `_` (hook convention), `virtual`
+// functions and overrides (they exist for dynamic dispatch), functions referenced zero times
+// (dead code is another concern) or more than once, and functions bound as user-defined
+// operators (their operator uses are not name references, and the binding requires a named
+// function).
 
 library MathLib {
     function onceQualified(uint256 v) internal pure returns (uint256) { //~NOTE: this internal function is used only once
@@ -46,7 +48,9 @@ contract Counter {
         return v == 0 ? 0 : recurse(v - 1);
     }
 
-    function pointed(uint256 v) internal pure returns (uint256) { //~NOTE: this internal function is used only once
+    // Referenced exactly once, but as a value assigned to a function pointer rather than a
+    // direct call: there is no call site to inline into, so it stays out of scope.
+    function pointed(uint256 v) internal pure returns (uint256) {
         return v + 6;
     }
 
@@ -56,7 +60,6 @@ contract Counter {
     }
 
     function runAgain(uint256 v) internal {
-        // a reference used as a value is a reference like any other
         function(uint256) internal pure returns (uint256) f = pointed;
         count = twice(f(v));
     }
@@ -178,3 +181,35 @@ contract TailOffCycle {
         return cycleA(v);
     }
 }
+
+// The remaining value positions: a helper returned as a function pointer and one passed as a
+// callback argument. Neither is a call site to inline into, so both stay out of scope, while
+// the function actually called once (`invoke`) is reported.
+contract ValuePositions {
+    function returned(uint256 v) internal pure returns (uint256) {
+        return v + 1;
+    }
+
+    function passed(uint256 v) internal pure returns (uint256) {
+        return v + 2;
+    }
+
+    function invoke( //~NOTE: this internal function is used only once
+        function(uint256) internal pure returns (uint256) cb,
+        uint256 v
+    ) internal pure returns (uint256) {
+        return cb(v);
+    }
+
+    function pickReturned()
+        internal
+        pure
+        returns (function(uint256) internal pure returns (uint256))
+    {
+        return returned;
+    }
+
+    function usePassed(uint256 v) internal pure returns (uint256) {
+        return invoke(passed, v);
+    }
+}

crates/lint/testdata/InternalFunctionUsedOnce.stderr

@@ -25,23 +25,23 @@ LL │     function helper(uint256 v) internal pure returns (uint256) {
 note[internal-function-used-once]: this internal function is used only once; consider inlining it into its caller
    ╭▸ ROOT/testdata/InternalFunctionUsedOnce.sol:LL:CC
    │
-LL │     function pointed(uint256 v) internal pure returns (uint256) {
-   │     ━━━━━━━━
+LL │ function freeOnce(uint256 v) pure returns (uint256) {
+   │ ━━━━━━━━
    │
    ╰ help: https://getfoundry.sh/forge/linting/internal-function-used-once
 
 note[internal-function-used-once]: this internal function is used only once; consider inlining it into its caller
    ╭▸ ROOT/testdata/InternalFunctionUsedOnce.sol:LL:CC
    │
-LL │ function freeOnce(uint256 v) pure returns (uint256) {
-   │ ━━━━━━━━
+LL │     function tail(uint256 v) internal pure returns (uint256) {
+   │     ━━━━━━━━
    │
    ╰ help: https://getfoundry.sh/forge/linting/internal-function-used-once
 
 note[internal-function-used-once]: this internal function is used only once; consider inlining it into its caller
    ╭▸ ROOT/testdata/InternalFunctionUsedOnce.sol:LL:CC
    │
-LL │     function tail(uint256 v) internal pure returns (uint256) {
+LL │     function invoke(
    │     ━━━━━━━━
    │
    ╰ help: https://getfoundry.sh/forge/linting/internal-function-used-once