From 923e6061844cef994d0c5efc567f95dee746ca2f Mon Sep 17 00:00:00 2001 From: Adrian Palacios Date: Fri, 20 Sep 2024 20:47:07 +0000 Subject: [PATCH] Most clippy fixes --- tools/kani-cov/src/coverage.rs | 21 ++++------- tools/kani-cov/src/merge.rs | 2 +- tools/kani-cov/src/report.rs | 16 ++++----- tools/kani-cov/src/summary.rs | 64 ++++++++++++++++------------------ 4 files changed, 44 insertions(+), 59 deletions(-) diff --git a/tools/kani-cov/src/coverage.rs b/tools/kani-cov/src/coverage.rs index b4a35d331481..1efd0129f7db 100644 --- a/tools/kani-cov/src/coverage.rs +++ b/tools/kani-cov/src/coverage.rs @@ -3,11 +3,13 @@ use console::style; use serde_derive::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::{collections::BTreeMap, fmt::Display}; use std::{fmt, fs}; use tree_sitter::{Node, Parser}; +pub type LineResults = Vec>; + #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "UPPERCASE")] pub enum CheckStatus { @@ -62,17 +64,6 @@ impl Display for CoverageRegion { } } -// impl CoverageRegion { -// pub fn from_str(str: String) -> Self { -// let str_splits: Vec<&str> = str.split([':', '-']).map(|s| s.trim()).collect(); -// assert_eq!(str_splits.len(), 5, "{str:?}"); -// let file = str_splits[0].to_string(); -// let start = (str_splits[1].parse().unwrap(), str_splits[2].parse().unwrap()); -// let end = (str_splits[3].parse().unwrap(), str_splits[4].parse().unwrap()); -// Self { file, start, end } -// } -// } - #[derive(Debug, Clone, Serialize, Deserialize)] pub enum CoverageTerm { Counter(u32), @@ -143,7 +134,7 @@ pub fn function_info_from_file(filepath: &PathBuf) -> Vec { function_info } -fn function_info_from_node<'a>(node: Node, source: &'a [u8]) -> FunctionInfo { +fn function_info_from_node(node: Node, source: &[u8]) -> FunctionInfo { let name = node .child_by_field_name("name") .and_then(|name| name.utf8_text(source).ok()) @@ -163,12 +154,12 @@ pub struct FunctionInfo { pub fn function_coverage_results( info: &FunctionInfo, - file: &PathBuf, + file: &Path, results: &CombinedCoverageResults, ) -> Option<(String, Vec)> { // `info` does not include file so how do we match? // use function just for now... - let filename = file.clone().into_os_string().into_string().unwrap(); + let filename = file.to_path_buf().into_os_string().into_string().unwrap(); let right_filename = results.data.keys().find(|p| filename.ends_with(*p)).unwrap(); // TODO: The filenames in kaniraw files should be absolute, just like in metadata // Otherwise the key for `results` just fails... diff --git a/tools/kani-cov/src/merge.rs b/tools/kani-cov/src/merge.rs index 76415e8457b5..48d542b4824e 100644 --- a/tools/kani-cov/src/merge.rs +++ b/tools/kani-cov/src/merge.rs @@ -111,7 +111,7 @@ fn save_combined_results( let output_path = if let Some(out) = output { out } else { &PathBuf::from("default_kanicov.json") }; - let file = OpenOptions::new().write(true).create(true).open(output_path)?; + let file = OpenOptions::new().write(true).create(true).truncate(true).open(output_path)?; let writer = BufWriter::new(file); serde_json::to_writer(writer, results)?; diff --git a/tools/kani-cov/src/report.rs b/tools/kani-cov/src/report.rs index cfd7ee9bf405..dd30c05bc9ac 100644 --- a/tools/kani-cov/src/report.rs +++ b/tools/kani-cov/src/report.rs @@ -69,7 +69,7 @@ pub fn print_coverage_results( ) -> Result<()> { let flattened_results: Vec<(usize, Option<(u32, MarkerInfo)>)> = results.into_iter().flatten().collect(); - println!("{}", filepath.to_string_lossy().to_string()); + println!("{}", filepath.to_string_lossy()); let file = File::open(filepath)?; let reader = BufReader::new(file); @@ -94,7 +94,7 @@ pub fn print_coverage_results( { // Filter out cases where the span is a single unit AND it ends after the line let results: Vec<&CovResult> = results - .into_iter() + .iter() .filter(|m| { if m.region.start.0 as usize == idx && m.region.end.0 as usize == idx @@ -113,13 +113,12 @@ pub fn print_coverage_results( && m.region.start.0 as usize == idx && m.region.end.0 as usize == idx }) - .map(|m| { + .flat_map(|m| { vec![ ((m.region.start.1 - 1) as usize, true), ((m.region.end.1 - 1) as usize, false), ] }) - .flatten() .collect(); // println!("COMPLETE: {complete_escapes:?}"); let mut starting_escapes: Vec<(usize, bool)> = results @@ -129,8 +128,7 @@ pub fn print_coverage_results( && m.region.start.0 as usize == idx && m.region.end.0 as usize != idx }) - .map(|m| vec![((m.region.start.1 - 1) as usize, true)]) - .flatten() + .flat_map(|m| vec![((m.region.start.1 - 1) as usize, true)]) .collect(); // println!("{starting_escapes:?}"); let mut ending_escapes: Vec<(usize, bool)> = results @@ -146,11 +144,11 @@ pub fn print_coverage_results( // println!("{starting_escapes:?}"); // println!("{ending_escapes:?}"); - if must_highlight && ending_escapes.len() > 0 { + if must_highlight && !ending_escapes.is_empty() { ending_escapes.push((0_usize, true)); must_highlight = false; } - if starting_escapes.len() > 0 { + if !starting_escapes.is_empty() { starting_escapes.push((line.len(), false)); must_highlight = true; } @@ -211,7 +209,7 @@ fn insert_escapes(str: &String, markers: Vec<(usize, bool)>, format: &ReportForm sym_markers.sort(); for (i, b) in sym_markers { new_str.insert_str(i + offset, b); - offset = offset + b.bytes().len(); + offset += b.bytes().len(); } new_str } diff --git a/tools/kani-cov/src/summary.rs b/tools/kani-cov/src/summary.rs index 8623e550c550..344936a7c212 100644 --- a/tools/kani-cov/src/summary.rs +++ b/tools/kani-cov/src/summary.rs @@ -9,7 +9,7 @@ use crate::{ args::{SummaryArgs, SummaryFormat}, coverage::{ function_coverage_results, function_info_from_file, CombinedCoverageResults, CovResult, - CoverageMetric, CoverageRegion, FileCoverageInfo, FunctionInfo, MarkerInfo, + CoverageMetric, CoverageRegion, FileCoverageInfo, FunctionInfo, LineResults, MarkerInfo, }, }; @@ -101,20 +101,20 @@ pub fn validate_summary_args(_args: &SummaryArgs) -> Result<()> { /// Basically, for each line we produce an `>` result /// where: /// * `None` means there were no coverage results associated with this line. -/// This may happen in lines that only contain a closing `}`, for example. +/// This may happen in lines that only contain a closing `}`, for example. /// * `Some(max, markers)` means there were coverage results associated with -/// the line or we deduced no results were possible based on function -/// information (i.e., the function was not reachable during verification). -/// Here, `max` represents the maximum number of times the line was covered by -/// any coverage result, and `markers` represents marker information which is -/// relevant to the line (including coverage results). +/// the line or we deduced no results were possible based on function +/// information (i.e., the function was not reachable during verification). +/// Here, `max` represents the maximum number of times the line was covered by +/// any coverage result, and `markers` represents marker information which is +/// relevant to the line (including coverage results). /// /// As a result, we essentially precompute here most of the information required /// for the generation of coverage reports. pub fn line_coverage_results( info: &FunctionInfo, fun_results: &Option<(String, Vec)>, -) -> Vec> { +) -> LineResults { let start_line: u32 = info.start.0.try_into().unwrap(); let end_line: u32 = info.end.0.try_into().unwrap(); @@ -192,6 +192,7 @@ fn region_coverage_info( } } +/// Output coverage information for a set of files fn print_coverage_info(info: &Vec, format: &SummaryFormat) { match format { SummaryFormat::Markdown => print_coverage_markdown_info(info), @@ -199,6 +200,7 @@ fn print_coverage_info(info: &Vec, format: &SummaryFormat) { } } +/// Output coverage information for a set of files in the markdown format fn print_coverage_markdown_info(info: &Vec) { fn safe_div(num: u32, denom: u32) -> Option { if denom == 0 { None } else { Some(num as f32 / denom as f32) } @@ -258,36 +260,30 @@ fn print_coverage_markdown_info(info: &Vec) { data_rows.push((filename, function_fmt, line_fmt, region_fmt)); } - let filename_sep: String = std::iter::repeat('-').take(max_filename_fmt_width).collect(); - let filename_space: String = std::iter::repeat(' ') - .take(max_filename_fmt_width - FILENAME_HEADER.len()) - .collect::(); - let function_sep: String = std::iter::repeat('-').take(max_function_fmt_width).collect(); - let function_space: String = std::iter::repeat(' ') - .take(max_function_fmt_width - FUNCTION_HEADER.len()) - .collect::(); - let line_sep: String = std::iter::repeat('-').take(max_line_fmt_width).collect(); - let line_space: String = - std::iter::repeat(' ').take(max_line_fmt_width - LINE_HEADER.len()).collect::(); - let region_sep: String = std::iter::repeat('-').take(max_region_fmt_width).collect(); - let region_space: String = - std::iter::repeat(' ').take(max_region_fmt_width - REGION_HEADER.len()).collect::(); + let filename_space = " ".repeat(max_filename_fmt_width - FILENAME_HEADER.len()); + let function_space = " ".repeat(max_function_fmt_width - FUNCTION_HEADER.len()); + let line_space = " ".repeat(max_line_fmt_width - LINE_HEADER.len()); + let region_space = " ".repeat(max_region_fmt_width - REGION_HEADER.len()); + + let header_row = format!( + "| {FILENAME_HEADER}{filename_space} | {FUNCTION_HEADER}{function_space} | {LINE_HEADER}{line_space} | {REGION_HEADER}{region_space} |" + ); + table_rows.push(header_row); + + let filename_sep = "-".repeat(max_filename_fmt_width); + let function_sep = "-".repeat(max_function_fmt_width); + let line_sep = "-".repeat(max_line_fmt_width); + let region_sep = "-".repeat(max_region_fmt_width); let sep_row = format!("| {filename_sep} | {function_sep} | {line_sep} | {region_sep} |"); - table_rows.push(format!("| {FILENAME_HEADER}{filename_space} | {FUNCTION_HEADER}{function_space} | {LINE_HEADER}{line_space} | {REGION_HEADER}{region_space} |")); table_rows.push(sep_row); + for (filename, function_fmt, line_fmt, region_fmt) in data_rows { - let filename_space: String = std::iter::repeat(' ') - .take(max_filename_fmt_width - filename.len()) - .collect::(); - let function_space: String = std::iter::repeat(' ') - .take(max_function_fmt_width - function_fmt.len()) - .collect::(); - let line_space: String = - std::iter::repeat(' ').take(max_line_fmt_width - line_fmt.len()).collect::(); - let region_space: String = std::iter::repeat(' ') - .take(max_region_fmt_width - region_fmt.len()) - .collect::(); + let filename_space = " ".repeat(max_filename_fmt_width - filename.len()); + let function_space = " ".repeat(max_function_fmt_width - function_fmt.len()); + let line_space = " ".repeat(max_line_fmt_width - line_fmt.len()); + let region_space = " ".repeat(max_region_fmt_width - region_fmt.len()); + let cur_row = format!( "| {filename}{filename_space} | {function_fmt}{function_space} | {line_fmt}{line_space} | {region_fmt}{region_space} |" );