[source_ref] include the ending line of a log statement (ttiimm#13)

If a log statement is broken up over multiple lines, the
user might refer to any of those lines when setting a
breakpoint.  So, we need to include the ending line
number in the SourceRef.

Files:
* lib.rs: Add a method to get the StatementsInFile
  object from the LogMatcher.  This is needed to
  handle a breakpoint request.  Also, adjust the
  java tree-sitter query so template strings
  are handled using Named placeholders.
* source_hier.rs: Completely ignore some
  directories in the source hierarchy, like version
  control ones.  Add methods to find a file in the
  source hierarchy.
* source_query.rs: Only return results from captures
  after the format string.
* source_ref.rs: Add ending line number.
* test_java.rs: Add a java test that uses slf4j-style
  log statements.

* [source_ref] add (?s) to match multi-line values

Author: tstack

src/lib.rs

@@ -61,7 +61,7 @@ pub enum LogError {
 /// Collection of log statements in a single source file
 #[derive(Debug)]
 pub struct StatementsInFile {
-    pub filename: String,
+    pub path: String,
     id: SourceFileID,
     pub log_statements: Vec<SourceRef>,
     /// A single matcher for all log statements.
@@ -113,14 +113,24 @@ impl LogMatcher {
     }
 
     /// Check if the given path is covered by any of the roots in this matcher.
-    pub fn match_path(&self, path: &Path) -> Option<PathBuf> {
+    pub fn match_path(&self, path: &Path) -> Option<(&PathBuf, &SourceTree)> {
         self.roots
             .iter()
             .filter(|(existing_path, _coll)| path.starts_with(existing_path))
-            .map(|(path, _coll)| path.clone())
             .next()
     }
 
+    pub fn find_source_file_statements(&self, path: &Path) -> Option<&StatementsInFile> {
+        if let Some((_root_path, src_tree)) = self.match_path(path) {
+            src_tree
+                .tree
+                .find_file(path)
+                .and_then(|info| src_tree.files_with_statements.get(&info.id))
+        } else {
+            None
+        }
+    }
+
     /// Traverse the roots looking for supported source files.
     #[must_use]
     pub fn discover_sources(&mut self, tracker: &ProgressTracker) -> Vec<LogError> {
@@ -197,7 +207,7 @@ impl LogMatcher {
                 // XXX this block and the else are basically the same, try to refactor
                 coll.files_with_statements
                     .values()
-                    .filter(|stmts| stmts.filename.contains(filename))
+                    .filter(|stmts| stmts.path.contains(filename))
                     .flat_map(|stmts| {
                         let file_matches = stmts.matcher.matches(body);
                         match file_matches.iter().next() {
@@ -231,7 +241,7 @@ impl LogMatcher {
     }
 }
 
-#[derive(Debug, PartialEq, Copy, Clone, Serialize)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Serialize)]
 pub enum SourceLanguage {
     Rust,
     Java,
@@ -254,6 +264,14 @@ const IDENTS_JAVA: &[&str] = &["logger", "log", "fine", "debug", "info", "warn",
 const IDENTS_CPP: &[&str] = &["debug", "info", "warn", "trace"];
 
 impl SourceLanguage {
+    pub fn as_str(&self) -> &'static str {
+        match self {
+            SourceLanguage::Rust => "Rust",
+            SourceLanguage::Java => "Java",
+            SourceLanguage::Cpp => "C++",
+        }
+    }
+
     fn from_extension(extension: &OsStr) -> Option<Self> {
         match extension.to_str() {
             Some("rs") => Some(Self::Rust),
@@ -288,12 +306,11 @@ impl SourceLanguage {
                     (method_invocation 
                         object: (identifier) @object-name
                         name: (identifier) @method-name
-                        arguments: (argument_list [
-                            (_ (string_literal) @log  (_ (this)? @this (identifier) @arguments))
-                            (_ (string_literal (_ (this)? @this (identifier) @arguments)) @log)
-                            (string_literal) @log (this)? @this (identifier) @arguments
-                            (string_literal) @log (this)? @this
-                        ])
+                        arguments: [
+                            (argument_list (template_expression
+                                template_argument: (string_literal) @arguments))
+                            (argument_list (string_literal) @arguments)
+                        ]
                         (#match? @object-name "log(ger)?|LOG(GER)?")
                         (#match? @method-name "fine|debug|info|warn|trace")
                     )
@@ -490,7 +507,7 @@ pub fn extract_logging(sources: &[CodeSource], tracker: &ProgressTracker) -> Vec
                             matched.push(src_ref);
                         }
                     }
-                    "identifier" | "this" => {
+                    "args" | "this" => {
                         if !matched.is_empty() {
                             let range = result.range;
                             let source = code.buffer.as_str();
@@ -506,6 +523,7 @@ pub fn extract_logging(sources: &[CodeSource], tracker: &ProgressTracker) -> Vec
                             {
                                 let length = matched.len() - 1;
                                 let prior_result: &mut SourceRef = matched.get_mut(length).unwrap();
+                                prior_result.end_line_no = result.range.end_point.row + 1;
                                 prior_result.vars.push(text.trim().to_string());
                             }
                         }
@@ -519,7 +537,7 @@ pub fn extract_logging(sources: &[CodeSource], tracker: &ProgressTracker) -> Vec
                 None
             } else {
                 Some(StatementsInFile {
-                    filename: matched.first().unwrap().source_path.clone(),
+                    path: matched.first().unwrap().source_path.clone(),
                     id: code.info.id,
                     log_statements: matched,
                     matcher: RegexSet::new(patterns).expect("To combine patterns"),

src/snapshots/log2src__source_hier__test__with_resources_dir-3.snap

@@ -7,12 +7,12 @@ expression: new_and_updated_events
     - 1
 - DeletedFile:
     - test_java.rs
-    - 5
+    - 6
 - NewFile:
     - Basic.java
     - language: Java
-      id: 7
+      id: 8
 - NewFile:
     - new.rs
     - language: Rust
-      id: 8
+      id: 9

src/snapshots/log2src__source_hier__test__with_resources_dir-4.snap

@@ -4,13 +4,16 @@ expression: deleted_dir_events
 ---
 - DeletedFile:
     - BasicWithUpper.java
-    - 4
+    - 5
 - DeletedFile:
     - BasicWithLog.java
-    - 3
+    - 4
 - DeletedFile:
     - BasicWithCustom.java
+    - 3
+- DeletedFile:
+    - BasicSlf4j.java
     - 2
 - DeletedFile:
     - Basic.java
-    - 7
+    - 8

src/snapshots/log2src__source_hier__test__with_resources_dir.snap

@@ -5,21 +5,25 @@ expression: events
 - NewFile:
     - test_rust.rs
     - language: Rust
-      id: 6
+      id: 7
 - NewFile:
     - test_java.rs
     - language: Rust
-      id: 5
+      id: 6
 - NewFile:
     - BasicWithUpper.java
     - language: Java
-      id: 4
+      id: 5
 - NewFile:
     - BasicWithLog.java
     - language: Java
-      id: 3
+      id: 4
 - NewFile:
     - BasicWithCustom.java
+    - language: Java
+      id: 3
+- NewFile:
+    - BasicSlf4j.java
     - language: Java
       id: 2
 - NewFile:

src/source_hier.rs

@@ -3,10 +3,14 @@ use serde::Serialize;
 use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::ffi::{OsStr, OsString};
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 use std::time::SystemTime;
 use std::{fs, io};
 
+fn is_ignored_dir(name: &OsStr) -> bool {
+    name == ".git" || name == ".hg" || name == ".svn" || name == ".vscode"
+}
+
 /// Result of a shallow check of a file system path.  Mainly interested in getting a directory
 /// listing without descending into the child trees.
 enum ShallowCheckResult {
@@ -24,7 +28,7 @@ enum ShallowCheckResult {
 pub struct SourceFileID(usize);
 
 /// A summary of a source code file
-#[derive(Copy, Clone, Debug, Serialize)]
+#[derive(Copy, Clone, Debug, Serialize, Eq, PartialEq)]
 pub struct SourceFileInfo {
     pub language: SourceLanguage,
     pub id: SourceFileID,
@@ -80,6 +84,7 @@ impl SourceHierContent {
             Ok(entries) => Self::Directory {
                 entries: entries
                     .into_iter()
+                    .filter(|entry| !is_ignored_dir(&entry.0))
                     .map(|(entry_name, meta)| {
                         (
                             entry_name.to_os_string(),
@@ -210,7 +215,8 @@ impl SourceHierContent {
                     let mut new_entries: Vec<(OsString, Result<fs::Metadata, io::Error>)> =
                         Vec::new();
                     for (name, meta) in latest_entries {
-                        if let Some(existing_entry) = entries.get_mut(&name) {
+                        if is_ignored_dir(&name.as_os_str()) {
+                        } else if let Some(existing_entry) = entries.get_mut(&name) {
                             existing_entry.sync(&path.join(&name), meta, deleted_events)
                         } else {
                             new_entries.push((name, meta));
@@ -229,6 +235,24 @@ impl SourceHierContent {
         };
         true
     }
+
+    pub fn find_file(&self, self_path: &Path, desired_path: &Path) -> Option<SourceFileInfo> {
+        match self {
+            SourceHierContent::File { info, .. } if desired_path == Path::new("") => Some(*info),
+            SourceHierContent::Directory { ref entries } => {
+                let mut components = desired_path.components();
+                if let Some(Component::Normal(name)) = components.next() {
+                    if let Some(node) = entries.get(name) {
+                        return node
+                            .content
+                            .find_file(&self_path.join(name), components.as_path());
+                    }
+                }
+                None
+            }
+            _ => None,
+        }
+    }
 }
 
 /// A node in the SourceHierTree.  It contains information that is common to all types of content
@@ -432,6 +456,13 @@ impl SourceHierTree {
         }
     }
 
+    pub fn find_file(&self, path: &Path) -> Option<SourceFileInfo> {
+        match path.strip_prefix(&self.root_path) {
+            Ok(sub_path) => self.root_node.content.find_file(&self.root_path, sub_path),
+            Err(_) => None,
+        }
+    }
+
     /// Visit every node in the hierarchy, depth-first, calling `f` on each.
     pub fn visit<F>(&self, mut f: F)
     where
@@ -468,17 +499,16 @@ impl SourceHierTree {
 
 #[cfg(test)]
 mod test {
-    use crate::source_hier::{ScanEvent, SourceHierTree};
+    use crate::source_hier::{ScanEvent, SourceFileID, SourceFileInfo, SourceHierTree};
+    use crate::SourceLanguage;
     use fs_extra::dir::copy;
     use fs_extra::dir::CopyOptions;
     use insta::assert_yaml_snapshot;
     use std::fs;
     use std::fs::File;
     use std::io::Write;
-    use std::ops::Sub;
     use std::path::Path;
     use std::path::PathBuf;
-    use std::time::{Duration, SystemTime};
     use tempfile::{tempdir, TempDir};
 
     fn setup_test_environment(source_dir: &Path) -> TempDir {
@@ -511,6 +541,10 @@ mod test {
     fn test_with_resources_dir() {
         let tests_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests");
         let temp_test_dir = setup_test_environment(&tests_path);
+        let _ = fs::create_dir(temp_test_dir.path().join(".git")).unwrap();
+        let _ = File::create_new(temp_test_dir.path().join(".git/config"))
+            .unwrap().write("abc".as_bytes())
+            .unwrap();
         let basic_path = temp_test_dir.path().join("tests/java/Basic.java");
         {
             let metadata = fs::metadata(&basic_path).unwrap();
@@ -522,6 +556,14 @@ mod test {
         tree.sync();
         let events: Vec<ScanEvent> = tree.scan().map(redact_event).collect();
         assert_yaml_snapshot!(events);
+        let find_res = tree.find_file(&basic_path);
+        assert_eq!(
+            find_res,
+            Some(SourceFileInfo {
+                language: SourceLanguage::Java,
+                id: SourceFileID(1)
+            })
+        );
         let no_events: Vec<ScanEvent> = tree.scan().map(redact_event).collect();
         assert_yaml_snapshot!(no_events);
         let _ = fs::remove_file(temp_test_dir.path().join("tests/test_java.rs")).unwrap();

src/source_query.rs

@@ -1,5 +1,4 @@
 use std::ops::Range;
-
 use tree_sitter::{
     Language, Node, Parser, Point, Query, QueryCursor, Range as TSRange, StreamingIterator, Tree,
 };
@@ -42,8 +41,16 @@ impl<'a> SourceQuery<'a> {
         let mut results = Vec::new();
         let matches = cursor.matches(&query, self.tree.root_node(), self.source.as_bytes());
         matches.for_each(|m| {
+            let mut got_string_literal = false;
             for capture in m.captures {
                 let mut child = capture.node;
+                if child.kind() == "string_literal" {
+                    // only return results after the format string literal, other captures
+                    // are not relevant.
+                    got_string_literal = true;
+                } else if !got_string_literal {
+                    continue;
+                }
                 let mut arg_start: Option<(usize, Point)> = None;
 
                 if filter_idx.is_none() || filter_idx.is_some_and(|f| f == capture.index) {
@@ -52,24 +59,24 @@ impl<'a> SourceQuery<'a> {
                         range: capture.node.range(),
                         name_range: Self::find_fn_range(child),
                     });
-                }
-                while let Some(next_child) = child.next_sibling() {
-                    if matches!(next_child.kind(), "," | ")") {
-                        if let Some(start) = arg_start {
-                            results.push(QueryResult {
-                                kind: "identifier".to_string(),
-                                range: TSRange {
-                                    start_byte: start.0,
-                                    start_point: start.1,
-                                    end_byte: next_child.start_byte(),
-                                    end_point: next_child.start_position(),
-                                },
-                                name_range: Self::find_fn_range(child),
-                            });
+                    while let Some(next_child) = child.next_sibling() {
+                        if matches!(next_child.kind(), "," | ")") {
+                            if let Some(start) = arg_start {
+                                results.push(QueryResult {
+                                    kind: "args".to_string(),
+                                    range: TSRange {
+                                        start_byte: start.0,
+                                        start_point: start.1,
+                                        end_byte: next_child.start_byte(),
+                                        end_point: next_child.start_position(),
+                                    },
+                                    name_range: Self::find_fn_range(child),
+                                });
+                            }
+                            arg_start = Some((next_child.end_byte(), next_child.end_position()));
                         }
-                        arg_start = Some((next_child.end_byte(), next_child.end_position()));
+                        child = next_child;
                     }
-                    child = next_child;
                 }
             }
         });