Allow users to not include macros in their coverage report

commit-id:dd9032f3

CHANGELOG.md

@@ -6,3 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+
+#### Added
+
+- Option to not include macros in coverage report. To get the same behavior as before use `--include macros`.
+
+#### Fixed
+
+- Bug were hit count was not correctly calculated for functions declared at same line
+
+#### Changed
+
+- `--include-test-functions` was remove in favor of `--include`. To get same behavior as before use `--include tests-functions`.

crates/cairo-coverage/src/cli.rs

@@ -1,6 +1,6 @@
 use anyhow::{ensure, Result};
 use camino::Utf8PathBuf;
-use clap::Parser;
+use clap::{Parser, ValueEnum};
 
 #[derive(Parser, Debug)]
 #[command(version)]
@@ -13,14 +13,17 @@ pub struct Cli {
     #[arg(short, long, default_value = "coverage.lcov")]
     pub output_path: Utf8PathBuf,
 
-    /// Run coverage on functions marked with `#[test]` attribute.
-    ///
-    /// [default: false]
-    ///
-    /// Note: We currently recommend setting this to false as there
-    /// might be issues with mappings for `#[test]` attribute.
-    #[arg(long, default_value_t = false)]
-    pub include_test_functions: bool,
+    /// Include additional components in the coverage report.
+    #[arg(long, short, num_args = 1..)]
+    pub include: Vec<IncludedComponent>,
+}
+
+#[derive(ValueEnum, Debug, Clone, Eq, PartialEq)]
+pub enum IncludedComponent {
+    /// Run coverage on functions marked with `#[test]` attribute
+    TestFunctions,
+    /// Run coverage on macros and generated code by them. This includes inline macros, attribute macros, and derive macros.
+    Macros,
 }
 
 fn parse_trace_file(path: &str) -> Result<Utf8PathBuf> {

crates/cairo-coverage/src/coverage_data/function.rs

@@ -1,6 +1,7 @@
 use crate::data_loader::LineRange;
 use crate::input::{InputData, SierraToCairoMap};
 use crate::types::{FileLocation, FunctionName, HitCount, LineNumber};
+use itertools::Itertools;
 use std::collections::HashMap;
 
 pub type FilesCoverageData = HashMap<FileLocation, FileCoverageData>;
@@ -63,7 +64,8 @@ impl FileCoverageDataOps for FileCoverageData {
     fn lines(&self) -> HashMap<LineNumber, HitCount> {
         self.values()
             .flat_map(|details| details.iter().map(|(&line, &hits)| (line, hits)))
-            .collect()
+            .into_grouping_map()
+            .fold(0, |hits1, _, hits2| hits1 + hits2)
     }
 
     fn unique_file_hit_count(&self) -> HitCount {

crates/cairo-coverage/src/input/data.rs

@@ -1,13 +1,11 @@
 use crate::data_loader::LoadedData;
-use crate::input::test_function_filter::TestFunctionFilter;
+use crate::input::filter::Filter;
 use crate::input::{create_sierra_to_cairo_map, SierraToCairoMap, UniqueExecutedSierraIds};
 use anyhow::{Context, Result};
 use cairo_lang_sierra::program::Program;
 use cairo_lang_sierra_to_casm::compiler::{CairoProgram, SierraToCasmConfig};
 use cairo_lang_sierra_to_casm::metadata::{calc_metadata, MetadataComputationConfig};
 
-const SNFORGE_TEST_EXECUTABLE: &str = "snforge_internal_test_executable";
-
 pub struct InputData {
     pub unique_executed_sierra_ids: UniqueExecutedSierraIds,
     pub sierra_to_cairo_map: SierraToCairoMap,
@@ -20,17 +18,9 @@ impl InputData {
             debug_info,
             call_traces,
         }: &LoadedData,
-        include_test_functions: bool,
+        filter: &Filter,
     ) -> Result<Self> {
-        let test_function_filter = TestFunctionFilter::new(
-            debug_info
-                .executables
-                .get(SNFORGE_TEST_EXECUTABLE)
-                .unwrap_or(&Vec::new()),
-            include_test_functions,
-        );
-
-        let sierra_to_cairo_map = create_sierra_to_cairo_map(debug_info, &test_function_filter)?;
+        let sierra_to_cairo_map = create_sierra_to_cairo_map(debug_info, filter)?;
         let casm = compile_to_casm(program)?;
         let unique_executed_sierra_ids = call_traces
             .iter()

crates/cairo-coverage/src/input/filter.rs

@@ -0,0 +1,90 @@
+use crate::cli::IncludedComponent;
+use crate::data_loader::LoadedData;
+use crate::input::sierra_to_cairo_map::StatementOrigin;
+use regex::Regex;
+use std::collections::HashSet;
+use std::iter::once;
+use std::sync::LazyLock;
+
+pub static VIRTUAL_FILE_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\[.*?]").unwrap());
+const SNFORGE_TEST_EXECUTABLE: &str = "snforge_internal_test_executable";
+
+#[derive(Eq, PartialEq, Hash)]
+enum ItemLabel {
+    TestFunction,
+    UserFunction,
+    NonUserFunction,
+    Macro,
+}
+
+impl From<IncludedComponent> for ItemLabel {
+    fn from(included_component: IncludedComponent) -> Self {
+        match included_component {
+            IncludedComponent::TestFunctions => ItemLabel::TestFunction,
+            IncludedComponent::Macros => ItemLabel::Macro,
+        }
+    }
+}
+
+pub struct Filter {
+    allowed_labels: HashSet<ItemLabel>,
+    test_functions: HashSet<String>,
+}
+
+impl Filter {
+    pub fn new(included_component: &[IncludedComponent], loaded_data: &LoadedData) -> Self {
+        let test_functions = loaded_data
+            .debug_info
+            .executables
+            .get(SNFORGE_TEST_EXECUTABLE)
+            .unwrap_or(&Vec::new())
+            .iter()
+            .map(ToString::to_string)
+            .collect();
+
+        let allowed_labels = included_component
+            .iter()
+            .cloned()
+            .map(ItemLabel::from)
+            .chain(once(ItemLabel::UserFunction))
+            .collect();
+
+        Self {
+            allowed_labels,
+            test_functions,
+        }
+    }
+
+    pub fn should_include(&self, statement_origin: &StatementOrigin) -> bool {
+        self.labels(statement_origin)
+            .is_subset(&self.allowed_labels)
+    }
+
+    fn labels(
+        &self,
+        StatementOrigin {
+            function_name,
+            file_location,
+            ..
+        }: &StatementOrigin,
+    ) -> HashSet<ItemLabel> {
+        let mut labels = HashSet::new();
+
+        if self.test_functions.contains(function_name) {
+            labels.insert(ItemLabel::TestFunction);
+        } else if VIRTUAL_FILE_REGEX.is_match(file_location) {
+            labels.insert(ItemLabel::Macro);
+        }
+
+        // TODO(#55)
+        // TODO: We should probably filter by path to user project not by path to cache
+        // TODO: Can get this from source_sierra_path in call trace
+        if file_location.contains("com.swmansion.scarb") || file_location.contains(".cache/scarb") {
+            labels.insert(ItemLabel::NonUserFunction);
+        } else {
+            labels.insert(ItemLabel::UserFunction);
+        }
+
+        labels
+    }
+}

crates/cairo-coverage/src/input/mod.rs

@@ -1,8 +1,9 @@
 mod data;
+mod filter;
 mod sierra_to_cairo_map;
-mod test_function_filter;
 mod unique_executed_sierra_ids;
 
 pub use data::InputData;
+pub use filter::Filter;
 pub use sierra_to_cairo_map::{create_sierra_to_cairo_map, SierraToCairoMap};
 pub use unique_executed_sierra_ids::UniqueExecutedSierraIds;

crates/cairo-coverage/src/input/sierra_to_cairo_map.rs

@@ -1,16 +1,12 @@
 use crate::data_loader::{CodeLocation, CoverageAnnotations, LineRange, ProfilerAnnotations};
-use crate::input::test_function_filter::TestFunctionFilter;
+use crate::input::filter::{Filter, VIRTUAL_FILE_REGEX};
 use crate::types::{FileLocation, FunctionName};
 use anyhow::{Context, Result};
 use cairo_lang_sierra::debug_info::{Annotations, DebugInfo};
 use cairo_lang_sierra::program::StatementIdx;
 use derived_deref::Deref;
-use regex::Regex;
 use serde::de::DeserializeOwned;
 use std::collections::HashMap;
-use std::sync::LazyLock;
-
-static VIRTUAL_FILE_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\[.*?]").unwrap());
 
 #[derive(Deref)]
 pub struct SierraToCairoMap(HashMap<StatementIdx, StatementOrigin>);
@@ -22,9 +18,17 @@ pub struct StatementOrigin {
     pub line_range: LineRange,
 }
 
+impl StatementOrigin {
+    pub fn remove_virtual_file_prefix(&mut self) {
+        self.file_location = VIRTUAL_FILE_REGEX
+            .replace_all(&self.file_location, "")
+            .to_string();
+    }
+}
+
 pub fn create_sierra_to_cairo_map(
     debug_info: &DebugInfo,
-    test_function_filter: &TestFunctionFilter,
+    filter: &Filter,
 ) -> Result<SierraToCairoMap> {
     let CoverageAnnotations {
         statements_code_locations,
@@ -41,10 +45,7 @@ pub fn create_sierra_to_cairo_map(
                 statements_functions
                     .get(&key)
                     .and_then(|function_names| {
-                        find_statement_origin(&code_locations, function_names)
-                    })
-                    .filter(|statement_origin| {
-                        test_function_filter.should_include(&statement_origin.function_name)
+                        find_statement_origin(&code_locations, function_names, filter)
                     })
                     .map(|statement_origin| (key, statement_origin))
             })
@@ -55,30 +56,23 @@ pub fn create_sierra_to_cairo_map(
 fn find_statement_origin(
     code_locations: &[CodeLocation],
     function_names: &[FunctionName],
+    filter: &Filter,
 ) -> Option<StatementOrigin> {
     code_locations
         .iter()
         .zip(function_names)
-        // TODO(#55)
-        // TODO: We should probably filter by path to user project not by path to cache
-        // TODO: Can get this from source_sierra_path in call trace
-        .find(|((file_location, _), _)| {
-            !file_location.contains("com.swmansion.scarb")
-                && !file_location.contains(".cache/scarb")
-        })
         .map(
             |((file_location, line_range), function_name)| StatementOrigin {
-                function_name: function_name.to_owned(),
-                file_location: remove_virtual_files(file_location),
+                function_name: function_name.clone(),
+                file_location: file_location.clone(),
                 line_range: line_range.move_by_1(),
             },
         )
-}
-
-fn remove_virtual_files(file_location: &str) -> FileLocation {
-    VIRTUAL_FILE_REGEX
-        .replace_all(file_location, "")
-        .to_string()
+        .find(|statement_origin| filter.should_include(statement_origin))
+        .map(|mut statement_origin| {
+            statement_origin.remove_virtual_file_prefix();
+            statement_origin
+        })
 }
 
 trait Namespace {

crates/cairo-coverage/src/main.rs

@@ -7,7 +7,7 @@ mod types;
 
 use crate::coverage_data::create_files_coverage_data_with_hits;
 use crate::data_loader::LoadedDataMap;
-use crate::input::InputData;
+use crate::input::{Filter, InputData};
 use crate::output::lcov::LcovFormat;
 use anyhow::{Context, Result};
 use clap::Parser;
@@ -28,7 +28,8 @@ fn main() -> Result<()> {
 
     let loaded_data = LoadedDataMap::load(&cli.trace_files)?;
     for (_, loaded_data) in loaded_data.iter() {
-        let input_data = InputData::new(loaded_data, cli.include_test_functions)?;
+        let filter = Filter::new(&cli.include, loaded_data);
+        let input_data = InputData::new(loaded_data, &filter)?;
         let coverage_data = create_files_coverage_data_with_hits(&input_data);
         let output_data = LcovFormat::from(coverage_data);
 

crates/cairo-coverage/tests/data/macros/Scarb.toml

@@ -0,0 +1,22 @@
+[package]
+name = "macros"
+version = "0.1.0"
+edition = "2024_07"
+
+# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html
+[dependencies]
+starknet = ">=2.8.0"
+
+[dev-dependencies]
+snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry", tag = "v0.29.0" }
+
+[[target.starknet-contract]]
+sierra = true
+
+[scripts]
+test = "snforge test"
+
+[profile.dev.cairo]
+unstable-add-statements-functions-debug-info = true
+unstable-add-statements-code-locations-debug-info = true
+inlining-strategy= "avoid"

crates/cairo-coverage/tests/data/macros/src/lib.cairo

@@ -0,0 +1,13 @@
+fn function_with_macro() {
+    assert!(1 == 1);
+}
+
+#[cfg(test)]
+mod tests {
+    use super::function_with_macro;
+
+    #[test]
+    fn function_with_macro_test() {
+        function_with_macro()
+    }
+}