fix(coverage): exclude only virtual fns without impl (#12216)

Author: grandizzy

crates/evm/coverage/src/analysis.rs

@@ -218,7 +218,15 @@ impl<'ast> ast::Visit<'ast> for SourceVisitor<'_> {
                     }
                 });
 
-                self.push_item_kind(CoverageItemKind::Function { name: name.into() }, item.span);
+                // Exclude function from coverage report if it is virtual without implementation.
+                let exclude_func = func.header.virtual_() && !func.is_implemented();
+                if !exclude_func {
+                    self.push_item_kind(
+                        CoverageItemKind::Function { name: name.into() },
+                        item.span,
+                    );
+                }
+
                 self.walk_item(item)?;
             }
             _ => {}
@@ -456,7 +464,7 @@ impl SourceAnalysis {
     /// Analyzes contracts in the sources held by the source analyzer.
     ///
     /// Coverage items are found by:
-    /// - Walking the AST of each contract (except interfaces and abstract contracts)
+    /// - Walking the AST of each contract (except interfaces)
     /// - Recording the items of each contract
     ///
     /// Each coverage item contains relevant information to find opcodes corresponding to them: the
@@ -483,9 +491,8 @@ impl SourceAnalysis {
                         // Visit only top-level contracts.
                         let ItemKind::Contract(contract) = &item.kind else { continue };
 
-                        // Skip interfaces and abstract contracts which have no function
-                        // implementations.
-                        if contract.kind.is_interface() || contract.kind.is_abstract_contract() {
+                        // Skip interfaces which have no function implementations.
+                        if contract.kind.is_interface() {
                             continue;
                         }
 

crates/forge/tests/cli/coverage.rs

@@ -2147,6 +2147,10 @@ interface ContractIf {
 
 abstract contract AbstractCounter {
     function _setNumber(uint256 newNumber) internal virtual;
+
+    function _incrementNumber(uint256 newNumber) internal virtual returns (uint256 inc) {
+        inc = newNumber + 1;
+    }
 }
 
 contract Counter is AbstractCounter, ContractIf {
@@ -2157,7 +2161,11 @@ contract Counter is AbstractCounter, ContractIf {
     }
 
     function _setNumber(uint256 newNumber) internal override {
-        number = newNumber;
+        number = _incrementNumber(newNumber);
+    }
+
+    function _incrementNumber(uint256 newNumber) internal override returns (uint256 inc) {
+        inc = super._incrementNumber(newNumber);
     }
 }
     "#,
@@ -2177,14 +2185,18 @@ contract CounterTest is DSTest {
     "#,
     );
 
+    // Test there are 4 functions reported:
+    // - `setNumber`, `_setNumber` and `_incrementNumber` from `Counter` contract
+    // - `_incrementNumber` from `AbstractCounter` (virtual with implementation). `_setNumber` is
+    // excluded as it is not implemented.
     cmd.arg("coverage").assert_success().stdout_eq(str![[r#"
 ...
 ╭-----------------+---------------+---------------+---------------+---------------╮
 | File            | % Lines       | % Statements  | % Branches    | % Funcs       |
 +=================================================================================+
-| src/Counter.sol | 100.00% (4/4) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |
+| src/Counter.sol | 100.00% (8/8) | 100.00% (4/4) | 100.00% (0/0) | 100.00% (4/4) |
 |-----------------+---------------+---------------+---------------+---------------|
-| Total           | 100.00% (4/4) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |
+| Total           | 100.00% (8/8) | 100.00% (4/4) | 100.00% (0/0) | 100.00% (4/4) |
 ╰-----------------+---------------+---------------+---------------+---------------╯
 ...
 "#]]);