Skip to content

Commit 097635b

Browse files
tstackttiimm
authored andcommitted
[src_ref] add quality level to generated matcher
If multiple regexes match a message, we should prefer the one with the most non-whitespace surrounding text. For example, between "Hello, {name}!" and "{salutation}, {name}!", we should prefer the "Hello" version when matched since that is more specific. Files: * lib.rs: Sort the matched SourceRefs by their quality. Refine the rust query to ignore some macros, allow for qualified macro names, and capture only the first literal since a literal arg might be used when there's an alignment (e.g. `log::debug!("foo {:16} {}", "bar", baz)`). * source_query.rs: Only return a sibling result if it is non-zero in length. * source_ref.rs: Add quality field.
1 parent 0c188bb commit 097635b

12 files changed

+220
-84
lines changed

src/lib.rs

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ mod source_query;
2121
mod source_ref;
2222

2323
// TODO: doesn't need to be exposed if we can clean up the arguments to do_mapping
24+
use crate::progress::WorkGuard;
2425
use crate::source_hier::{ScanEvent, SourceFileID, SourceHierContent, SourceHierTree};
2526
use crate::source_ref::FormatArgument;
2627
pub use code_source::CodeSource;
@@ -31,7 +32,6 @@ pub use progress::WorkInfo;
3132
use source_query::QueryResult;
3233
pub use source_query::SourceQuery;
3334
pub use source_ref::SourceRef;
34-
use crate::progress::WorkGuard;
3535

3636
#[derive(Error, Debug, Diagnostic, Clone)]
3737
pub enum LogError {
@@ -231,7 +231,11 @@ impl LogMatcher {
231231
})
232232
.collect::<Vec<&SourceRef>>()
233233
};
234-
if let Some(src_ref) = matches.first() {
234+
if let Some(src_ref) = matches
235+
.iter()
236+
.sorted_by(|lhs, rhs| rhs.quality.cmp(&lhs.quality))
237+
.next()
238+
{
235239
let variables = extract_variables(log_ref, src_ref);
236240
return Some(LogMapping {
237241
log_ref: log_ref.clone(),
@@ -297,10 +301,11 @@ impl SourceLanguage {
297301
SourceLanguage::Rust => {
298302
// XXX: assumes it's a debug macro
299303
r#"
300-
(macro_invocation macro: (identifier)
301-
(token_tree
304+
(macro_invocation macro: (_) @macro-name
305+
(token_tree .
302306
(string_literal) @log
303307
)
308+
(#not-any-of? @macro-name "format" "vec")
304309
)
305310
"#
306311
}
@@ -419,6 +424,7 @@ impl<'a> LogRef<'a> {
419424
pub fn link_to_source<'a>(log_ref: &LogRef, src_refs: &'a [SourceRef]) -> Option<&'a SourceRef> {
420425
src_refs
421426
.iter()
427+
.sorted_by(|lhs, rhs| rhs.quality.cmp(&lhs.quality))
422428
.find(|&source_ref| source_ref.captures(log_ref.body()).is_some())
423429
}
424430

@@ -447,9 +453,8 @@ pub fn extract_variables<'a>(log_ref: &LogRef<'a>, src_ref: &'a SourceRef) -> Ve
447453
None => log_ref.line,
448454
};
449455
if let Some(captures) = src_ref.captures(line) {
450-
for (index, (cap, placeholder)) in
451-
std::iter::zip(captures.iter().skip(1), src_ref.args.iter()).enumerate()
452-
{
456+
let mut placeholder_index = 0;
457+
for (cap, placeholder) in std::iter::zip(captures.iter().skip(1), src_ref.args.iter()) {
453458
let expr = match placeholder {
454459
FormatArgument::Named(name) => name.clone(),
455460
FormatArgument::Positional(pos) => src_ref
@@ -458,7 +463,12 @@ pub fn extract_variables<'a>(log_ref: &LogRef<'a>, src_ref: &'a SourceRef) -> Ve
458463
.map(|s| s.as_str())
459464
.unwrap_or("<unknown>")
460465
.to_string(),
461-
FormatArgument::Placeholder => src_ref.vars[index].to_string(),
466+
FormatArgument::Placeholder => {
467+
let res = src_ref.vars[placeholder_index].to_string();
468+
469+
placeholder_index += 1;
470+
res
471+
}
462472
};
463473
variables.push(VariablePair {
464474
expr,
@@ -557,6 +567,7 @@ pub fn extract_logging(sources: &[CodeSource], tracker: &ProgressTracker) -> Vec
557567
#[cfg(test)]
558568
mod tests {
559569
use super::*;
570+
use insta::assert_yaml_snapshot;
560571
use std::ptr;
561572

562573
#[test]
@@ -622,12 +633,21 @@ fn foo(i: u32) {
622633
}
623634
624635
fn nope(i: u32, j: i32) {
625-
debug!("this won't match i={}; j={}", i, j);
636+
log::debug!("this won't match i={}; j={}", i, j);
637+
}
638+
639+
fn namedarg0(salutation: &str, name: &str) {
640+
debug!("{salutation}, {name}!"); // lower quality than the next one
626641
}
627642
628643
fn namedarg(name: &str) {
644+
let msg = format!("Goodbye, {name}!");
629645
debug!("Hello, {name}!");
630646
}
647+
648+
fn namedarg2(salutation: &str, name: &str) {
649+
debug!("{salutation}, {name}!"); // lower quality than the previous one
650+
}
631651
"#;
632652

633653
#[test]
@@ -637,20 +657,7 @@ fn namedarg(name: &str) {
637657
.pop()
638658
.unwrap()
639659
.log_statements;
640-
assert_eq!(src_refs.len(), 3);
641-
let first = &src_refs[0];
642-
assert_eq!(first.line_no, 7);
643-
assert_eq!(first.column, 11);
644-
assert_eq!(first.name, "main");
645-
assert_eq!(first.text, "\"you're only as funky as your last cut\"");
646-
assert!(first.vars.is_empty());
647-
648-
let second = &src_refs[1];
649-
assert_eq!(second.line_no, 18);
650-
assert_eq!(second.column, 11);
651-
assert_eq!(second.name, "nope");
652-
assert_eq!(second.text, "\"this won't match i={}; j={}\"");
653-
assert_eq!(second.vars[0], "i");
660+
assert_yaml_snapshot!(src_refs);
654661
}
655662

656663
#[test]
@@ -667,11 +674,26 @@ fn namedarg(name: &str) {
667674
.pop()
668675
.unwrap()
669676
.log_statements;
670-
assert_eq!(src_refs.len(), 3);
677+
assert_eq!(src_refs.len(), 5);
671678
let result = link_to_source(&log_ref, &src_refs);
672679
assert!(ptr::eq(result.unwrap(), &src_refs[0]));
673680
}
674681

682+
#[test]
683+
fn test_link_to_quality_source() {
684+
let lf = LogFormat::new(
685+
r#"^\[\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z \w+ \w+\]\s+(?<body>.*)"#.to_string(),
686+
);
687+
let log_ref = LogRef::with_format("[2024-05-09T19:58:53Z DEBUG main] Hello, Leander!", lf);
688+
let code = CodeSource::from_string(&Path::new("in-mem.rs"), TEST_SOURCE);
689+
let src_refs = extract_logging(&[code], &ProgressTracker::new())
690+
.pop()
691+
.unwrap()
692+
.log_statements;
693+
let result = link_to_source(&log_ref, &src_refs);
694+
assert_yaml_snapshot!(result);
695+
}
696+
675697
const MULTILINE_SOURCE: &str = r#"
676698
#[macro_use]
677699
extern crate log;
@@ -717,7 +739,7 @@ fn main() {
717739
.pop()
718740
.unwrap()
719741
.log_statements;
720-
assert_eq!(src_refs.len(), 3);
742+
assert_eq!(src_refs.len(), 5);
721743
let result = link_to_source(&log_ref, &src_refs);
722744
assert!(result.is_none());
723745
}
@@ -730,7 +752,7 @@ fn main() {
730752
.pop()
731753
.unwrap()
732754
.log_statements;
733-
assert_eq!(src_refs.len(), 3);
755+
assert_eq!(src_refs.len(), 5);
734756
let vars = extract_variables(&log_ref, &src_refs[1]);
735757
assert_eq!(
736758
vars,
@@ -755,8 +777,8 @@ fn main() {
755777
.pop()
756778
.unwrap()
757779
.log_statements;
758-
assert_eq!(src_refs.len(), 3);
759-
let vars = extract_variables(&log_ref, &src_refs[2]);
780+
assert_eq!(src_refs.len(), 5);
781+
let vars = extract_variables(&log_ref, &src_refs[3]);
760782
assert_eq!(
761783
vars,
762784
vec![VariablePair {
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
source: src/lib.rs
3+
expression: src_refs
4+
---
5+
- sourcePath: in-mem.rs
6+
language: Rust
7+
lineNumber: 7
8+
endLineNumber: 7
9+
column: 11
10+
name: main
11+
text: "\"you're only as funky as your last cut\""
12+
quality: 30
13+
pattern: "(?s)^you're only as funky as your last cut$"
14+
args: []
15+
vars: []
16+
- sourcePath: in-mem.rs
17+
language: Rust
18+
lineNumber: 18
19+
endLineNumber: 18
20+
column: 16
21+
name: nope
22+
text: "\"this won't match i={}; j={}\""
23+
quality: 19
24+
pattern: "(?s)^this won't match i=(.+); j=(.+)$"
25+
args:
26+
- Placeholder
27+
- Placeholder
28+
vars:
29+
- i
30+
- j
31+
- sourcePath: in-mem.rs
32+
language: Rust
33+
lineNumber: 22
34+
endLineNumber: 22
35+
column: 11
36+
name: namedarg0
37+
text: "\"{salutation}, {name}!\""
38+
quality: 2
39+
pattern: "(?s)^(.+), (.+)!$"
40+
args:
41+
- Named: salutation
42+
- Named: name
43+
vars: []
44+
- sourcePath: in-mem.rs
45+
language: Rust
46+
lineNumber: 27
47+
endLineNumber: 27
48+
column: 11
49+
name: namedarg
50+
text: "\"Hello, {name}!\""
51+
quality: 7
52+
pattern: "(?s)^Hello, (.+)!$"
53+
args:
54+
- Named: name
55+
vars: []
56+
- sourcePath: in-mem.rs
57+
language: Rust
58+
lineNumber: 31
59+
endLineNumber: 31
60+
column: 11
61+
name: namedarg2
62+
text: "\"{salutation}, {name}!\""
63+
quality: 2
64+
pattern: "(?s)^(.+), (.+)!$"
65+
args:
66+
- Named: salutation
67+
- Named: name
68+
vars: []
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
source: src/lib.rs
3+
expression: result
4+
---
5+
sourcePath: in-mem.rs
6+
language: Rust
7+
lineNumber: 27
8+
endLineNumber: 27
9+
column: 11
10+
name: namedarg
11+
text: "\"Hello, {name}!\""
12+
quality: 7
13+
pattern: "(?s)^Hello, (.+)!$"
14+
args:
15+
- Named: name
16+
vars: []

src/source_query.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,18 @@ impl<'a> SourceQuery<'a> {
6262
while let Some(next_child) = child.next_sibling() {
6363
if matches!(next_child.kind(), "," | ")") {
6464
if let Some(start) = arg_start {
65-
results.push(QueryResult {
66-
kind: "args".to_string(),
67-
range: TSRange {
68-
start_byte: start.0,
69-
start_point: start.1,
70-
end_byte: next_child.start_byte(),
71-
end_point: next_child.start_position(),
72-
},
73-
name_range: Self::find_fn_range(child),
74-
});
65+
if start.0 < next_child.start_byte() {
66+
results.push(QueryResult {
67+
kind: "args".to_string(),
68+
range: TSRange {
69+
start_byte: start.0,
70+
start_point: start.1,
71+
end_byte: next_child.start_byte(),
72+
end_point: next_child.start_position(),
73+
},
74+
name_range: Self::find_fn_range(child),
75+
});
76+
}
7577
}
7678
arg_start = Some((next_child.end_byte(), next_child.end_position()));
7779
}

0 commit comments

Comments
 (0)