From 39c5922d14160d611b149b727cc8061badeb95a9 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Wed, 20 Nov 2024 19:01:23 +0530 Subject: [PATCH 01/20] fix(`coverage`): write coverage artifacts to separate dir --- crates/forge/bin/cmd/coverage.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 7e60a5451efc..a87305caabcf 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -93,6 +93,15 @@ impl CoverageArgs { fn build(&self, config: &Config) -> Result<(Project, ProjectCompileOutput)> { // Set up the project let mut project = config.create_project(false, false)?; + + // Set a different artifacts path for coverage. `out/coverage`. + // This is done to avoid overwriting the artifacts of the main build that maybe built with + // different optimizer settings or --via-ir. Optimizer settings are disabled for + // coverage builds. + let coverage_artifacts_path = project.artifacts_path().join("coverage"); + project.paths.artifacts = coverage_artifacts_path.clone(); + project.paths.build_infos = coverage_artifacts_path.join("build-info"); + if self.ir_minimum { // print warning message sh_warn!("{}", concat!( @@ -124,6 +133,8 @@ impl CoverageArgs { project.settings.solc.via_ir = None; } + sh_warn!("optimizer settings have been disabled for accurate coverage reports")?; + let output = ProjectCompiler::default() .compile(&project)? .with_stripped_file_prefixes(project.root()); From 20b09e8d0621d7ab2c433f4b6cae16bc96758cd4 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:09:18 +0530 Subject: [PATCH 02/20] adjust cache path for coverage --- crates/forge/bin/cmd/coverage.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index a87305caabcf..3eb1ae69de21 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -102,6 +102,18 @@ impl CoverageArgs { project.paths.artifacts = coverage_artifacts_path.clone(); project.paths.build_infos = coverage_artifacts_path.join("build-info"); + // Set a different compiler cache path for coverage. `cache/coverage`. + let cache_file = project + .paths + .cache + .components() + .last() + .ok_or_else(|| eyre::eyre!("Cache path is empty"))?; + + let cache_dir = + project.paths.cache.parent().ok_or_else(|| eyre::eyre!("Cache path is empty"))?; + project.paths.cache = cache_dir.join("coverage").join(cache_file); + if self.ir_minimum { // print warning message sh_warn!("{}", concat!( From 581cf4c3e78d73d006a79c971427d7c561d91a22 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:55:40 +0530 Subject: [PATCH 03/20] partial test --- crates/forge/tests/cli/coverage.rs | 67 ++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index 65900c592ba5..89f81c4808fd 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1447,3 +1447,70 @@ contract AContract { "#]]); }); + +forgetest!(test_diff_coverage_dir, |prj, cmd| { + prj.insert_ds_test(); + prj.add_source( + "AContract.sol", + r#" +contract AContract { + int public i; + + function init() public { + i = 0; + } + + function foo() public { + i = 1; + } +} + "#, + ) + .unwrap(); + + prj.add_source( + "AContractTest.sol", + r#" +import "./test.sol"; +import {AContract} from "./AContract.sol"; + +contract AContractTest is DSTest { + AContract a; + + function setUp() public { + a = new AContract(); + a.init(); + } + + function testFoo() public { + a.foo(); + } +} + "#, + ) + .unwrap(); + + // forge build + cmd.arg("build").assert_success(); + + // forge coverage + // Assert 100% coverage (init function coverage called in setUp is accounted). + 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% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | + +"#]], + ); + + // forge build - This should not compile the contracts again. + cmd.forge_fuse().arg("build").assert_success().stdout_eq( + r#"No files changed, compilation skipped +"#, + ); + + // TODO: forge coverage - Should not compile again. +}); From a0d7e1954bacc12f1417a2e9ddbad43214767aef Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:25:08 +0530 Subject: [PATCH 04/20] enable cache --- crates/forge/bin/cmd/coverage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 3eb1ae69de21..1aa15d029065 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -92,7 +92,7 @@ impl CoverageArgs { /// Builds the project. fn build(&self, config: &Config) -> Result<(Project, ProjectCompileOutput)> { // Set up the project - let mut project = config.create_project(false, false)?; + let mut project = config.create_project(config.cache, false)?; // Set a different artifacts path for coverage. `out/coverage`. // This is done to avoid overwriting the artifacts of the main build that maybe built with From c59364a061bb47f7fafe71b3f997ad0e718111c0 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Thu, 21 Nov 2024 22:32:59 +0530 Subject: [PATCH 05/20] don't assume recompilation --- crates/forge/bin/cmd/coverage.rs | 48 +++++++++++++++++------------- crates/forge/tests/cli/coverage.rs | 10 +++++++ 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 1aa15d029065..952a9278a90f 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -17,7 +17,7 @@ use foundry_cli::utils::{LoadConfig, STATIC_FUZZ_SEED}; use foundry_common::{compile::ProjectCompiler, fs}; use foundry_compilers::{ artifacts::{sourcemap::SourceMap, CompactBytecode, CompactDeployedBytecode, SolcLanguage}, - Artifact, ArtifactId, Project, ProjectCompileOutput, + output, Artifact, ArtifactId, Project, ProjectCompileOutput, }; use foundry_config::{Config, SolcReq}; use rayon::prelude::*; @@ -162,29 +162,35 @@ impl CoverageArgs { // Collect source files. let project_paths = &project.paths; let mut versioned_sources = HashMap::>::default(); - for (path, source_file, version) in output.output().sources.sources_with_version() { - report.add_source(version.clone(), source_file.id as usize, path.clone()); - // Filter out dependencies - if !self.include_libs && project_paths.has_library_ancestor(path) { - continue; - } + if !output.output().sources.is_empty() { + // Freshly compiled sources + for (path, source_file, version) in output.output().sources.sources_with_version() { + report.add_source(version.clone(), source_file.id as usize, path.clone()); + + // Filter out dependencies + if !self.include_libs && project_paths.has_library_ancestor(path) { + continue; + } - if let Some(ast) = &source_file.ast { - let file = project_paths.root.join(path); - trace!(root=?project_paths.root, ?file, "reading source file"); - - let source = SourceFile { - ast, - source: fs::read_to_string(&file) - .wrap_err("Could not read source code for analysis")?, - }; - versioned_sources - .entry(version.clone()) - .or_default() - .sources - .insert(source_file.id as usize, source); + if let Some(ast) = &source_file.ast { + let file = project_paths.root.join(path); + trace!(root=?project_paths.root, ?file, "reading source file"); + + let source = SourceFile { + ast, + source: fs::read_to_string(&file) + .wrap_err("Could not read source code for analysis")?, + }; + versioned_sources + .entry(version.clone()) + .or_default() + .sources + .insert(source_file.id as usize, source); + } } + } else { + // Cached sources } // Get source maps and bytecodes diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index 89f81c4808fd..f1c6983aaa03 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1513,4 +1513,14 @@ contract AContractTest is DSTest { ); // TODO: forge coverage - Should not compile again. + 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% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | + +"#]], + ); }); From 66cfb3d2d5f541270cd595f33048c4d1d353a9bc Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Thu, 21 Nov 2024 23:59:09 +0530 Subject: [PATCH 06/20] fix(`coverage`): dont recompile when cache exists --- crates/evm/coverage/src/analysis.rs | 4 +-- crates/forge/bin/cmd/coverage.rs | 38 +++++++++++++++++++++++++++-- crates/forge/tests/cli/coverage.rs | 8 +++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/crates/evm/coverage/src/analysis.rs b/crates/evm/coverage/src/analysis.rs index c18ba823b2d0..ea69b3113a9d 100644 --- a/crates/evm/coverage/src/analysis.rs +++ b/crates/evm/coverage/src/analysis.rs @@ -3,7 +3,7 @@ use alloy_primitives::map::HashMap; use foundry_common::TestFunctionExt; use foundry_compilers::artifacts::ast::{self, Ast, Node, NodeType}; use rayon::prelude::*; -use std::sync::Arc; +use std::{borrow::Cow, sync::Arc}; /// A visitor that walks the AST of a single contract and finds coverage items. #[derive(Clone, Debug)] @@ -592,5 +592,5 @@ pub struct SourceFile<'a> { /// The source code. pub source: String, /// The AST of the source code. - pub ast: &'a Ast, + pub ast: Cow<'a, Ast>, } diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 952a9278a90f..c8acf4fd10a6 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -17,12 +17,13 @@ use foundry_cli::utils::{LoadConfig, STATIC_FUZZ_SEED}; use foundry_common::{compile::ProjectCompiler, fs}; use foundry_compilers::{ artifacts::{sourcemap::SourceMap, CompactBytecode, CompactDeployedBytecode, SolcLanguage}, - output, Artifact, ArtifactId, Project, ProjectCompileOutput, + Artifact, ArtifactId, Project, ProjectCompileOutput, }; use foundry_config::{Config, SolcReq}; use rayon::prelude::*; use semver::Version; use std::{ + borrow::Cow, path::{Path, PathBuf}, sync::Arc, }; @@ -178,7 +179,7 @@ impl CoverageArgs { trace!(root=?project_paths.root, ?file, "reading source file"); let source = SourceFile { - ast, + ast: Cow::Borrowed(ast), source: fs::read_to_string(&file) .wrap_err("Could not read source code for analysis")?, }; @@ -191,6 +192,39 @@ impl CoverageArgs { } } else { // Cached sources + for (id, artifact) in output.artifact_ids() { + // Filter out dependencies + if !self.include_libs && project_paths.has_library_ancestor(&id.source) { + continue; + } + + let version = id.version; + let source_file = if let Some(source_file) = artifact.source_file() { + source_file + } else { + sh_warn!("ast source file not found for {}", id.source.display())?; + continue; + }; + + report.add_source(version.clone(), source_file.id as usize, id.source.clone()); + + if let Some(ast) = source_file.ast { + let file = project_paths.root.join(id.source); + trace!(root=?project_paths.root, ?file, "reading source file"); + + let source = SourceFile { + ast: Cow::Owned(ast), + source: fs::read_to_string(&file) + .wrap_err("Could not read source code for analysis")?, + }; + + versioned_sources + .entry(version.clone()) + .or_default() + .sources + .insert(source_file.id as usize, source); + } + } } // Get source maps and bytecodes diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index f1c6983aaa03..290e26b3a205 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1506,16 +1506,16 @@ contract AContractTest is DSTest { "#]], ); - // forge build - This should not compile the contracts again. + // forge build - Should not compile the contracts again. cmd.forge_fuse().arg("build").assert_success().stdout_eq( r#"No files changed, compilation skipped "#, ); - // TODO: forge coverage - Should not compile again. + // forge coverage - Should not compile the contracts again. cmd.forge_fuse().arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq( - str![[r#" - + str![[r#"No files changed, compilation skipped +... | File | % Lines | % Statements | % Branches | % Funcs | |-------------------|---------------|---------------|---------------|---------------| | src/AContract.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | From 0eadcfa1309ec9970a9176d7de16a1063e762364 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:47:15 +0530 Subject: [PATCH 07/20] account for cached compiled artifacts at once --- crates/forge/bin/cmd/coverage.rs | 81 ++++++++++-------------------- crates/forge/tests/cli/coverage.rs | 2 +- 2 files changed, 27 insertions(+), 56 deletions(-) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index c8acf4fd10a6..e8146528612f 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -159,71 +159,42 @@ impl CoverageArgs { #[instrument(name = "prepare", skip_all)] fn prepare(&self, project: &Project, output: &ProjectCompileOutput) -> Result { let mut report = CoverageReport::default(); - // Collect source files. let project_paths = &project.paths; let mut versioned_sources = HashMap::>::default(); - if !output.output().sources.is_empty() { - // Freshly compiled sources - for (path, source_file, version) in output.output().sources.sources_with_version() { - report.add_source(version.clone(), source_file.id as usize, path.clone()); - - // Filter out dependencies - if !self.include_libs && project_paths.has_library_ancestor(path) { - continue; - } - - if let Some(ast) = &source_file.ast { - let file = project_paths.root.join(path); - trace!(root=?project_paths.root, ?file, "reading source file"); - - let source = SourceFile { - ast: Cow::Borrowed(ast), - source: fs::read_to_string(&file) - .wrap_err("Could not read source code for analysis")?, - }; - versioned_sources - .entry(version.clone()) - .or_default() - .sources - .insert(source_file.id as usize, source); - } + // Account cached and freshly compiled sources + for (id, artifact) in output.artifact_ids() { + // Filter out dependencies + if !self.include_libs && project_paths.has_library_ancestor(&id.source) { + continue; } - } else { - // Cached sources - for (id, artifact) in output.artifact_ids() { - // Filter out dependencies - if !self.include_libs && project_paths.has_library_ancestor(&id.source) { - continue; - } - let version = id.version; - let source_file = if let Some(source_file) = artifact.source_file() { - source_file - } else { - sh_warn!("ast source file not found for {}", id.source.display())?; - continue; - }; + let version = id.version; + let source_file = if let Some(source_file) = artifact.source_file() { + source_file + } else { + sh_warn!("ast source file not found for {}", id.source.display())?; + continue; + }; - report.add_source(version.clone(), source_file.id as usize, id.source.clone()); + report.add_source(version.clone(), source_file.id as usize, id.source.clone()); - if let Some(ast) = source_file.ast { - let file = project_paths.root.join(id.source); - trace!(root=?project_paths.root, ?file, "reading source file"); + if let Some(ast) = source_file.ast { + let file = project_paths.root.join(id.source); + trace!(root=?project_paths.root, ?file, "reading source file"); - let source = SourceFile { - ast: Cow::Owned(ast), - source: fs::read_to_string(&file) - .wrap_err("Could not read source code for analysis")?, - }; + let source = SourceFile { + ast: Cow::Owned(ast), + source: fs::read_to_string(&file) + .wrap_err("Could not read source code for analysis")?, + }; - versioned_sources - .entry(version.clone()) - .or_default() - .sources - .insert(source_file.id as usize, source); - } + versioned_sources + .entry(version.clone()) + .or_default() + .sources + .insert(source_file.id as usize, source); } } diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index 290e26b3a205..fbc30c68e129 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1448,7 +1448,7 @@ contract AContract { "#]]); }); -forgetest!(test_diff_coverage_dir, |prj, cmd| { +forgetest!(test_coverage_caching, |prj, cmd| { prj.insert_ds_test(); prj.add_source( "AContract.sol", From 9902adf6db18bf27890419016a6cc3cc12e71bde Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:22:58 +0530 Subject: [PATCH 08/20] test with multi solc versions --- crates/forge/tests/cli/coverage.rs | 136 +++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index fbc30c68e129..b0e31a57f261 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1524,3 +1524,139 @@ contract AContractTest is DSTest { "#]], ); }); + +forgetest!(test_coverage_multi_solc_versions, |prj, cmd| { + prj.insert_ds_test(); + + let counter = r#" + pragma solidity 0.8.13; + contract Counter { + uint256 public number; + + function setNumber(uint256 newNumber) public { + number = newNumber; + } + + function increment() public { + number++; + } + } + "#; + let counter2 = r#" + pragma solidity 0.8.27; + contract Counter2 { + uint256 public number; + + function setNumber(uint256 newNumber) public { + number = newNumber; + } + + function increment() public { + number++; + } + } + "#; + + let counter_test = r#" + pragma solidity ^0.8.13; + import "./test.sol"; + import {Counter} from "./Counter.sol"; + + contract CounterTest is DSTest { + Counter public counter; + + function setUp() public { + counter = new Counter(); + counter.setNumber(0); + } + + function test_Increment() public { + counter.increment(); + assertEq(counter.number(), 1); + } + + function testFuzz_SetNumber(uint256 x) public { + counter.setNumber(x); + assertEq(counter.number(), x); + } + } + "#; + + let counter2_test = r#" + pragma solidity ^0.8.13; + import "./test.sol"; + import {Counter2} from "./Counter2.sol"; + + contract Counter2Test is DSTest { + Counter2 public counter; + + function setUp() public { + counter = new Counter2(); + counter.setNumber(0); + } + + function test_Increment() public { + counter.increment(); + assertEq(counter.number(), 1); + } + + function testFuzz_SetNumber(uint256 x) public { + counter.setNumber(x); + assertEq(counter.number(), x); + } + } + "#; + + prj.add_source("Counter.sol", counter).unwrap(); + prj.add_source("Counter2.sol", counter2).unwrap(); + prj.add_source("CounterTest.sol", counter_test).unwrap(); + prj.add_source("Counter2Test.sol", counter2_test).unwrap(); + + // no-cache + cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[ + r#"[COMPILING_FILES] with [SOLC_VERSION] +[COMPILING_FILES] with [SOLC_VERSION] +[SOLC_VERSION] [ELAPSED] +[SOLC_VERSION] [ELAPSED] +... +| File | % Lines | % Statements | % Branches | % Funcs | +|------------------|---------------|---------------|---------------|---------------| +| src/Counter.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| src/Counter2.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| Total | 100.00% (4/4) | 100.00% (4/4) | 100.00% (0/0) | 100.00% (4/4) | + +"# + ]]); + + // cache + cmd.forge_fuse().arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq( + str![[r#"No files changed, compilation skipped +... +| File | % Lines | % Statements | % Branches | % Funcs | +|------------------|---------------|---------------|---------------|---------------| +| src/Counter.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| src/Counter2.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| Total | 100.00% (4/4) | 100.00% (4/4) | 100.00% (0/0) | 100.00% (4/4) | + +"#]], + ); + + // Replace solc version in Counter2.sol + let counter2 = counter2.replace("0.8.27", "0.8.25"); + + prj.add_source("Counter2.sol", &counter2).unwrap(); + + // Should recompile Counter2.sol + cmd.forge_fuse().arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq( + str![[r#"[COMPILING_FILES] with [SOLC_VERSION] +[SOLC_VERSION] [ELAPSED] +... +| File | % Lines | % Statements | % Branches | % Funcs | +|------------------|---------------|---------------|---------------|---------------| +| src/Counter.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| src/Counter2.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | +| Total | 100.00% (4/4) | 100.00% (4/4) | 100.00% (0/0) | 100.00% (4/4) | + +"#]], + ); +}); From 0433863cc1689cb68be7ae54c3d8bb96fe84801d Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:31:28 +0530 Subject: [PATCH 09/20] fix(`forge clean`): remove cache/coverage dir --- crates/config/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 6444802e3bc9..333ebfa87957 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -985,6 +985,8 @@ impl Config { }; remove_test_dir(&self.fuzz.failure_persist_dir); remove_test_dir(&self.invariant.failure_persist_dir); + remove_test_dir(&Some(self.cache_path.clone().join("coverage"))); + remove_test_dir(&Some(self.cache_path.clone())); // Remove snapshot directory. let snapshot_dir = project.root().join(&self.snapshots); From e90a354541c42935d7b28bfce4edd20ca9d391e7 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 22 Nov 2024 15:28:21 +0400 Subject: [PATCH 10/20] add test --- crates/forge/tests/cli/coverage.rs | 62 ++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index b0e31a57f261..a8d722e0728c 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1660,3 +1660,65 @@ forgetest!(test_coverage_multi_solc_versions, |prj, cmd| { "#]], ); }); + +// checks that `clean` also works with the "out" value set in Config +forgetest!(coverage_cache, |prj, cmd| { + prj.add_source("A", r#" +contract A { + function f() public pure returns (uint) { + return 1; + } +}"#).unwrap(); + + prj.add_source("B", r#" +contract B { + function f() public pure returns (uint) { + return 1; + } +}"#).unwrap(); + + let a_test = prj.add_test("A.t.sol", r#" +import {A} from "../src/A.sol"; + +contract ATest { + function test() public { + A a = new A(); + a.f(); + } +} + "#).unwrap(); + + prj.add_test("B.t.sol", r#" + import {B} from "../src/B.sol"; + + contract BTest { + function test() public { + B a = new B(); + a.f(); + } + } + "#).unwrap(); + + cmd.forge_fuse().arg("coverage").assert_success().stdout_eq(str![[r#" +... +Ran 2 test suites [ELAPSED]: 2 tests passed, 0 failed, 0 skipped (2 total tests) +| File | % Lines | % Statements | % Branches | % Funcs | +|-----------|---------------|---------------|---------------|---------------| +| src/A.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | +| src/B.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | +| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | + +"#]]); + + prj.add_test("A.t.sol", &format!("{} ", std::fs::read_to_string(a_test).unwrap())).unwrap(); + + cmd.forge_fuse().arg("coverage").assert_success().stdout_eq(str![[r#" +... +Ran 2 test suites [ELAPSED]: 2 tests passed, 0 failed, 0 skipped (2 total tests) +| File | % Lines | % Statements | % Branches | % Funcs | +|-----------|---------------|---------------|---------------|---------------| +| src/A.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | +| Total | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | + +"#]]); +}); From 7d639e6a05b856f59ad040379e76deb675e6e735 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Wed, 27 Nov 2024 16:08:01 +0530 Subject: [PATCH 11/20] fix(`coverage`): account for `build_id` in source identification (#9418) * fix: use `SourceIdentifier` in SourceFiles mapping * fix(`coverage`): use `SourceIdentifier` in analysis and report. * fix * nit * nit --- crates/evm/coverage/src/analysis.rs | 37 ++++++++++++++++++++----- crates/evm/coverage/src/anchors.rs | 10 ++++--- crates/evm/coverage/src/lib.rs | 23 ++++++++-------- crates/forge/bin/cmd/coverage.rs | 35 ++++++++++++++++-------- crates/forge/src/coverage.rs | 6 +++-- crates/forge/tests/cli/coverage.rs | 42 +++++++++++++++++++++-------- 6 files changed, 109 insertions(+), 44 deletions(-) diff --git a/crates/evm/coverage/src/analysis.rs b/crates/evm/coverage/src/analysis.rs index ea69b3113a9d..9f1977dee51d 100644 --- a/crates/evm/coverage/src/analysis.rs +++ b/crates/evm/coverage/src/analysis.rs @@ -1,5 +1,6 @@ use super::{CoverageItem, CoverageItemKind, SourceLocation}; use alloy_primitives::map::HashMap; +use core::fmt; use foundry_common::TestFunctionExt; use foundry_compilers::artifacts::ast::{self, Ast, Node, NodeType}; use rayon::prelude::*; @@ -9,7 +10,7 @@ use std::{borrow::Cow, sync::Arc}; #[derive(Clone, Debug)] pub struct ContractVisitor<'a> { /// The source ID of the contract. - source_id: usize, + source_id: SourceIdentifier, /// The source code that contains the AST being walked. source: &'a str, @@ -26,7 +27,7 @@ pub struct ContractVisitor<'a> { } impl<'a> ContractVisitor<'a> { - pub fn new(source_id: usize, source: &'a str, contract_name: &'a Arc) -> Self { + pub fn new(source_id: SourceIdentifier, source: &'a str, contract_name: &'a Arc) -> Self { Self { source_id, source, contract_name, branch_id: 0, last_line: 0, items: Vec::new() } } @@ -473,7 +474,7 @@ impl<'a> ContractVisitor<'a> { let loc_start = self.source.char_indices().map(|(i, _)| i).nth(loc.start).unwrap_or_default(); SourceLocation { - source_id: self.source_id, + source_id: self.source_id.clone(), contract_name: self.contract_name.clone(), start: loc.start as u32, length: loc.length.map(|x| x as u32), @@ -538,7 +539,7 @@ impl<'a> SourceAnalyzer<'a> { .sources .sources .par_iter() - .flat_map_iter(|(&source_id, SourceFile { source, ast })| { + .flat_map_iter(|(source_id, SourceFile { source, ast })| { ast.nodes.iter().map(move |node| { if !matches!(node.node_type, NodeType::ContractDefinition) { return Ok(vec![]); @@ -556,7 +557,7 @@ impl<'a> SourceAnalyzer<'a> { .attribute("name") .ok_or_else(|| eyre::eyre!("Contract has no name"))?; - let mut visitor = ContractVisitor::new(source_id, source, &name); + let mut visitor = ContractVisitor::new(source_id.clone(), source, &name); visitor.visit_contract(node)?; let mut items = visitor.items; @@ -583,7 +584,31 @@ impl<'a> SourceAnalyzer<'a> { #[derive(Debug, Default)] pub struct SourceFiles<'a> { /// The versioned sources. - pub sources: HashMap>, + /// Keyed by build_id and source_id. + pub sources: HashMap>, +} + +/// Serves as a unique identifier for sources across multiple compiler runs. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SourceIdentifier { + /// Source ID is unique for each source file per compilation job but may not be across + /// different jobs. + pub source_id: usize, + /// Artifact build id is same for all sources in a single compilation job. But always unique + /// across different jobs. + pub build_id: String, +} + +impl fmt::Display for SourceIdentifier { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "source_id={} build_id={}", self.source_id, self.build_id,) + } +} + +impl SourceIdentifier { + pub fn new(source_id: usize, build_id: String) -> Self { + Self { source_id, build_id } + } } /// The source code and AST of a file. diff --git a/crates/evm/coverage/src/anchors.rs b/crates/evm/coverage/src/anchors.rs index 6643524d61de..4d5e9e45e8f4 100644 --- a/crates/evm/coverage/src/anchors.rs +++ b/crates/evm/coverage/src/anchors.rs @@ -1,3 +1,5 @@ +use crate::analysis::SourceIdentifier; + use super::{CoverageItem, CoverageItemKind, ItemAnchor, SourceLocation}; use alloy_primitives::map::{DefaultHashBuilder, HashMap, HashSet}; use eyre::ensure; @@ -11,12 +13,13 @@ pub fn find_anchors( source_map: &SourceMap, ic_pc_map: &IcPcMap, items: &[CoverageItem], - items_by_source_id: &HashMap>, + items_by_source_id: &HashMap>, + source_id: &SourceIdentifier, ) -> Vec { let mut seen = HashSet::with_hasher(DefaultHashBuilder::default()); source_map .iter() - .filter_map(|element| items_by_source_id.get(&(element.index()? as usize))) + .filter_map(|_element| items_by_source_id.get(source_id)) .flatten() .filter_map(|&item_id| { if !seen.insert(item_id) { @@ -171,7 +174,8 @@ pub fn find_anchor_branch( /// Calculates whether `element` is within the range of the target `location`. fn is_in_source_range(element: &SourceElement, location: &SourceLocation) -> bool { // Source IDs must match. - let source_ids_match = element.index().is_some_and(|a| a as usize == location.source_id); + let source_ids_match = + element.index().is_some_and(|a| a as usize == location.source_id.source_id); if !source_ids_match { return false; } diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index ad4ab53e3cf1..481cc0bb1f96 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -12,6 +12,7 @@ extern crate foundry_common; extern crate tracing; use alloy_primitives::{map::HashMap, Bytes, B256}; +use analysis::SourceIdentifier; use eyre::{Context, Result}; use foundry_compilers::artifacts::sourcemap::SourceMap; use semver::Version; @@ -36,9 +37,9 @@ pub use inspector::CoverageCollector; #[derive(Clone, Debug, Default)] pub struct CoverageReport { /// A map of source IDs to the source path. - pub source_paths: HashMap<(Version, usize), PathBuf>, + pub source_paths: HashMap<(Version, SourceIdentifier), PathBuf>, /// A map of source paths to source IDs. - pub source_paths_to_ids: HashMap<(Version, PathBuf), usize>, + pub source_paths_to_ids: HashMap<(Version, PathBuf), SourceIdentifier>, /// All coverage items for the codebase, keyed by the compiler version. pub items: HashMap>, /// All item anchors for the codebase, keyed by their contract ID. @@ -51,14 +52,14 @@ pub struct CoverageReport { impl CoverageReport { /// Add a source file path. - pub fn add_source(&mut self, version: Version, source_id: usize, path: PathBuf) { - self.source_paths.insert((version.clone(), source_id), path.clone()); + pub fn add_source(&mut self, version: Version, source_id: SourceIdentifier, path: PathBuf) { + self.source_paths.insert((version.clone(), source_id.clone()), path.clone()); self.source_paths_to_ids.insert((version, path), source_id); } /// Get the source ID for a specific source file path. - pub fn get_source_id(&self, version: Version, path: PathBuf) -> Option { - self.source_paths_to_ids.get(&(version, path)).copied() + pub fn get_source_id(&self, version: Version, path: PathBuf) -> Option { + self.source_paths_to_ids.get(&(version, path)).cloned() } /// Add the source maps. @@ -89,7 +90,7 @@ impl CoverageReport { for (version, items) in self.items.iter() { for item in items { let Some(path) = - self.source_paths.get(&(version.clone(), item.loc.source_id)).cloned() + self.source_paths.get(&(version.clone(), item.loc.source_id.clone())).cloned() else { continue; }; @@ -107,7 +108,7 @@ impl CoverageReport { for (version, items) in self.items.iter() { for item in items { let Some(path) = - self.source_paths.get(&(version.clone(), item.loc.source_id)).cloned() + self.source_paths.get(&(version.clone(), item.loc.source_id.clone())).cloned() else { continue; }; @@ -160,7 +161,7 @@ impl CoverageReport { self.items.retain(|version, items| { items.retain(|item| { self.source_paths - .get(&(version.clone(), item.loc.source_id)) + .get(&(version.clone(), item.loc.source_id.clone())) .map(|path| filter(path)) .unwrap_or(false) }); @@ -259,7 +260,7 @@ impl HitMap { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ContractId { pub version: Version, - pub source_id: usize, + pub source_id: SourceIdentifier, pub contract_name: Arc, } @@ -348,7 +349,7 @@ impl Display for CoverageItem { #[derive(Clone, Debug)] pub struct SourceLocation { /// The source ID. - pub source_id: usize, + pub source_id: SourceIdentifier, /// The contract this source range is in. pub contract_name: Arc, /// Start byte in the source code. diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index e8146528612f..2dceea991512 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -4,7 +4,7 @@ use clap::{Parser, ValueEnum, ValueHint}; use eyre::{Context, Result}; use forge::{ coverage::{ - analysis::{SourceAnalysis, SourceAnalyzer, SourceFile, SourceFiles}, + analysis::{SourceAnalysis, SourceAnalyzer, SourceFile, SourceFiles, SourceIdentifier}, anchors::find_anchors, BytecodeReporter, ContractId, CoverageReport, CoverageReporter, DebugReporter, ItemAnchor, LcovReporter, SummaryReporter, @@ -162,7 +162,6 @@ impl CoverageArgs { // Collect source files. let project_paths = &project.paths; let mut versioned_sources = HashMap::>::default(); - // Account cached and freshly compiled sources for (id, artifact) in output.artifact_ids() { // Filter out dependencies @@ -171,6 +170,7 @@ impl CoverageArgs { } let version = id.version; + let build_id = id.build_id; let source_file = if let Some(source_file) = artifact.source_file() { source_file } else { @@ -178,7 +178,8 @@ impl CoverageArgs { continue; }; - report.add_source(version.clone(), source_file.id as usize, id.source.clone()); + let identifier = SourceIdentifier::new(source_file.id as usize, build_id.clone()); + report.add_source(version.clone(), identifier.clone(), id.source.clone()); if let Some(ast) = source_file.ast { let file = project_paths.root.join(id.source); @@ -194,7 +195,7 @@ impl CoverageArgs { .entry(version.clone()) .or_default() .sources - .insert(source_file.id as usize, source); + .insert(identifier, source); } } @@ -219,17 +220,23 @@ impl CoverageArgs { ); for (item_id, item) in source_analysis.items.iter().enumerate() { - items_by_source_id.entry(item.loc.source_id).or_default().push(item_id); + items_by_source_id.entry(item.loc.source_id.clone()).or_default().push(item_id); } let anchors = artifacts .par_iter() .filter(|artifact| artifact.contract_id.version == *version) .map(|artifact| { - let creation_code_anchors = - artifact.creation.find_anchors(&source_analysis, &items_by_source_id); - let deployed_code_anchors = - artifact.deployed.find_anchors(&source_analysis, &items_by_source_id); + let creation_code_anchors = artifact.creation.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); + let deployed_code_anchors = artifact.deployed.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); (artifact.contract_id.clone(), (creation_code_anchors, deployed_code_anchors)) }) .collect::>(); @@ -387,7 +394,11 @@ pub struct ArtifactData { } impl ArtifactData { - pub fn new(id: &ArtifactId, source_id: usize, artifact: &impl Artifact) -> Option { + pub fn new( + id: &ArtifactId, + source_id: SourceIdentifier, + artifact: &impl Artifact, + ) -> Option { Some(Self { contract_id: ContractId { version: id.version.clone(), @@ -432,7 +443,8 @@ impl BytecodeData { pub fn find_anchors( &self, source_analysis: &SourceAnalysis, - items_by_source_id: &HashMap>, + items_by_source_id: &HashMap>, + source_id: &SourceIdentifier, ) -> Vec { find_anchors( &self.bytecode, @@ -440,6 +452,7 @@ impl BytecodeData { &self.ic_pc_map, &source_analysis.items, items_by_source_id, + source_id, ) } } diff --git a/crates/forge/src/coverage.rs b/crates/forge/src/coverage.rs index de8d0a8aa853..80a14119685b 100644 --- a/crates/forge/src/coverage.rs +++ b/crates/forge/src/coverage.rs @@ -222,8 +222,10 @@ impl CoverageReporter for BytecodeReporter { .map(|h| format!("[{h:03}]")) .unwrap_or(" ".to_owned()); let source_id = source_element.index(); - let source_path = source_id.and_then(|i| { - report.source_paths.get(&(contract_id.version.clone(), i as usize)) + let source_path = source_id.and_then(|_i| { + report + .source_paths + .get(&(contract_id.version.clone(), contract_id.source_id.clone())) }); let code = format!("{code:?}"); diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs index a8d722e0728c..67a0dd4d1304 100644 --- a/crates/forge/tests/cli/coverage.rs +++ b/crates/forge/tests/cli/coverage.rs @@ -1662,22 +1662,35 @@ forgetest!(test_coverage_multi_solc_versions, |prj, cmd| { }); // checks that `clean` also works with the "out" value set in Config -forgetest!(coverage_cache, |prj, cmd| { - prj.add_source("A", r#" +// this test verifies that the coverage is preserved across compiler runs that may result in files +// with different source_id's +forgetest!(coverage_cache_across_compiler_runs, |prj, cmd| { + prj.add_source( + "A", + r#" contract A { function f() public pure returns (uint) { return 1; } -}"#).unwrap(); +}"#, + ) + .unwrap(); - prj.add_source("B", r#" + prj.add_source( + "B", + r#" contract B { function f() public pure returns (uint) { return 1; } -}"#).unwrap(); +}"#, + ) + .unwrap(); - let a_test = prj.add_test("A.t.sol", r#" + let a_test = prj + .add_test( + "A.t.sol", + r#" import {A} from "../src/A.sol"; contract ATest { @@ -1686,9 +1699,13 @@ contract ATest { a.f(); } } - "#).unwrap(); + "#, + ) + .unwrap(); - prj.add_test("B.t.sol", r#" + prj.add_test( + "B.t.sol", + r#" import {B} from "../src/B.sol"; contract BTest { @@ -1697,8 +1714,10 @@ contract ATest { a.f(); } } - "#).unwrap(); - + "#, + ) + .unwrap(); + cmd.forge_fuse().arg("coverage").assert_success().stdout_eq(str![[r#" ... Ran 2 test suites [ELAPSED]: 2 tests passed, 0 failed, 0 skipped (2 total tests) @@ -1718,7 +1737,8 @@ Ran 2 test suites [ELAPSED]: 2 tests passed, 0 failed, 0 skipped (2 total tests) | File | % Lines | % Statements | % Branches | % Funcs | |-----------|---------------|---------------|---------------|---------------| | src/A.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | -| Total | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | +| src/B.sol | 100.00% (1/1) | 100.00% (1/1) | 100.00% (0/0) | 100.00% (1/1) | +| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) | "#]]); }); From 2b1af191558d8b34b8625ab3276538ccbcdd16b3 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:28:22 +0530 Subject: [PATCH 12/20] fix --- crates/evm/coverage/src/anchors.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/evm/coverage/src/anchors.rs b/crates/evm/coverage/src/anchors.rs index 4d5e9e45e8f4..0c004226a61b 100644 --- a/crates/evm/coverage/src/anchors.rs +++ b/crates/evm/coverage/src/anchors.rs @@ -19,7 +19,10 @@ pub fn find_anchors( let mut seen = HashSet::with_hasher(DefaultHashBuilder::default()); source_map .iter() - .filter_map(|_element| items_by_source_id.get(source_id)) + .filter_map(|element| { + items_by_source_id + .get(&SourceIdentifier::new(element.index()? as usize, source_id.build_id.clone())) + }) .flatten() .filter_map(|&item_id| { if !seen.insert(item_id) { From 498e42784877514322a9d9d0a57f842aa816679a Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:34:44 +0530 Subject: [PATCH 13/20] rm version as key in report --- crates/evm/coverage/src/lib.rs | 27 ++++++++++----------------- crates/forge/bin/cmd/coverage.rs | 8 +++----- crates/forge/src/coverage.rs | 2 +- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index 481cc0bb1f96..b1edb9fb5951 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -37,9 +37,9 @@ pub use inspector::CoverageCollector; #[derive(Clone, Debug, Default)] pub struct CoverageReport { /// A map of source IDs to the source path. - pub source_paths: HashMap<(Version, SourceIdentifier), PathBuf>, + pub source_paths: HashMap, /// A map of source paths to source IDs. - pub source_paths_to_ids: HashMap<(Version, PathBuf), SourceIdentifier>, + pub source_paths_to_ids: HashMap, /// All coverage items for the codebase, keyed by the compiler version. pub items: HashMap>, /// All item anchors for the codebase, keyed by their contract ID. @@ -52,14 +52,14 @@ pub struct CoverageReport { impl CoverageReport { /// Add a source file path. - pub fn add_source(&mut self, version: Version, source_id: SourceIdentifier, path: PathBuf) { - self.source_paths.insert((version.clone(), source_id.clone()), path.clone()); - self.source_paths_to_ids.insert((version, path), source_id); + pub fn add_source(&mut self, source_id: SourceIdentifier, path: PathBuf) { + self.source_paths.insert(source_id.clone(), path.clone()); + self.source_paths_to_ids.insert(path, source_id); } /// Get the source ID for a specific source file path. - pub fn get_source_id(&self, version: Version, path: PathBuf) -> Option { - self.source_paths_to_ids.get(&(version, path)).cloned() + pub fn get_source_id(&self, path: PathBuf) -> Option { + self.source_paths_to_ids.get(&path).cloned() } /// Add the source maps. @@ -89,9 +89,7 @@ impl CoverageReport { for (version, items) in self.items.iter() { for item in items { - let Some(path) = - self.source_paths.get(&(version.clone(), item.loc.source_id.clone())).cloned() - else { + let Some(path) = self.source_paths.get(&item.loc.source_id).cloned() else { continue; }; *summaries.entry(path).or_default() += item; @@ -107,9 +105,7 @@ impl CoverageReport { for (version, items) in self.items.iter() { for item in items { - let Some(path) = - self.source_paths.get(&(version.clone(), item.loc.source_id.clone())).cloned() - else { + let Some(path) = self.source_paths.get(&item.loc.source_id).cloned() else { continue; }; items_by_source.entry(path).or_default().push(item.clone()); @@ -160,10 +156,7 @@ impl CoverageReport { pub fn filter_out_ignored_sources(&mut self, filter: impl Fn(&Path) -> bool) { self.items.retain(|version, items| { items.retain(|item| { - self.source_paths - .get(&(version.clone(), item.loc.source_id.clone())) - .map(|path| filter(path)) - .unwrap_or(false) + self.source_paths.get(&item.loc.source_id).map(|path| filter(path)).unwrap_or(false) }); !items.is_empty() }); diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 2dceea991512..5063edccec31 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -179,7 +179,7 @@ impl CoverageArgs { }; let identifier = SourceIdentifier::new(source_file.id as usize, build_id.clone()); - report.add_source(version.clone(), identifier.clone(), id.source.clone()); + report.add_source(identifier.clone(), id.source.clone()); if let Some(ast) = source_file.ast { let file = project_paths.root.join(id.source); @@ -204,7 +204,7 @@ impl CoverageArgs { .artifact_ids() .par_bridge() .filter_map(|(id, artifact)| { - let source_id = report.get_source_id(id.version.clone(), id.source.clone())?; + let source_id = report.get_source_id(id.source.clone())?; ArtifactData::new(&id, source_id, artifact) }) .collect(); @@ -306,9 +306,7 @@ impl CoverageArgs { }); for (artifact_id, map, is_deployed_code) in data { - if let Some(source_id) = - report.get_source_id(artifact_id.version.clone(), artifact_id.source.clone()) - { + if let Some(source_id) = report.get_source_id(artifact_id.source.clone()) { report.add_hit_map( &ContractId { version: artifact_id.version.clone(), diff --git a/crates/forge/src/coverage.rs b/crates/forge/src/coverage.rs index 80a14119685b..1762c4baa5ca 100644 --- a/crates/forge/src/coverage.rs +++ b/crates/forge/src/coverage.rs @@ -225,7 +225,7 @@ impl CoverageReporter for BytecodeReporter { let source_path = source_id.and_then(|_i| { report .source_paths - .get(&(contract_id.version.clone(), contract_id.source_id.clone())) + .get(&contract_id.source_id) }); let code = format!("{code:?}"); From 06f01dcc52843ad3eacc0b8ee39fc2b41fb9441a Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:51:32 +0530 Subject: [PATCH 14/20] rm version key from ContractId and report.items --- crates/evm/coverage/src/lib.rs | 18 +++++++----------- crates/forge/bin/cmd/coverage.rs | 16 ++++------------ crates/forge/src/coverage.rs | 2 +- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index b1edb9fb5951..e7c1e2cfce51 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -15,7 +15,6 @@ use alloy_primitives::{map::HashMap, Bytes, B256}; use analysis::SourceIdentifier; use eyre::{Context, Result}; use foundry_compilers::artifacts::sourcemap::SourceMap; -use semver::Version; use std::{ collections::BTreeMap, fmt::Display, @@ -41,7 +40,7 @@ pub struct CoverageReport { /// A map of source paths to source IDs. pub source_paths_to_ids: HashMap, /// All coverage items for the codebase, keyed by the compiler version. - pub items: HashMap>, + pub items: HashMap>, /// All item anchors for the codebase, keyed by their contract ID. pub anchors: HashMap, Vec)>, /// All the bytecode hits for the codebase. @@ -71,8 +70,10 @@ impl CoverageReport { } /// Add coverage items to this report. - pub fn add_items(&mut self, version: Version, items: impl IntoIterator) { - self.items.entry(version).or_default().extend(items); + pub fn add_items(&mut self, items: impl IntoIterator) { + for item in items.into_iter() { + self.items.entry(item.loc.source_id.clone()).or_default().push(item); + } } /// Add anchors to this report. @@ -139,7 +140,7 @@ impl CoverageReport { for anchor in anchors { if let Some(&hits) = hit_map.hits.get(&anchor.instruction) { self.items - .get_mut(&contract_id.version) + .get_mut(&contract_id.source_id) .and_then(|items| items.get_mut(anchor.item_id)) .expect("Anchor refers to non-existent coverage item") .hits += hits; @@ -252,18 +253,13 @@ impl HitMap { /// A unique identifier for a contract #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ContractId { - pub version: Version, pub source_id: SourceIdentifier, pub contract_name: Arc, } impl Display for ContractId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "Contract \"{}\" (solc {}, source ID {})", - self.contract_name, self.version, self.source_id - ) + write!(f, "Contract \"{}\" source ID {}", self.contract_name, self.source_id) } } diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 5063edccec31..411fdb8b8276 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -225,7 +225,7 @@ impl CoverageArgs { let anchors = artifacts .par_iter() - .filter(|artifact| artifact.contract_id.version == *version) + .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) .map(|artifact| { let creation_code_anchors = artifact.creation.find_anchors( &source_analysis, @@ -242,7 +242,7 @@ impl CoverageArgs { .collect::>(); report.add_anchors(anchors); - report.add_items(version.clone(), source_analysis.items); + report.add_items(source_analysis.items); } report.add_source_maps(artifacts.into_iter().map(|artifact| { @@ -308,11 +308,7 @@ impl CoverageArgs { for (artifact_id, map, is_deployed_code) in data { if let Some(source_id) = report.get_source_id(artifact_id.source.clone()) { report.add_hit_map( - &ContractId { - version: artifact_id.version.clone(), - source_id, - contract_name: artifact_id.name.as_str().into(), - }, + &ContractId { source_id, contract_name: artifact_id.name.as_str().into() }, map, is_deployed_code, )?; @@ -398,11 +394,7 @@ impl ArtifactData { artifact: &impl Artifact, ) -> Option { Some(Self { - contract_id: ContractId { - version: id.version.clone(), - source_id, - contract_name: id.name.as_str().into(), - }, + contract_id: ContractId { source_id, contract_name: id.name.as_str().into() }, creation: BytecodeData::new( artifact.get_source_map()?.ok()?, artifact diff --git a/crates/forge/src/coverage.rs b/crates/forge/src/coverage.rs index 1762c4baa5ca..061f9f517157 100644 --- a/crates/forge/src/coverage.rs +++ b/crates/forge/src/coverage.rs @@ -178,7 +178,7 @@ impl CoverageReporter for DebugReporter { " - Refers to item: {}", report .items - .get(&contract_id.version) + .get(&contract_id.source_id) .and_then(|items| items.get(anchor.item_id)) .map_or("None".to_owned(), |item| item.to_string()) ); From 38db6898ba360ac859c7b7f829e15e3ae244e734 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 17:59:52 +0530 Subject: [PATCH 15/20] fix: scale anchor.items_id --- crates/evm/coverage/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index e7c1e2cfce51..99e2f7aef6b3 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -47,6 +47,8 @@ pub struct CoverageReport { pub bytecode_hits: HashMap, /// The bytecode -> source mappings. pub source_maps: HashMap, + /// Anchor Item ID by source ID + pub item_ids_by_source_id: HashMap>, } impl CoverageReport { @@ -141,7 +143,11 @@ impl CoverageReport { if let Some(&hits) = hit_map.hits.get(&anchor.instruction) { self.items .get_mut(&contract_id.source_id) - .and_then(|items| items.get_mut(anchor.item_id)) + .and_then(|items| { + let scaled_item_id = anchor.item_id % items.len(); + + items.get_mut(scaled_item_id) + }) .expect("Anchor refers to non-existent coverage item") .hits += hits; } From a6edce9a48403305eecb71e56e7ee9913898d517 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 18:15:27 +0530 Subject: [PATCH 16/20] fix: rm dependence of coverage on version --- Cargo.lock | 1 - crates/evm/coverage/Cargo.toml | 1 - crates/evm/coverage/src/lib.rs | 14 +++---- crates/forge/bin/cmd/coverage.rs | 70 +++++++++++++++----------------- 4 files changed, 39 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e6f33e224c89..28e74de3b0ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4022,7 +4022,6 @@ dependencies = [ "foundry-evm-core", "rayon", "revm", - "semver 1.0.23", "tracing", ] diff --git a/crates/evm/coverage/Cargo.toml b/crates/evm/coverage/Cargo.toml index e38d33e2a590..a371dd08edc4 100644 --- a/crates/evm/coverage/Cargo.toml +++ b/crates/evm/coverage/Cargo.toml @@ -21,6 +21,5 @@ foundry-evm-core.workspace = true alloy-primitives.workspace = true eyre.workspace = true revm.workspace = true -semver.workspace = true tracing.workspace = true rayon.workspace = true diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index 99e2f7aef6b3..d5e32c6af051 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -90,9 +90,9 @@ impl CoverageReport { pub fn summary_by_file(&self) -> impl Iterator { let mut summaries = BTreeMap::new(); - for (version, items) in self.items.iter() { + for (source_id, items) in self.items.iter() { for item in items { - let Some(path) = self.source_paths.get(&item.loc.source_id).cloned() else { + let Some(path) = self.source_paths.get(source_id).cloned() else { continue; }; *summaries.entry(path).or_default() += item; @@ -106,9 +106,9 @@ impl CoverageReport { pub fn items_by_source(&self) -> impl Iterator)> { let mut items_by_source: BTreeMap<_, Vec<_>> = BTreeMap::new(); - for (version, items) in self.items.iter() { + for (source_id, items) in self.items.iter() { for item in items { - let Some(path) = self.source_paths.get(&item.loc.source_id).cloned() else { + let Some(path) = self.source_paths.get(source_id).cloned() else { continue; }; items_by_source.entry(path).or_default().push(item.clone()); @@ -161,9 +161,9 @@ impl CoverageReport { /// This function should only be called after all the sources were used, otherwise, the output /// will be missing the ones that are dependent on them. pub fn filter_out_ignored_sources(&mut self, filter: impl Fn(&Path) -> bool) { - self.items.retain(|version, items| { - items.retain(|item| { - self.source_paths.get(&item.loc.source_id).map(|path| filter(path)).unwrap_or(false) + self.items.retain(|source_id, items| { + items.retain(|_item| { + self.source_paths.get(source_id).map(|path| filter(path)).unwrap_or(false) }); !items.is_empty() }); diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 411fdb8b8276..d6ff588c55f1 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -161,7 +161,8 @@ impl CoverageArgs { let mut report = CoverageReport::default(); // Collect source files. let project_paths = &project.paths; - let mut versioned_sources = HashMap::>::default(); + + let mut sources = SourceFiles::default(); // Account cached and freshly compiled sources for (id, artifact) in output.artifact_ids() { // Filter out dependencies @@ -169,7 +170,6 @@ impl CoverageArgs { continue; } - let version = id.version; let build_id = id.build_id; let source_file = if let Some(source_file) = artifact.source_file() { source_file @@ -191,11 +191,7 @@ impl CoverageArgs { .wrap_err("Could not read source code for analysis")?, }; - versioned_sources - .entry(version.clone()) - .or_default() - .sources - .insert(identifier, source); + sources.sources.entry(identifier).or_insert(source); } } @@ -210,41 +206,39 @@ impl CoverageArgs { .collect(); // Add coverage items - for (version, sources) in &versioned_sources { - let source_analysis = SourceAnalyzer::new(sources).analyze()?; - - // Build helper mapping used by `find_anchors` - let mut items_by_source_id = HashMap::<_, Vec<_>>::with_capacity_and_hasher( - source_analysis.items.len(), - Default::default(), - ); + let source_analysis = SourceAnalyzer::new(&sources).analyze()?; - for (item_id, item) in source_analysis.items.iter().enumerate() { - items_by_source_id.entry(item.loc.source_id.clone()).or_default().push(item_id); - } + // Build helper mapping used by `find_anchors` + let mut items_by_source_id = HashMap::<_, Vec<_>>::with_capacity_and_hasher( + source_analysis.items.len(), + Default::default(), + ); - let anchors = artifacts - .par_iter() - .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) - .map(|artifact| { - let creation_code_anchors = artifact.creation.find_anchors( - &source_analysis, - &items_by_source_id, - &artifact.contract_id.source_id, - ); - let deployed_code_anchors = artifact.deployed.find_anchors( - &source_analysis, - &items_by_source_id, - &artifact.contract_id.source_id, - ); - (artifact.contract_id.clone(), (creation_code_anchors, deployed_code_anchors)) - }) - .collect::>(); - - report.add_anchors(anchors); - report.add_items(source_analysis.items); + for (item_id, item) in source_analysis.items.iter().enumerate() { + items_by_source_id.entry(item.loc.source_id.clone()).or_default().push(item_id); } + let anchors = artifacts + .par_iter() + .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) + .map(|artifact| { + let creation_code_anchors = artifact.creation.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); + let deployed_code_anchors = artifact.deployed.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); + (artifact.contract_id.clone(), (creation_code_anchors, deployed_code_anchors)) + }) + .collect::>(); + + report.add_anchors(anchors); + report.add_items(source_analysis.items); + report.add_source_maps(artifacts.into_iter().map(|artifact| { (artifact.contract_id, (artifact.creation.source_map, artifact.deployed.source_map)) })); From 82977395d83bcd9355be65423f4b7e07d0786da0 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 18:19:34 +0530 Subject: [PATCH 17/20] fix: respect artifact.build_id while processing hitmaps --- crates/forge/bin/cmd/coverage.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index d6ff588c55f1..a088e37df058 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -302,7 +302,13 @@ impl CoverageArgs { for (artifact_id, map, is_deployed_code) in data { if let Some(source_id) = report.get_source_id(artifact_id.source.clone()) { report.add_hit_map( - &ContractId { source_id, contract_name: artifact_id.name.as_str().into() }, + &ContractId { + source_id: SourceIdentifier::new( + source_id.source_id, + artifact_id.build_id.clone(), + ), + contract_name: artifact_id.name.as_str().into(), + }, map, is_deployed_code, )?; From 13d3fd74f3a773572d465f7ce737b1a9fea9cbc0 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Fri, 29 Nov 2024 18:22:38 +0530 Subject: [PATCH 18/20] fix: respect element idx --- crates/forge/src/coverage.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/forge/src/coverage.rs b/crates/forge/src/coverage.rs index 061f9f517157..8aad563bee7e 100644 --- a/crates/forge/src/coverage.rs +++ b/crates/forge/src/coverage.rs @@ -1,6 +1,7 @@ //! Coverage reports. use alloy_primitives::map::HashMap; +use analysis::SourceIdentifier; use comfy_table::{presets::ASCII_MARKDOWN, Attribute, Cell, Color, Row, Table}; use evm_disassembler::disassemble_bytes; use foundry_common::fs; @@ -222,10 +223,11 @@ impl CoverageReporter for BytecodeReporter { .map(|h| format!("[{h:03}]")) .unwrap_or(" ".to_owned()); let source_id = source_element.index(); - let source_path = source_id.and_then(|_i| { - report - .source_paths - .get(&contract_id.source_id) + let source_path = source_id.and_then(|i| { + report.source_paths.get(&SourceIdentifier::new( + i as usize, + contract_id.source_id.build_id.clone(), + )) }); let code = format!("{code:?}"); From 973cfe02766dd67241b9a5654294663ee3b23359 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:46:10 +0530 Subject: [PATCH 19/20] key source_ids by version and path --- Cargo.lock | 1 + crates/evm/coverage/Cargo.toml | 1 + crates/evm/coverage/src/lib.rs | 11 ++--- crates/forge/bin/cmd/coverage.rs | 74 +++++++++++++++++--------------- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28e74de3b0ec..e6f33e224c89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4022,6 +4022,7 @@ dependencies = [ "foundry-evm-core", "rayon", "revm", + "semver 1.0.23", "tracing", ] diff --git a/crates/evm/coverage/Cargo.toml b/crates/evm/coverage/Cargo.toml index a371dd08edc4..569942d4f519 100644 --- a/crates/evm/coverage/Cargo.toml +++ b/crates/evm/coverage/Cargo.toml @@ -23,3 +23,4 @@ eyre.workspace = true revm.workspace = true tracing.workspace = true rayon.workspace = true +semver.workspace = true diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index d5e32c6af051..fa53ef0069d9 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -15,6 +15,7 @@ use alloy_primitives::{map::HashMap, Bytes, B256}; use analysis::SourceIdentifier; use eyre::{Context, Result}; use foundry_compilers::artifacts::sourcemap::SourceMap; +use semver::Version; use std::{ collections::BTreeMap, fmt::Display, @@ -38,7 +39,7 @@ pub struct CoverageReport { /// A map of source IDs to the source path. pub source_paths: HashMap, /// A map of source paths to source IDs. - pub source_paths_to_ids: HashMap, + pub source_paths_to_ids: HashMap<(Version, PathBuf), SourceIdentifier>, /// All coverage items for the codebase, keyed by the compiler version. pub items: HashMap>, /// All item anchors for the codebase, keyed by their contract ID. @@ -53,14 +54,14 @@ pub struct CoverageReport { impl CoverageReport { /// Add a source file path. - pub fn add_source(&mut self, source_id: SourceIdentifier, path: PathBuf) { + pub fn add_source(&mut self, version: Version, source_id: SourceIdentifier, path: PathBuf) { self.source_paths.insert(source_id.clone(), path.clone()); - self.source_paths_to_ids.insert(path, source_id); + self.source_paths_to_ids.insert((version, path), source_id); } /// Get the source ID for a specific source file path. - pub fn get_source_id(&self, path: PathBuf) -> Option { - self.source_paths_to_ids.get(&path).cloned() + pub fn get_source_id(&self, version: Version, path: PathBuf) -> Option { + self.source_paths_to_ids.get(&(version, path)).cloned() } /// Add the source maps. diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index a088e37df058..d222123b426d 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -162,7 +162,7 @@ impl CoverageArgs { // Collect source files. let project_paths = &project.paths; - let mut sources = SourceFiles::default(); + let mut versioned_sources = HashMap::>::default(); // Account cached and freshly compiled sources for (id, artifact) in output.artifact_ids() { // Filter out dependencies @@ -179,7 +179,7 @@ impl CoverageArgs { }; let identifier = SourceIdentifier::new(source_file.id as usize, build_id.clone()); - report.add_source(identifier.clone(), id.source.clone()); + report.add_source(id.version.clone(), identifier.clone(), id.source.clone()); if let Some(ast) = source_file.ast { let file = project_paths.root.join(id.source); @@ -191,7 +191,7 @@ impl CoverageArgs { .wrap_err("Could not read source code for analysis")?, }; - sources.sources.entry(identifier).or_insert(source); + versioned_sources.entry(id.version).or_default().sources.insert(identifier, source); } } @@ -200,44 +200,46 @@ impl CoverageArgs { .artifact_ids() .par_bridge() .filter_map(|(id, artifact)| { - let source_id = report.get_source_id(id.source.clone())?; + let source_id = report.get_source_id(id.version.clone(), id.source.clone())?; ArtifactData::new(&id, source_id, artifact) }) .collect(); - // Add coverage items - let source_analysis = SourceAnalyzer::new(&sources).analyze()?; + for (_version, sources) in versioned_sources { + // Add coverage items + let source_analysis = SourceAnalyzer::new(&sources).analyze()?; - // Build helper mapping used by `find_anchors` - let mut items_by_source_id = HashMap::<_, Vec<_>>::with_capacity_and_hasher( - source_analysis.items.len(), - Default::default(), - ); + // Build helper mapping used by `find_anchors` + let mut items_by_source_id = HashMap::<_, Vec<_>>::with_capacity_and_hasher( + source_analysis.items.len(), + Default::default(), + ); - for (item_id, item) in source_analysis.items.iter().enumerate() { - items_by_source_id.entry(item.loc.source_id.clone()).or_default().push(item_id); - } - - let anchors = artifacts - .par_iter() - .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) - .map(|artifact| { - let creation_code_anchors = artifact.creation.find_anchors( - &source_analysis, - &items_by_source_id, - &artifact.contract_id.source_id, - ); - let deployed_code_anchors = artifact.deployed.find_anchors( - &source_analysis, - &items_by_source_id, - &artifact.contract_id.source_id, - ); - (artifact.contract_id.clone(), (creation_code_anchors, deployed_code_anchors)) - }) - .collect::>(); + for (item_id, item) in source_analysis.items.iter().enumerate() { + items_by_source_id.entry(item.loc.source_id.clone()).or_default().push(item_id); + } - report.add_anchors(anchors); - report.add_items(source_analysis.items); + let anchors = artifacts + .par_iter() + .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) + .map(|artifact| { + let creation_code_anchors = artifact.creation.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); + let deployed_code_anchors = artifact.deployed.find_anchors( + &source_analysis, + &items_by_source_id, + &artifact.contract_id.source_id, + ); + (artifact.contract_id.clone(), (creation_code_anchors, deployed_code_anchors)) + }) + .collect::>(); + + report.add_anchors(anchors); + report.add_items(source_analysis.items); + } report.add_source_maps(artifacts.into_iter().map(|artifact| { (artifact.contract_id, (artifact.creation.source_map, artifact.deployed.source_map)) @@ -300,7 +302,9 @@ impl CoverageArgs { }); for (artifact_id, map, is_deployed_code) in data { - if let Some(source_id) = report.get_source_id(artifact_id.source.clone()) { + if let Some(source_id) = + report.get_source_id(artifact_id.version.clone(), artifact_id.source.clone()) + { report.add_hit_map( &ContractId { source_id: SourceIdentifier::new( From 162458750a79a4ff8df7e65ec1d206515d5fce05 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:10:30 +0530 Subject: [PATCH 20/20] nit --- crates/evm/coverage/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs index 7177f8108e14..e33beed2a349 100644 --- a/crates/evm/coverage/src/lib.rs +++ b/crates/evm/coverage/src/lib.rs @@ -104,7 +104,7 @@ impl CoverageReport { let mut by_file: BTreeMap<&Path, T> = BTreeMap::new(); for (key, items) in &self.items { for item in items { - let Some(path) = self.source_paths.get(&key) else { continue }; + let Some(path) = self.source_paths.get(key) else { continue }; f(by_file.entry(path).or_default(), item); } }