Skip to content

Commit d03fbae

Browse files
committed
refactor: reduce visit_imports complexity in imports.rs
Introduced `ImportScanner` struct to encapsulate traversal context and state. Extracted import processing logic into a dedicated `process_import` method. Updated `ImportsValidator::validate` to use the new scanner. This improves maintainability and readability by reducing function complexity and the number of arguments passed in recursive calls.
1 parent a93b724 commit d03fbae

1 file changed

Lines changed: 131 additions & 107 deletions

File tree

crates/agnix-core/src/rules/imports.rs

Lines changed: 131 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -181,20 +181,19 @@ impl Validator for ImportsValidator {
181181
local_cache.entry(root_path.clone()).or_insert(root_imports);
182182
}
183183

184-
visit_imports(
185-
&root_path,
186-
None,
187-
shared_cache,
188-
&mut local_cache,
189-
&mut visited_depth,
190-
&mut stack,
191-
&mut diagnostics,
192-
&mut seen_diagnostics,
184+
let mut scanner = ImportScanner {
193185
config,
194-
is_claude_md,
195-
&project_root,
196-
fs.as_ref(),
197-
);
186+
fs: fs.as_ref(),
187+
project_root: &project_root,
188+
root_is_claude_md: is_claude_md,
189+
shared_cache,
190+
local_cache: &mut local_cache,
191+
visited_depth: &mut visited_depth,
192+
stack: &mut stack,
193+
diagnostics: &mut diagnostics,
194+
seen_diagnostics: &mut seen_diagnostics,
195+
};
196+
scanner.visit(&root_path, None);
198197

199198
// Validate markdown links (REF-002)
200199
// Only check agent config files, not generic markdown. Generic markdown
@@ -220,66 +219,105 @@ impl Validator for ImportsValidator {
220219
}
221220
}
222221

223-
#[allow(clippy::too_many_arguments)]
224-
fn visit_imports(
225-
file_path: &PathBuf,
226-
content_override: Option<&str>,
227-
shared_cache: Option<&ImportCache>,
228-
local_cache: &mut HashMap<PathBuf, Vec<Import>>,
229-
visited_depth: &mut HashMap<PathBuf, usize>,
230-
stack: &mut Vec<PathBuf>,
231-
diagnostics: &mut Vec<Diagnostic>,
232-
seen_diagnostics: &mut HashSet<DiagnosticKey>,
233-
config: &LintConfig,
222+
struct ImportScanner<'a> {
223+
config: &'a LintConfig,
224+
fs: &'a dyn FileSystem,
225+
project_root: &'a Path,
234226
root_is_claude_md: bool,
235-
project_root: &Path,
236-
fs: &dyn FileSystem,
237-
) {
238-
let depth = stack.len();
239-
if let Some(prev_depth) = visited_depth.get(file_path) {
240-
// Skip only when we have already visited this file at an equal or
241-
// shallower depth. If we discover a shallower path later, revisit it
242-
// so traversal can continue with the tighter depth budget.
243-
if *prev_depth <= depth {
244-
return;
245-
}
246-
}
247-
visited_depth.insert(file_path.clone(), depth);
227+
shared_cache: Option<&'a ImportCache>,
228+
local_cache: &'a mut HashMap<PathBuf, Vec<Import>>,
229+
visited_depth: &'a mut HashMap<PathBuf, usize>,
230+
stack: &'a mut Vec<PathBuf>,
231+
diagnostics: &'a mut Vec<Diagnostic>,
232+
seen_diagnostics: &'a mut HashSet<DiagnosticKey>,
233+
}
248234

249-
let imports = get_imports_for_file(file_path, content_override, shared_cache, local_cache, fs);
250-
let Some(imports) = imports else { return };
235+
impl<'a> ImportScanner<'a> {
236+
fn visit(&mut self, file_path: &PathBuf, content_override: Option<&str>) {
237+
let depth = self.stack.len();
238+
if let Some(prev_depth) = self.visited_depth.get(file_path) {
239+
// Skip only when we have already visited this file at an equal or
240+
// shallower depth. If we discover a shallower path later, revisit it
241+
// so traversal can continue with the tighter depth budget.
242+
if *prev_depth <= depth {
243+
return;
244+
}
245+
}
246+
self.visited_depth.insert(file_path.clone(), depth);
247+
248+
let imports = get_imports_for_file(
249+
file_path,
250+
content_override,
251+
self.shared_cache,
252+
self.local_cache,
253+
self.fs,
254+
);
255+
let Some(imports) = imports else { return };
251256

252-
let base_dir = file_path.parent().unwrap_or(Path::new("."));
253-
let normalized_base = normalize_existing_path(base_dir, fs);
254-
let normalized_root = project_root;
257+
let base_dir = file_path.parent().unwrap_or(Path::new("."));
258+
let normalized_base = normalize_existing_path(base_dir, self.fs);
255259

256-
// Determine file type for current file to route its own diagnostics
257-
let filename = file_path.file_name().and_then(|n| n.to_str()).unwrap_or("");
258-
let is_claude_md = matches!(filename, "CLAUDE.md" | "CLAUDE.local.md");
260+
// Determine file type for current file to route its own diagnostics
261+
let filename = file_path.file_name().and_then(|n| n.to_str()).unwrap_or("");
262+
let is_claude_md = matches!(filename, "CLAUDE.md" | "CLAUDE.local.md");
259263

260-
// Check rules based on CURRENT file type for missing imports
261-
// Check rules based on ROOT file type for cycles/depth (applies to entire chain)
262-
let check_not_found = (is_claude_md && config.is_rule_enabled("CC-MEM-001"))
263-
|| (!is_claude_md && config.is_rule_enabled("REF-001"));
264-
let check_cycle = root_is_claude_md && config.is_rule_enabled("CC-MEM-002");
265-
let check_depth = root_is_claude_md && config.is_rule_enabled("CC-MEM-003");
264+
// Check rules based on CURRENT file type for missing imports
265+
// Check rules based on ROOT file type for cycles/depth (applies to entire chain)
266+
let check_not_found = (is_claude_md && self.config.is_rule_enabled("CC-MEM-001"))
267+
|| (!is_claude_md && self.config.is_rule_enabled("REF-001"));
268+
let check_cycle = self.root_is_claude_md && self.config.is_rule_enabled("CC-MEM-002");
269+
let check_depth = self.root_is_claude_md && self.config.is_rule_enabled("CC-MEM-003");
266270

267-
if !(check_not_found || check_cycle || check_depth) {
268-
return;
269-
}
270-
271-
let rule_not_found = if is_claude_md {
272-
"CC-MEM-001"
273-
} else {
274-
"REF-001"
275-
};
276-
let rule_cycle = "CC-MEM-002";
277-
let rule_depth = "CC-MEM-003";
271+
if !(check_not_found || check_cycle || check_depth) {
272+
return;
273+
}
278274

279-
stack.push(file_path.clone());
275+
let rule_not_found = if is_claude_md {
276+
"CC-MEM-001"
277+
} else {
278+
"REF-001"
279+
};
280+
let rule_cycle = "CC-MEM-002";
281+
let rule_depth = "CC-MEM-003";
282+
283+
self.stack.push(file_path.clone());
284+
285+
for import in imports {
286+
self.process_import(
287+
import,
288+
file_path,
289+
base_dir,
290+
&normalized_base,
291+
rule_not_found,
292+
rule_cycle,
293+
rule_depth,
294+
check_not_found,
295+
check_cycle,
296+
check_depth,
297+
depth,
298+
);
299+
}
280300

281-
for import in imports {
301+
self.stack.pop();
302+
}
303+
304+
#[allow(clippy::too_many_arguments)]
305+
fn process_import(
306+
&mut self,
307+
import: &Import,
308+
file_path: &PathBuf,
309+
base_dir: &Path,
310+
normalized_base: &PathBuf,
311+
rule_not_found: &str,
312+
rule_cycle: &str,
313+
rule_depth: &str,
314+
check_not_found: bool,
315+
check_cycle: bool,
316+
check_depth: bool,
317+
depth: usize,
318+
) {
282319
let resolved = resolve_import_path(&import.path, base_dir);
320+
let normalized_root = self.project_root;
283321

284322
// Validate path to prevent traversal attacks
285323
// Reject absolute paths and paths that escape the project root
@@ -291,8 +329,8 @@ fn visit_imports(
291329
{
292330
if check_not_found {
293331
push_unique_diagnostic(
294-
diagnostics,
295-
seen_diagnostics,
332+
self.diagnostics,
333+
self.seen_diagnostics,
296334
Diagnostic::error(
297335
file_path.clone(),
298336
import.line,
@@ -303,15 +341,15 @@ fn visit_imports(
303341
.with_suggestion(t!("rules.cc_mem_001.absolute_suggestion")),
304342
);
305343
}
306-
continue;
344+
return;
307345
}
308346

309-
let normalized_resolved = normalize_join(&normalized_base, &import.path);
347+
let normalized_resolved = normalize_join(normalized_base, &import.path);
310348
if !normalized_resolved.starts_with(normalized_root) {
311349
if check_not_found {
312350
push_unique_diagnostic(
313-
diagnostics,
314-
seen_diagnostics,
351+
self.diagnostics,
352+
self.seen_diagnostics,
315353
Diagnostic::error(
316354
file_path.clone(),
317355
import.line,
@@ -322,16 +360,16 @@ fn visit_imports(
322360
.with_suggestion(t!("rules.cc_mem_001.escapes_suggestion")),
323361
);
324362
}
325-
continue;
363+
return;
326364
}
327365

328-
let normalized = if fs.exists(&resolved) {
329-
let canonical_resolved = normalize_existing_path(&resolved, fs);
366+
let normalized = if self.fs.exists(&resolved) {
367+
let canonical_resolved = normalize_existing_path(&resolved, self.fs);
330368
if !canonical_resolved.starts_with(normalized_root) {
331369
if check_not_found {
332370
push_unique_diagnostic(
333-
diagnostics,
334-
seen_diagnostics,
371+
self.diagnostics,
372+
self.seen_diagnostics,
335373
Diagnostic::error(
336374
file_path.clone(),
337375
import.line,
@@ -342,7 +380,7 @@ fn visit_imports(
342380
.with_suggestion(t!("rules.cc_mem_001.escapes_suggestion")),
343381
);
344382
}
345-
continue;
383+
return;
346384
}
347385
canonical_resolved
348386
} else {
@@ -352,25 +390,25 @@ fn visit_imports(
352390
// Try file-relative resolution first, then project-root resolution.
353391
// Claude Code resolves @imports relative to the project root, not
354392
// the importing file's directory.
355-
let normalized = if fs.exists(&normalized) {
393+
let normalized = if self.fs.exists(&normalized) {
356394
normalized
357395
} else {
358396
// Fallback: try resolving relative to project root
359-
let root_resolved = project_root.join(&import.path);
360-
if fs.exists(&root_resolved) {
397+
let root_resolved = self.project_root.join(&import.path);
398+
if self.fs.exists(&root_resolved) {
361399
root_resolved
362400
} else {
363401
normalized
364402
}
365403
};
366404

367-
let import_exists = fs.exists(&normalized);
405+
let import_exists = self.fs.exists(&normalized);
368406

369407
if !import_exists {
370408
if check_not_found {
371409
push_unique_diagnostic(
372-
diagnostics,
373-
seen_diagnostics,
410+
self.diagnostics,
411+
self.seen_diagnostics,
374412
Diagnostic::error(
375413
file_path.clone(),
376414
import.line,
@@ -384,19 +422,19 @@ fn visit_imports(
384422
)),
385423
);
386424
}
387-
continue;
425+
return;
388426
}
389427

390428
// Always check for cycles/depth to prevent infinite recursion
391-
let has_cycle = stack.contains(&normalized);
429+
let has_cycle = self.stack.contains(&normalized);
392430
let exceeds_depth = depth + 1 > MAX_IMPORT_DEPTH;
393431

394432
// Emit diagnostics if rules are enabled for this file type
395433
if check_cycle && has_cycle {
396-
let cycle = format_cycle(stack, &normalized);
434+
let cycle = format_cycle(self.stack, &normalized);
397435
push_unique_diagnostic(
398-
diagnostics,
399-
seen_diagnostics,
436+
self.diagnostics,
437+
self.seen_diagnostics,
400438
Diagnostic::error(
401439
file_path.clone(),
402440
import.line,
@@ -406,13 +444,13 @@ fn visit_imports(
406444
)
407445
.with_suggestion(t!("rules.cc_mem_002.suggestion")),
408446
);
409-
continue;
447+
return;
410448
}
411449

412450
if check_depth && exceeds_depth {
413451
push_unique_diagnostic(
414-
diagnostics,
415-
seen_diagnostics,
452+
self.diagnostics,
453+
self.seen_diagnostics,
416454
Diagnostic::error(
417455
file_path.clone(),
418456
import.line,
@@ -426,31 +464,17 @@ fn visit_imports(
426464
)
427465
.with_suggestion(t!("rules.cc_mem_003.suggestion")),
428466
);
429-
continue;
467+
return;
430468
}
431469

432470
// Only recurse if no cycle/depth issues
433471
if !has_cycle && !exceeds_depth {
434-
visit_imports(
435-
&normalized,
436-
None,
437-
shared_cache,
438-
local_cache,
439-
visited_depth,
440-
stack,
441-
diagnostics,
442-
seen_diagnostics,
443-
config,
444-
root_is_claude_md,
445-
project_root,
446-
fs,
447-
);
472+
self.visit(&normalized, None);
448473
}
449474
}
450-
451-
stack.pop();
452475
}
453476

477+
454478
/// Get imports for a file, using shared cache if available, otherwise local cache.
455479
///
456480
/// This function uses a read-then-write lock pattern for the shared cache:

0 commit comments

Comments
 (0)