codelens_engine/
redundant_definitions.rs

1//! Detects "thin wrapper" definitions — functions whose entire body is a
2//! single call to another function with one literal default argument
3//! (e.g. `pub fn record_X(&self) { self.record_X_for_session(None) }`).
4//!
5//! This is a syntactic-only detector that catches the exact pattern flagged
6//! by self-dogfooding in v1.13.0 Phase 1-A: 16 `record_*` wrappers all
7//! forwarding to their `_for_session(None)` substrate. The substrate could
8//! be flagged by `find_dead_code_v2` only AFTER the wrappers were removed,
9//! so this complement helps surface the cluster pre-deletion.
10
11use crate::project::{collect_files, ProjectRoot};
12use anyhow::Result;
13use regex::Regex;
14use serde::Serialize;
15use std::path::Path;
16use std::sync::LazyLock;
17
18/// Matches a Rust one-line wrapper:
19///   `pub fn NAME(args) [-> RetType] { self.OTHER(args, LITERAL) [;] }`
20///   `fn NAME(args) [-> RetType] { OTHER(args, LITERAL) [;] }`
21/// where LITERAL is one of: `None`, `Default::default()`, `false`, `true`,
22/// a bare integer literal, or a quoted string literal.
23static RUST_ONE_LINE_WRAPPER_RE: LazyLock<Regex> = LazyLock::new(|| {
24    // (?m): multiline (^/$ are line anchors).
25    // Inner args use [^){};|] to reject:
26    //   - `;` (statement boundary — would mean multi-statement body)
27    //   - `{` `}` (block — closure body, struct literal)
28    //   - `|` (closure pipe — `|x| ...` indicates non-trivial logic, not a literal default)
29    Regex::new(
30        r#"(?m)^\s*(?:pub(?:\([^)]*\))?\s+)?fn\s+(?P<wrapper>[A-Za-z_][A-Za-z0-9_]*)\s*\([^)]*\)\s*(?:->\s*[^{]+?)?\s*\{\s*(?:self\.|Self::)?(?P<target>[A-Za-z_][A-Za-z0-9_]*)\s*\([^){};|]*?(?P<default>None|Default::default\(\)|true|false|-?\d+|"[^"]*")?\s*\)\s*;?\s*\}\s*$"#
31    ).unwrap()
32});
33
34#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
35pub struct RedundantDefinitionEntry {
36    pub file: String,
37    pub wrapper: String,
38    pub target: String,
39    pub line: usize,
40    pub default_arg: Option<String>,
41    pub kind: &'static str,
42}
43
44/// Finds Rust one-line wrappers in the project.
45///
46/// Returns a list of (wrapper, target) pairs. Callers can group by `target`
47/// to find substrates with multiple wrappers — the highest-leverage cleanup
48/// opportunity per Phase 1-A's findings.
49pub fn find_redundant_definitions(
50    project: &ProjectRoot,
51    max_results: usize,
52) -> Result<Vec<RedundantDefinitionEntry>> {
53    let mut results: Vec<RedundantDefinitionEntry> = Vec::new();
54    let candidates = collect_files(project.as_path(), is_rust_file)?;
55
56    for path in &candidates {
57        let source = match std::fs::read_to_string(path) {
58            Ok(s) => s,
59            Err(_) => continue,
60        };
61        let relative = project.to_relative(path);
62        if is_test_file(&relative) {
63            continue;
64        }
65        let production_source = strip_cfg_test_modules(&source);
66        scan_rust_source(&production_source, &relative, &mut results);
67        if max_results > 0 && results.len() >= max_results {
68            break;
69        }
70    }
71
72    results.sort_by(|a, b| {
73        a.target
74            .cmp(&b.target)
75            .then(a.file.cmp(&b.file))
76            .then(a.line.cmp(&b.line))
77    });
78    if max_results > 0 && results.len() > max_results {
79        results.truncate(max_results);
80    }
81    Ok(results)
82}
83
84fn scan_rust_source(source: &str, file: &str, out: &mut Vec<RedundantDefinitionEntry>) {
85    for m in RUST_ONE_LINE_WRAPPER_RE.captures_iter(source) {
86        let wrapper = m
87            .name("wrapper")
88            .map(|m| m.as_str().to_owned())
89            .unwrap_or_default();
90        let target = m
91            .name("target")
92            .map(|m| m.as_str().to_owned())
93            .unwrap_or_default();
94        if wrapper.is_empty() || target.is_empty() || wrapper == target {
95            continue;
96        }
97        let default_arg = m.name("default").map(|m| m.as_str().to_owned());
98        let line = byte_offset_to_line(source, m.get(0).map(|m| m.start()).unwrap_or(0));
99        out.push(RedundantDefinitionEntry {
100            file: file.to_owned(),
101            wrapper,
102            target,
103            line,
104            default_arg,
105            kind: "rust_one_line_wrapper",
106        });
107    }
108}
109
110fn byte_offset_to_line(source: &str, offset: usize) -> usize {
111    source[..offset.min(source.len())].matches('\n').count() + 1
112}
113
114fn is_rust_file(path: &Path) -> bool {
115    path.extension().and_then(|s| s.to_str()) == Some("rs")
116}
117
118/// Skip canonical test/example/bench harness files. Heuristic: any path
119/// segment named `tests`, `bench`, `benches`, `examples`, or any file
120/// ending in `_tests.rs`/`_test.rs`. Self-detector module is also
121/// skipped to avoid flagging its own fixture strings.
122fn is_test_file(relative: &str) -> bool {
123    if relative == "crates/codelens-engine/src/redundant_definitions.rs" {
124        return true;
125    }
126    let lower = relative.to_ascii_lowercase();
127    if lower.ends_with("_tests.rs") || lower.ends_with("_test.rs") {
128        return true;
129    }
130    lower.split('/').any(|seg| {
131        matches!(
132            seg,
133            "tests" | "test" | "bench" | "benches" | "examples" | "fixtures"
134        )
135    })
136}
137
138/// Removes `#[cfg(test)] mod ... { ... }` blocks (matched by regex with
139/// nested braces handled by depth counting). This filters out test-only
140/// helper functions that share the wrapper shape but are not production
141/// code paths.
142fn strip_cfg_test_modules(source: &str) -> String {
143    let mut out = String::with_capacity(source.len());
144    let mut chars = source.char_indices().peekable();
145    while let Some(&(idx, ch)) = chars.peek() {
146        if ch == '#' {
147            // try to match #[cfg(test)] mod NAME {
148            let rest = &source[idx..];
149            if let Some(open_brace_offset) = match_cfg_test_mod_header(rest) {
150                let body_start = idx + open_brace_offset;
151                let body_end = match find_matching_brace(source, body_start) {
152                    Some(e) => e,
153                    None => {
154                        out.push(ch);
155                        chars.next();
156                        continue;
157                    }
158                };
159                // skip over the entire `#[cfg(test)] mod NAME { ... }`
160                while let Some(&(j, _)) = chars.peek() {
161                    if j > body_end {
162                        break;
163                    }
164                    chars.next();
165                }
166                continue;
167            }
168        }
169        out.push(ch);
170        chars.next();
171    }
172    out
173}
174
175/// If `rest` starts with `#[cfg(test)] mod NAME {`, return the byte
176/// offset of the `{`. Otherwise None. Tolerant of attribute whitespace
177/// variants but not of comments inside the attribute.
178fn match_cfg_test_mod_header(rest: &str) -> Option<usize> {
179    static HEADER_RE: LazyLock<Regex> = LazyLock::new(|| {
180        Regex::new(r"^#\s*\[\s*cfg\s*\(\s*test\s*\)\s*\]\s*(?:pub(?:\([^)]*\))?\s+)?mod\s+[A-Za-z_][A-Za-z0-9_]*\s*\{")
181            .unwrap()
182    });
183    HEADER_RE.find(rest).map(|m| m.end() - 1)
184}
185
186/// Returns the byte index of the matching `}` for the `{` at `open_idx`.
187fn find_matching_brace(source: &str, open_idx: usize) -> Option<usize> {
188    let bytes = source.as_bytes();
189    if bytes.get(open_idx) != Some(&b'{') {
190        return None;
191    }
192    let mut depth = 1usize;
193    let mut i = open_idx + 1;
194    while i < bytes.len() {
195        match bytes[i] {
196            b'{' => depth += 1,
197            b'}' => {
198                depth -= 1;
199                if depth == 0 {
200                    return Some(i);
201                }
202            }
203            _ => {}
204        }
205        i += 1;
206    }
207    None
208}
209
210#[cfg(test)]
211mod tests {
212    use super::*;
213
214    #[test]
215    fn detects_self_dot_wrapper_with_none_default() {
216        let source = r#"
217impl Foo {
218    pub fn record_x(&self) { self.record_x_for_session(None) }
219    pub fn record_y(&self) { self.record_y_for_session(None); }
220}
221        "#;
222        let mut out = Vec::new();
223        scan_rust_source(source, "telemetry.rs", &mut out);
224        assert_eq!(out.len(), 2, "got: {:?}", out);
225        assert_eq!(out[0].wrapper, "record_x");
226        assert_eq!(out[0].target, "record_x_for_session");
227        assert_eq!(out[0].default_arg.as_deref(), Some("None"));
228        assert_eq!(out[1].wrapper, "record_y");
229    }
230
231    #[test]
232    fn detects_bare_function_wrapper() {
233        let source = r#"
234pub fn helper(x: u32) -> bool { inner(x, false) }
235        "#;
236        let mut out = Vec::new();
237        scan_rust_source(source, "lib.rs", &mut out);
238        assert_eq!(out.len(), 1);
239        assert_eq!(out[0].wrapper, "helper");
240        assert_eq!(out[0].target, "inner");
241        assert_eq!(out[0].default_arg.as_deref(), Some("false"));
242    }
243
244    #[test]
245    fn skips_call_with_closure_arg() {
246        // Detector v3: a body that passes a closure to its delegate is NOT a thin
247        // wrapper — the closure carries logic. This was the false-positive shape
248        // that flagged `mutate_session_metrics` 13× during v1.13.3 dogfooding.
249        let source = r#"
250pub fn record_x(&self) {
251    self.mutate_session_metrics(None, |session| {
252        session.foo += 1;
253    });
254}
255        "#;
256        let mut out = Vec::new();
257        scan_rust_source(source, "events.rs", &mut out);
258        assert!(out.is_empty(), "closure-arg call must not match: {:?}", out);
259    }
260
261    #[test]
262    fn skips_self_recursive_call() {
263        let source = r#"
264pub fn loop_me(&self) { self.loop_me(0) }
265        "#;
266        let mut out = Vec::new();
267        scan_rust_source(source, "x.rs", &mut out);
268        // wrapper == target → not flagged (would be infinite recursion, not delegation)
269        assert!(out.is_empty(), "got: {:?}", out);
270    }
271
272    #[test]
273    fn strip_cfg_test_modules_removes_test_block() {
274        let source = r#"
275pub fn real_thing(&self) { self.real_thing_inner(None) }
276
277#[cfg(test)]
278mod tests {
279    fn helper() { foo(true) }
280}
281"#;
282        let stripped = super::strip_cfg_test_modules(source);
283        assert!(stripped.contains("real_thing"));
284        assert!(
285            !stripped.contains("foo(true)"),
286            "test mod should be gone: {}",
287            stripped
288        );
289    }
290
291    #[test]
292    fn is_test_file_recognizes_canonical_paths() {
293        assert!(super::is_test_file("crates/foo/tests/something.rs"));
294        assert!(super::is_test_file("crates/foo/src/internals_tests.rs"));
295        assert!(super::is_test_file("benchmarks/bench/runner.rs"));
296        assert!(!super::is_test_file("crates/foo/src/main.rs"));
297        assert!(super::is_test_file(
298            "crates/codelens-engine/src/redundant_definitions.rs"
299        ));
300    }
301
302    #[test]
303    #[ignore]
304    fn dogfood_self_repo() {
305        // Run with: cargo test -p codelens-engine dogfood_self_repo -- --ignored --nocapture
306        let repo = std::env::var("CODELENS_REPO_ROOT")
307            .unwrap_or_else(|_| "/Users/bagjaeseog/codelens-mcp-plugin".to_owned());
308        let project = crate::project::ProjectRoot::new(repo).expect("project root");
309        let results =
310            super::find_redundant_definitions(&project, 200).expect("find_redundant_definitions");
311        eprintln!(
312            "\n=== {} redundant definitions in self (post v2 filtering) ===\n",
313            results.len()
314        );
315        let mut groups: std::collections::BTreeMap<&str, Vec<&RedundantDefinitionEntry>> =
316            std::collections::BTreeMap::new();
317        for r in &results {
318            groups.entry(r.target.as_str()).or_default().push(r);
319        }
320        let mut multi: Vec<_> = groups.iter().filter(|(_, v)| v.len() >= 2).collect();
321        multi.sort_by(|a, b| b.1.len().cmp(&a.1.len()));
322        eprintln!("Multi-wrapper clusters: {}\n", multi.len());
323        for (target, members) in &multi {
324            eprintln!("  {} ← {}", target, members.len());
325            for m in members.iter().take(3) {
326                eprintln!("      {} at {}:{}", m.wrapper, m.file, m.line);
327            }
328            if members.len() > 3 {
329                eprintln!("      ... +{} more", members.len() - 3);
330            }
331        }
332        eprintln!("\nFirst 30 hits:\n");
333        for r in results.iter().take(30) {
334            eprintln!("  {} -> {}  ({}:{})", r.wrapper, r.target, r.file, r.line);
335        }
336    }
337
338    #[test]
339    fn skips_multi_statement_body() {
340        let source = r#"
341pub fn complex(&self) {
342    let x = 1;
343    self.do_thing(x, None);
344}
345        "#;
346        let mut out = Vec::new();
347        scan_rust_source(source, "x.rs", &mut out);
348        assert!(
349            out.is_empty(),
350            "multi-statement should not match: {:?}",
351            out
352        );
353    }
354}
codelens_engine/redundant_definitions.rs

codelens_engine/
redundant_definitions.rs
