fix(coverage): require should be branch (#8433)

foundry-rs · Jul 13, 2024 · 5902a6f · 5902a6f
1 parent 547e975
commit 5902a6f
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 9 deletions.
diff --git a/crates/evm/coverage/src/analysis.rs b/crates/evm/coverage/src/analysis.rs
@@ -325,17 +325,42 @@ impl<'a> ContractVisitor<'a> {
         //  tupleexpression
         //  yulfunctioncall
         match node.node_type {
-            NodeType::Assignment |
-            NodeType::UnaryOperation |
-            NodeType::FunctionCall |
-            NodeType::Conditional => {
+            NodeType::Assignment | NodeType::UnaryOperation | NodeType::Conditional => {
                 self.push_item(CoverageItem {
                     kind: CoverageItemKind::Statement,
                     loc: self.source_location_for(&node.src),
                     hits: 0,
                 });
                 Ok(())
             }
+            NodeType::FunctionCall => {
+                self.push_item(CoverageItem {
+                    kind: CoverageItemKind::Statement,
+                    loc: self.source_location_for(&node.src),
+                    hits: 0,
+                });
+
+                let expr: Option<Node> = node.attribute("expression");
+                if let Some(NodeType::Identifier) = expr.as_ref().map(|expr| &expr.node_type) {
+                    // Might be a require call, add branch coverage.
+                    let name: Option<String> = expr.and_then(|expr| expr.attribute("name"));
+                    if let Some("require") = name.as_deref() {
+                        let branch_id = self.branch_id;
+                        self.branch_id += 1;
+                        self.push_item(CoverageItem {
+                            kind: CoverageItemKind::Branch { branch_id, path_id: 0 },
+                            loc: self.source_location_for(&node.src),
+                            hits: 0,
+                        });
+                        self.push_item(CoverageItem {
+                            kind: CoverageItemKind::Branch { branch_id, path_id: 1 },
+                            loc: self.source_location_for(&node.src),
+                            hits: 0,
+                        });
+                    }
+                }
+                Ok(())
+            }
             NodeType::BinaryOperation => {
                 self.push_item(CoverageItem {
                     kind: CoverageItemKind::Statement,

diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs
@@ -167,15 +167,14 @@ contract BContractTest is DSTest {
 "#]]);
 });
 
-forgetest!(test_assert_require_coverage, |prj, cmd| {
+forgetest!(test_assert_coverage, |prj, cmd| {
     prj.insert_ds_test();
     prj.add_source(
         "AContract.sol",
         r#"
 contract AContract {
     function checkA() external pure returns (bool) {
         assert(10 > 2);
-        require(10 > 2, "true");
         return true;
     }
 }
@@ -200,17 +199,84 @@ contract AContractTest is DSTest {
     )
     .unwrap();
 
-    // Assert 100% coverage (assert and require properly covered).
+    // Assert 100% coverage (assert properly covered).
     cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[r#"
 ...
 | File              | % Lines       | % Statements  | % Branches    | % Funcs       |
 |-------------------|---------------|---------------|---------------|---------------|
-| src/AContract.sol | 100.00% (3/3) | 100.00% (3/3) | 100.00% (0/0) | 100.00% (1/1) |
-| Total             | 100.00% (3/3) | 100.00% (3/3) | 100.00% (0/0) | 100.00% (1/1) |
+| src/AContract.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (1/1) |
+| Total             | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (1/1) |
 
 "#]]);
 });
 
+forgetest!(test_require_coverage, |prj, cmd| {
+    prj.insert_ds_test();
+    prj.add_source(
+        "AContract.sol",
+        r#"
+contract AContract {
+    function checkRequire(bool doNotRevert) public view {
+        require(doNotRevert, "reverted");
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    prj.add_source(
+        "AContractTest.sol",
+        r#"
+import "./test.sol";
+import {AContract} from "./AContract.sol";
+
+interface Vm {
+    function expectRevert(bytes calldata revertData) external;
+}
+
+contract AContractTest is DSTest {
+    Vm constant vm = Vm(HEVM_ADDRESS);
+    function testRequireRevert() external {
+        AContract a = new AContract();
+        vm.expectRevert(abi.encodePacked("reverted"));
+        a.checkRequire(false);
+    }
+
+    function testRequireNoRevert() external {
+        AContract a = new AContract();
+        a.checkRequire(true);
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    // Assert 50% branch coverage if only revert tested.
+    cmd.arg("coverage")
+        .args(["--mt".to_string(), "testRequireRevert".to_string()])
+        .assert_success()
+        .stdout_eq(str![[r#"
+...
+| File              | % Lines       | % Statements  | % Branches   | % Funcs       |
+|-------------------|---------------|---------------|--------------|---------------|
+| src/AContract.sol | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
+| Total             | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
+
+"#]]);
+
+    // Assert 100% branch coverage.
+    cmd.forge_fuse().arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(
+        str![[r#"
+...
+| File              | % Lines       | % Statements  | % Branches    | % Funcs       |
+|-------------------|---------------|---------------|---------------|---------------|
+| src/AContract.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (2/2) | 100.00% (1/1) |
+| Total             | 100.00% (1/1) | 100.00% (1/1) | 100.00% (2/2) | 100.00% (1/1) |
+
+"#]],
+    );
+});
+
 forgetest!(test_line_hit_not_doubled, |prj, cmd| {
     prj.insert_ds_test();
     prj.add_source(