fix(coverage): use only string byte indexes and 0-indexed line numbers (#13190)

1 files changed, 263 insertions, 307 deletions

diff --git a/cli/tools/coverage.rs b/cli/tools/coverage.rs
index 042724e81..c91fca512 100644
--- a/cli/tools/coverage.rs
+++ b/cli/tools/coverage.rs
@@ -9,8 +9,8 @@ use crate::proc_state::ProcState;
 use crate::source_maps::SourceMapGetter;
 use crate::tools::fmt::format_json;
 
-use deno_ast::swc::common::Span;
 use deno_ast::MediaType;
+use deno_ast::ModuleSpecifier;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::url::Url;
@@ -25,15 +25,15 @@ use std::fs::File;
 use std::io::BufWriter;
 use std::io::Write;
 use std::path::PathBuf;
-use std::sync::Arc;
+use text_lines::TextLines;
 use uuid::Uuid;
 
-// TODO(caspervonb) all of these structs can and should be made private, possibly moved to
-// inspector::protocol.
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[serde(rename_all = "camelCase")]
 struct CoverageRange {
+  /// Start byte index.
   start_offset: usize,
+  /// End byte index.
   end_offset: usize,
   count: usize,
 }
@@ -171,392 +171,339 @@ impl CoverageCollector {
   }
 }
 
-enum CoverageReporterKind {
-  Pretty,
-  Lcov,
+struct BranchCoverageItem {
+  line_index: usize,
+  block_number: usize,
+  branch_number: usize,
+  taken: Option<usize>,
+  is_hit: bool,
 }
 
-fn create_reporter(
-  kind: CoverageReporterKind,
-) -> Box<dyn CoverageReporter + Send> {
-  match kind {
-    CoverageReporterKind::Lcov => Box::new(LcovCoverageReporter::new()),
-    CoverageReporterKind::Pretty => Box::new(PrettyCoverageReporter::new()),
-  }
+struct FunctionCoverageItem {
+  name: String,
+  line_index: usize,
+  execution_count: usize,
 }
 
-trait CoverageReporter {
-  fn visit_coverage(
-    &mut self,
-    script_coverage: &ScriptCoverage,
-    script_source: &str,
-    maybe_source_map: Option<Vec<u8>>,
-    maybe_original_source: Option<Arc<String>>,
-  );
-
-  fn done(&mut self);
+struct CoverageReport {
+  url: ModuleSpecifier,
+  named_functions: Vec<FunctionCoverageItem>,
+  branches: Vec<BranchCoverageItem>,
+  found_lines: Vec<(usize, usize)>,
 }
 
-struct LcovCoverageReporter {}
+fn generate_coverage_report(
+  script_coverage: &ScriptCoverage,
+  script_source: &str,
+  maybe_source_map: &Option<Vec<u8>>,
+) -> CoverageReport {
+  let maybe_source_map = maybe_source_map
+    .as_ref()
+    .map(|source_map| SourceMap::from_slice(source_map).unwrap());
+  let text_lines = TextLines::new(script_source);
+
+  let comment_spans = deno_ast::lex(script_source, MediaType::JavaScript)
+    .into_iter()
+    .filter(|item| {
+      matches!(item.inner, deno_ast::TokenOrComment::Comment { .. })
+    })
+    .map(|item| item.span)
+    .collect::<Vec<_>>();
+
+  let url = Url::parse(&script_coverage.url).unwrap();
+  let mut coverage_report = CoverageReport {
+    url,
+    named_functions: Vec::with_capacity(
+      script_coverage
+        .functions
+        .iter()
+        .filter(|f| !f.function_name.is_empty())
+        .count(),
+    ),
+    branches: Vec::new(),
+    found_lines: Vec::new(),
+  };
 
-impl LcovCoverageReporter {
-  pub fn new() -> LcovCoverageReporter {
-    LcovCoverageReporter {}
-  }
-}
+  for function in &script_coverage.functions {
+    if function.function_name.is_empty() {
+      continue;
+    }
 
-impl CoverageReporter for LcovCoverageReporter {
-  fn visit_coverage(
-    &mut self,
-    script_coverage: &ScriptCoverage,
-    script_source: &str,
-    maybe_source_map: Option<Vec<u8>>,
-    _maybe_original_source: Option<Arc<String>>,
-  ) {
-    // TODO(caspervonb) cleanup and reduce duplication between reporters, pre-compute line coverage
-    // elsewhere.
-    let maybe_source_map = maybe_source_map
-      .map(|source_map| SourceMap::from_slice(&source_map).unwrap());
-
-    let url = Url::parse(&script_coverage.url).unwrap();
-    let file_path = url.to_file_path().unwrap();
-    println!("SF:{}", file_path.to_str().unwrap());
-
-    let mut functions_found = 0;
-    for function in &script_coverage.functions {
-      if function.function_name.is_empty() {
-        continue;
-      }
+    let source_line_index =
+      text_lines.line_index(function.ranges[0].start_offset);
+    let line_index = if let Some(source_map) = maybe_source_map.as_ref() {
+      source_map
+        .tokens()
+        .find(|token| token.get_dst_line() as usize == source_line_index)
+        .map(|token| token.get_src_line() as usize)
+        .unwrap_or(0)
+    } else {
+      source_line_index
+    };
 
-      let source_line = script_source
-        .chars()
-        .take(function.ranges[0].start_offset)
-        .filter(|c| *c == '\n')
-        .count()
-        + 1;
+    coverage_report.named_functions.push(FunctionCoverageItem {
+      name: function.function_name.clone(),
+      line_index,
+      execution_count: function.ranges[0].count,
+    });
+  }
 
+  for (block_number, function) in script_coverage.functions.iter().enumerate() {
+    let block_hits = function.ranges[0].count;
+    for (branch_number, range) in function.ranges[1..].iter().enumerate() {
+      let source_line_index = text_lines.line_index(range.start_offset);
       let line_index = if let Some(source_map) = maybe_source_map.as_ref() {
         source_map
           .tokens()
-          .find(|token| token.get_dst_line() as usize == source_line)
+          .find(|token| token.get_dst_line() as usize == source_line_index)
           .map(|token| token.get_src_line() as usize)
           .unwrap_or(0)
       } else {
-        source_line
+        source_line_index
       };
 
-      let function_name = &function.function_name;
-
-      println!("FN:{},{}", line_index + 1, function_name);
-
-      functions_found += 1;
-    }
-
-    let mut functions_hit = 0;
-    for function in &script_coverage.functions {
-      if function.function_name.is_empty() {
-        continue;
-      }
-
-      let execution_count = function.ranges[0].count;
-      let function_name = &function.function_name;
-
-      println!("FNDA:{},{}", execution_count, function_name);
+      // From https://manpages.debian.org/unstable/lcov/geninfo.1.en.html:
+      //
+      // Block number and branch number are gcc internal IDs for the branch. Taken is either '-'
+      // if the basic block containing the branch was never executed or a number indicating how
+      // often that branch was taken.
+      //
+      // However with the data we get from v8 coverage profiles it seems we can't actually hit
+      // this as appears it won't consider any nested branches it hasn't seen but its here for
+      // the sake of accuracy.
+      let taken = if block_hits > 0 {
+        Some(range.count)
+      } else {
+        None
+      };
 
-      if execution_count != 0 {
-        functions_hit += 1;
-      }
+      coverage_report.branches.push(BranchCoverageItem {
+        line_index,
+        block_number,
+        branch_number,
+        taken,
+        is_hit: range.count > 0,
+      })
     }
+  }
 
-    println!("FNF:{}", functions_found);
-    println!("FNH:{}", functions_hit);
-
-    let mut branches_found = 0;
-    let mut branches_hit = 0;
-    for (block_number, function) in script_coverage.functions.iter().enumerate()
-    {
-      let block_hits = function.ranges[0].count;
-      for (branch_number, range) in function.ranges[1..].iter().enumerate() {
-        let source_line = script_source
-          .chars()
-          .take(range.start_offset)
-          .filter(|c| *c == '\n')
-          .count()
-          + 1;
-
-        let line_index = if let Some(source_map) = maybe_source_map.as_ref() {
-          source_map
-            .tokens()
-            .find(|token| token.get_dst_line() as usize == source_line)
-            .map(|token| token.get_src_line() as usize)
-            .unwrap_or(0)
-        } else {
-          source_line
-        };
-
-        // From https://manpages.debian.org/unstable/lcov/geninfo.1.en.html:
-        //
-        // Block number and branch number are gcc internal IDs for the branch. Taken is either '-'
-        // if the basic block containing the branch was never executed or a number indicating how
-        // often that branch was taken.
-        //
-        // However with the data we get from v8 coverage profiles it seems we can't actually hit
-        // this as appears it won't consider any nested branches it hasn't seen but its here for
-        // the sake of accuracy.
-        let taken = if block_hits > 0 {
-          range.count.to_string()
-        } else {
-          "-".to_string()
-        };
-
-        println!(
-          "BRDA:{},{},{},{}",
-          line_index + 1,
-          block_number,
-          branch_number,
-          taken
-        );
-
-        branches_found += 1;
-        if range.count > 0 {
-          branches_hit += 1;
+  // TODO(caspervonb): collect uncovered ranges on the lines so that we can highlight specific
+  // parts of a line in color (word diff style) instead of the entire line.
+  let mut line_counts = Vec::with_capacity(text_lines.lines_count());
+  for line_index in 0..text_lines.lines_count() {
+    let line_start_offset = text_lines.line_start(line_index);
+    let line_end_offset = text_lines.line_end(line_index);
+    let ignore = comment_spans.iter().any(|span| {
+      (span.lo.0 as usize) <= line_start_offset
+        && (span.hi.0 as usize) >= line_end_offset
+    }) || script_source[line_start_offset..line_end_offset]
+      .trim()
+      .is_empty();
+    let mut count = 0;
+
+    if ignore {
+      count = 1;
+    } else {
+      // Count the hits of ranges that include the entire line which will always be at-least one
+      // as long as the code has been evaluated.
+      for function in &script_coverage.functions {
+        for range in &function.ranges {
+          if range.start_offset <= line_start_offset
+            && range.end_offset >= line_end_offset
+          {
+            count += range.count;
+          }
         }
       }
-    }
-
-    println!("BRF:{}", branches_found);
-    println!("BRH:{}", branches_hit);
-
-    let lines = script_source.split('\n').collect::<Vec<_>>();
-    let line_offsets = {
-      let mut offsets: Vec<(usize, usize)> = Vec::new();
-      let mut index = 0;
-
-      for line in &lines {
-        offsets.push((index, index + line.len() + 1));
-        index += line.len() + 1;
-      }
 
-      offsets
-    };
-
-    let line_counts = line_offsets
-      .iter()
-      .map(|(line_start_offset, line_end_offset)| {
-        let mut count = 0;
-
-        // Count the hits of ranges that include the entire line which will always be at-least one
-        // as long as the code has been evaluated.
-        for function in &script_coverage.functions {
-          for range in &function.ranges {
-            if range.start_offset <= *line_start_offset
-              && range.end_offset >= *line_end_offset
-            {
-              count += range.count;
-            }
+      // We reset the count if any block with a zero count overlaps with the line range.
+      for function in &script_coverage.functions {
+        for range in &function.ranges {
+          if range.count > 0 {
+            continue;
           }
-        }
 
-        // We reset the count if any block with a zero count overlaps with the line range.
-        for function in &script_coverage.functions {
-          for range in &function.ranges {
-            if range.count > 0 {
-              continue;
-            }
-
-            let overlaps = std::cmp::max(line_end_offset, &range.end_offset)
-              - std::cmp::min(line_start_offset, &range.start_offset)
-              < (line_end_offset - line_start_offset)
-                + (range.end_offset - range.start_offset);
-
-            if overlaps {
-              count = 0;
-            }
+          let overlaps = range.start_offset < line_end_offset
+            && range.end_offset > line_start_offset;
+          if overlaps {
+            count = 0;
           }
         }
+      }
+    }
 
-        count
-      })
-      .collect::<Vec<usize>>();
+    line_counts.push(count);
+  }
 
-    let found_lines = if let Some(source_map) = maybe_source_map.as_ref() {
+  coverage_report.found_lines =
+    if let Some(source_map) = maybe_source_map.as_ref() {
       let mut found_lines = line_counts
         .iter()
         .enumerate()
         .map(|(index, count)| {
-          source_map
+          // get all the mappings from this destination line to a different src line
+          let mut results = source_map
             .tokens()
             .filter(move |token| token.get_dst_line() as usize == index)
             .map(move |token| (token.get_src_line() as usize, *count))
+            .collect::<Vec<_>>();
+          // only keep the results that point at different src lines
+          results.sort_unstable_by_key(|(index, _)| *index);
+          results.dedup_by_key(|(index, _)| *index);
+          results.into_iter()
         })
         .flatten()
         .collect::<Vec<(usize, usize)>>();
 
       found_lines.sort_unstable_by_key(|(index, _)| *index);
-      found_lines.dedup_by_key(|(index, _)| *index);
+      // combine duplicated lines
+      for i in (1..found_lines.len()).rev() {
+        if found_lines[i].0 == found_lines[i - 1].0 {
+          found_lines[i - 1].1 += found_lines[i].1;
+          found_lines.remove(i);
+        }
+      }
       found_lines
     } else {
       line_counts
-        .iter()
+        .into_iter()
         .enumerate()
-        .map(|(index, count)| (index, *count))
+        .map(|(index, count)| (index, count))
         .collect::<Vec<(usize, usize)>>()
     };
 
-    for (index, count) in &found_lines {
-      println!("DA:{},{}", index + 1, count);
-    }
-
-    let lines_hit = found_lines.iter().filter(|(_, count)| *count != 0).count();
-
-    println!("LH:{}", lines_hit);
+  coverage_report
+}
 
-    let lines_found = found_lines.len();
-    println!("LF:{}", lines_found);
+enum CoverageReporterKind {
+  Pretty,
+  Lcov,
+}
 
-    println!("end_of_record");
+fn create_reporter(
+  kind: CoverageReporterKind,
+) -> Box<dyn CoverageReporter + Send> {
+  match kind {
+    CoverageReporterKind::Lcov => Box::new(LcovCoverageReporter::new()),
+    CoverageReporterKind::Pretty => Box::new(PrettyCoverageReporter::new()),
   }
+}
 
-  fn done(&mut self) {}
+trait CoverageReporter {
+  fn report(&mut self, coverage_report: &CoverageReport, file_text: &str);
+
+  fn done(&mut self);
 }
 
-struct PrettyCoverageReporter {}
+struct LcovCoverageReporter {}
 
-impl PrettyCoverageReporter {
-  pub fn new() -> PrettyCoverageReporter {
-    PrettyCoverageReporter {}
+impl LcovCoverageReporter {
+  pub fn new() -> LcovCoverageReporter {
+    LcovCoverageReporter {}
   }
 }
 
-impl CoverageReporter for PrettyCoverageReporter {
-  fn visit_coverage(
-    &mut self,
-    script_coverage: &ScriptCoverage,
-    script_source: &str,
-    maybe_source_map: Option<Vec<u8>>,
-    maybe_original_source: Option<Arc<String>>,
-  ) {
-    let maybe_source_map = maybe_source_map
-      .map(|source_map| SourceMap::from_slice(&source_map).unwrap());
-
-    let mut ignored_spans: Vec<Span> = Vec::new();
-    for item in deno_ast::lex(script_source, MediaType::JavaScript) {
-      if let deno_ast::TokenOrComment::Token(_) = item.inner {
-        continue;
-      }
-
-      ignored_spans.push(item.span);
+impl CoverageReporter for LcovCoverageReporter {
+  fn report(&mut self, coverage_report: &CoverageReport, _file_text: &str) {
+    let file_path = coverage_report
+      .url
+      .to_file_path()
+      .ok()
+      .map(|p| p.to_str().map(|p| p.to_string()))
+      .flatten()
+      .unwrap_or_else(|| coverage_report.url.to_string());
+    println!("SF:{}", file_path);
+
+    for function in &coverage_report.named_functions {
+      println!("FN:{},{}", function.line_index + 1, function.name);
     }
 
-    let lines = script_source.split('\n').collect::<Vec<_>>();
-
-    let line_offsets = {
-      let mut offsets: Vec<(usize, usize)> = Vec::new();
-      let mut index = 0;
-
-      for line in &lines {
-        offsets.push((index, index + line.len() + 1));
-        index += line.len() + 1;
-      }
-
-      offsets
-    };
+    for function in &coverage_report.named_functions {
+      println!("FNDA:{},{}", function.execution_count, function.name);
+    }
 
-    // TODO(caspervonb): collect uncovered ranges on the lines so that we can highlight specific
-    // parts of a line in color (word diff style) instead of the entire line.
-    let line_counts = line_offsets
+    let functions_found = coverage_report.named_functions.len();
+    println!("FNF:{}", functions_found);
+    let functions_hit = coverage_report
+      .named_functions
       .iter()
-      .enumerate()
-      .map(|(index, (line_start_offset, line_end_offset))| {
-        let ignore = ignored_spans.iter().any(|span| {
-          (span.lo.0 as usize) <= *line_start_offset
-            && (span.hi.0 as usize) >= *line_end_offset
-        });
-
-        if ignore {
-          return (index, 1);
-        }
+      .filter(|f| f.execution_count > 0)
+      .count();
+    println!("FNH:{}", functions_hit);
 
-        let mut count = 0;
+    for branch in &coverage_report.branches {
+      let taken = if let Some(taken) = &branch.taken {
+        taken.to_string()
+      } else {
+        "-".to_string()
+      };
 
-        // Count the hits of ranges that include the entire line which will always be at-least one
-        // as long as the code has been evaluated.
-        for function in &script_coverage.functions {
-          for range in &function.ranges {
-            if range.start_offset <= *line_start_offset
-              && range.end_offset >= *line_end_offset
-            {
-              count += range.count;
-            }
-          }
-        }
+      println!(
+        "BRDA:{},{},{},{}",
+        branch.line_index + 1,
+        branch.block_number,
+        branch.branch_number,
+        taken
+      );
+    }
 
-        // We reset the count if any block with a zero count overlaps with the line range.
-        for function in &script_coverage.functions {
-          for range in &function.ranges {
-            if range.count > 0 {
-              continue;
-            }
+    let branches_found = coverage_report.branches.len();
+    println!("BRF:{}", branches_found);
+    let branches_hit =
+      coverage_report.branches.iter().filter(|b| b.is_hit).count();
+    println!("BRH:{}", branches_hit);
 
-            let overlaps = std::cmp::max(line_end_offset, &range.end_offset)
-              - std::cmp::min(line_start_offset, &range.start_offset)
-              < (line_end_offset - line_start_offset)
-                + (range.end_offset - range.start_offset);
+    for (index, count) in &coverage_report.found_lines {
+      println!("DA:{},{}", index + 1, count);
+    }
 
-            if overlaps {
-              count = 0;
-            }
-          }
-        }
+    let lines_hit = coverage_report
+      .found_lines
+      .iter()
+      .filter(|(_, count)| *count != 0)
+      .count();
+    println!("LH:{}", lines_hit);
 
-        (index, count)
-      })
-      .collect::<Vec<(usize, usize)>>();
+    let lines_found = coverage_report.found_lines.len();
+    println!("LF:{}", lines_found);
 
-    let lines = if let Some(original_source) = maybe_original_source.as_ref() {
-      original_source.split('\n').collect::<Vec<_>>()
-    } else {
-      lines
-    };
+    println!("end_of_record");
+  }
 
-    let line_counts = if let Some(source_map) = maybe_source_map.as_ref() {
-      let mut line_counts = line_counts
-        .iter()
-        .map(|(index, count)| {
-          source_map
-            .tokens()
-            .filter(move |token| token.get_dst_line() as usize == *index)
-            .map(move |token| (token.get_src_line() as usize, *count))
-        })
-        .flatten()
-        .collect::<Vec<(usize, usize)>>();
+  fn done(&mut self) {}
+}
 
-      line_counts.sort_unstable_by_key(|(index, _)| *index);
-      line_counts.dedup_by_key(|(index, _)| *index);
+struct PrettyCoverageReporter {}
 
-      line_counts
-    } else {
-      line_counts
-    };
+impl PrettyCoverageReporter {
+  pub fn new() -> PrettyCoverageReporter {
+    PrettyCoverageReporter {}
+  }
+}
 
-    print!("cover {} ... ", script_coverage.url);
+impl CoverageReporter for PrettyCoverageReporter {
+  fn report(&mut self, coverage_report: &CoverageReport, file_text: &str) {
+    let lines = file_text.split('\n').collect::<Vec<_>>();
+    print!("cover {} ... ", coverage_report.url);
 
-    let hit_lines = line_counts
+    let hit_lines = coverage_report
+      .found_lines
       .iter()
-      .filter(|(_, count)| *count != 0)
+      .filter(|(_, count)| *count > 0)
       .map(|(index, _)| *index);
 
-    let missed_lines = line_counts
+    let missed_lines = coverage_report
+      .found_lines
       .iter()
       .filter(|(_, count)| *count == 0)
       .map(|(index, _)| *index);
 
-    let lines_found = line_counts.len();
+    let lines_found = coverage_report.found_lines.len();
     let lines_hit = hit_lines.count();
     let line_ratio = lines_hit as f32 / lines_found as f32;
 
     let line_coverage =
-      format!("{:.3}% ({}/{})", line_ratio * 100.0, lines_hit, lines_found,);
+      format!("{:.3}% ({}/{})", line_ratio * 100.0, lines_hit, lines_found);
 
     if line_ratio >= 0.9 {
       println!("{}", colors::green(&line_coverage));
@@ -715,12 +662,21 @@ pub async fn cover_files(
       .get_source(&module_specifier)
       .map(|f| f.source);
 
-    reporter.visit_coverage(
+    let coverage_report = generate_coverage_report(
       &script_coverage,
       script_source,
-      maybe_source_map,
-      maybe_cached_source,
+      &maybe_source_map,
     );
+
+    let file_text = if let Some(original_source) =
+      maybe_source_map.and(maybe_cached_source.as_ref())
+    {
+      original_source.as_str()
+    } else {
+      script_source
+    };
+
+    reporter.report(&coverage_report, file_text);
   }
 
   reporter.done();