rumdl_lib/
fix_coordinator.rs

1use crate::config::Config;
2use crate::lint_context::LintContext;
3use crate::rule::{LintWarning, Rule};
4use std::collections::hash_map::DefaultHasher;
5use std::collections::{HashMap, HashSet};
6use std::hash::{Hash, Hasher};
7
8/// Maximum number of fix iterations before stopping (same as Ruff)
9const MAX_ITERATIONS: usize = 100;
10
11/// Result of applying fixes iteratively
12///
13/// This struct provides named fields instead of a tuple to prevent
14/// confusion about the meaning of each value.
15#[derive(Debug, Clone)]
16pub struct FixResult {
17    /// Total number of rules that successfully applied fixes
18    pub rules_fixed: usize,
19    /// Number of fix iterations performed
20    pub iterations: usize,
21    /// Number of LintContext instances created during fixing
22    pub context_creations: usize,
23    /// Names of rules that applied fixes
24    pub fixed_rule_names: HashSet<String>,
25    /// Whether the fix process converged (content stabilized)
26    pub converged: bool,
27}
28
29/// Calculate hash of content for convergence detection
30fn hash_content(content: &str) -> u64 {
31    let mut hasher = DefaultHasher::new();
32    content.hash(&mut hasher);
33    hasher.finish()
34}
35
36/// Coordinates rule fixing to minimize the number of passes needed
37pub struct FixCoordinator {
38    /// Rules that should run before others (rule -> rules that depend on it)
39    dependencies: HashMap<&'static str, Vec<&'static str>>,
40}
41
42impl Default for FixCoordinator {
43    fn default() -> Self {
44        Self::new()
45    }
46}
47
48impl FixCoordinator {
49    pub fn new() -> Self {
50        let mut dependencies = HashMap::new();
51
52        // CRITICAL DEPENDENCIES:
53        // These dependencies prevent cascading issues that require multiple passes
54
55        // MD010 (tabs->spaces) MUST run before:
56        // - MD007 (list indentation) - because tabs affect indent calculation
57        // - MD005 (list indent consistency) - same reason
58        dependencies.insert("MD010", vec!["MD007", "MD005"]);
59
60        // MD013 (line length) MUST run before:
61        // - MD009 (trailing spaces) - line wrapping might add trailing spaces that need cleanup
62        // - MD012 (multiple blanks) - reflowing can affect blank lines
63        // Note: MD013 now trims trailing whitespace during reflow to prevent mid-line spaces
64        dependencies.insert("MD013", vec!["MD009", "MD012"]);
65
66        // MD004 (list style) should run before:
67        // - MD007 (list indentation) - changing markers affects indentation
68        dependencies.insert("MD004", vec!["MD007"]);
69
70        // MD022/MD023 (heading spacing) should run before:
71        // - MD012 (multiple blanks) - heading fixes can affect blank lines
72        dependencies.insert("MD022", vec!["MD012"]);
73        dependencies.insert("MD023", vec!["MD012"]);
74
75        Self { dependencies }
76    }
77
78    /// Get the optimal order for running rules based on dependencies
79    pub fn get_optimal_order<'a>(&self, rules: &'a [Box<dyn Rule>]) -> Vec<&'a dyn Rule> {
80        // Build a map of rule names to rules for quick lookup
81        let rule_map: HashMap<&str, &dyn Rule> = rules.iter().map(|r| (r.name(), r.as_ref())).collect();
82
83        // Build reverse dependencies (rule -> rules it depends on)
84        let mut reverse_deps: HashMap<&str, HashSet<&str>> = HashMap::new();
85        for (prereq, dependents) in &self.dependencies {
86            for dependent in dependents {
87                reverse_deps.entry(dependent).or_default().insert(prereq);
88            }
89        }
90
91        // Perform topological sort
92        let mut sorted = Vec::new();
93        let mut visited = HashSet::new();
94        let mut visiting = HashSet::new();
95
96        fn visit<'a>(
97            rule_name: &str,
98            rule_map: &HashMap<&str, &'a dyn Rule>,
99            reverse_deps: &HashMap<&str, HashSet<&str>>,
100            visited: &mut HashSet<String>,
101            visiting: &mut HashSet<String>,
102            sorted: &mut Vec<&'a dyn Rule>,
103        ) {
104            if visited.contains(rule_name) {
105                return;
106            }
107
108            if visiting.contains(rule_name) {
109                // Cycle detected, but we'll just skip it
110                return;
111            }
112
113            visiting.insert(rule_name.to_string());
114
115            // Visit dependencies first
116            if let Some(deps) = reverse_deps.get(rule_name) {
117                for dep in deps {
118                    if rule_map.contains_key(dep) {
119                        visit(dep, rule_map, reverse_deps, visited, visiting, sorted);
120                    }
121                }
122            }
123
124            visiting.remove(rule_name);
125            visited.insert(rule_name.to_string());
126
127            // Add this rule to sorted list
128            if let Some(&rule) = rule_map.get(rule_name) {
129                sorted.push(rule);
130            }
131        }
132
133        // Visit all rules
134        for rule in rules {
135            visit(
136                rule.name(),
137                &rule_map,
138                &reverse_deps,
139                &mut visited,
140                &mut visiting,
141                &mut sorted,
142            );
143        }
144
145        // Add any rules not in dependency graph
146        for rule in rules {
147            if !sorted.iter().any(|r| r.name() == rule.name()) {
148                sorted.push(rule.as_ref());
149            }
150        }
151
152        sorted
153    }
154
155    /// Apply fixes iteratively until no more fixes are needed or max iterations reached
156    pub fn apply_fixes_iterative(
157        &self,
158        rules: &[Box<dyn Rule>],
159        all_warnings: &[LintWarning],
160        content: &mut String,
161        config: &Config,
162        max_iterations: usize,
163    ) -> Result<FixResult, String> {
164        // Use the minimum of max_iterations parameter and MAX_ITERATIONS constant
165        let max_iterations = max_iterations.min(MAX_ITERATIONS);
166
167        // Get optimal rule order
168        let ordered_rules = self.get_optimal_order(rules);
169
170        // Group warnings by rule for quick lookup
171        let mut warnings_by_rule: HashMap<&str, Vec<&LintWarning>> = HashMap::new();
172        for warning in all_warnings {
173            if let Some(ref rule_name) = warning.rule_name {
174                warnings_by_rule.entry(rule_name.as_str()).or_default().push(warning);
175            }
176        }
177
178        let mut total_fixed = 0;
179        let mut total_ctx_creations = 0;
180        let mut iterations = 0;
181        let mut previous_hash = hash_content(content);
182
183        // Keep track of which rules have been processed successfully
184        let mut processed_rules = HashSet::new();
185
186        // Track which rules actually applied fixes
187        let mut fixed_rule_names = HashSet::new();
188
189        // Keep applying fixes until content stabilizes
190        while iterations < max_iterations {
191            iterations += 1;
192
193            let mut fixes_in_iteration = 0;
194            let mut any_fix_applied = false;
195
196            // Process one rule at a time with its own context
197            for rule in &ordered_rules {
198                // Skip rules we've already successfully processed
199                if processed_rules.contains(rule.name()) {
200                    continue;
201                }
202
203                // Only process rules that had warnings
204                if !warnings_by_rule.contains_key(rule.name()) {
205                    processed_rules.insert(rule.name());
206                    continue;
207                }
208
209                // Check if rule is disabled
210                if config
211                    .global
212                    .unfixable
213                    .iter()
214                    .any(|r| r.eq_ignore_ascii_case(rule.name()))
215                {
216                    processed_rules.insert(rule.name());
217                    continue;
218                }
219
220                if !config.global.fixable.is_empty()
221                    && !config
222                        .global
223                        .fixable
224                        .iter()
225                        .any(|r| r.eq_ignore_ascii_case(rule.name()))
226                {
227                    processed_rules.insert(rule.name());
228                    continue;
229                }
230
231                // Create context for this specific rule
232                let ctx = LintContext::new(content, config.markdown_flavor(), None);
233                total_ctx_creations += 1;
234
235                // Apply fix
236                match rule.fix(&ctx) {
237                    Ok(fixed_content) => {
238                        if fixed_content != *content {
239                            *content = fixed_content;
240                            fixes_in_iteration += 1;
241                            any_fix_applied = true;
242                            processed_rules.insert(rule.name());
243                            fixed_rule_names.insert(rule.name().to_string());
244
245                            // If this rule has dependents, break to start fresh iteration
246                            if self.dependencies.contains_key(rule.name()) {
247                                break;
248                            }
249                            // Otherwise continue with the next rule
250                        } else {
251                            // No changes from this rule, mark as processed
252                            processed_rules.insert(rule.name());
253                        }
254                    }
255                    Err(_) => {
256                        // Error applying fix, mark as processed to avoid retrying
257                        processed_rules.insert(rule.name());
258                    }
259                }
260            }
261
262            total_fixed += fixes_in_iteration;
263
264            // Check if content has stabilized (hash-based convergence)
265            let current_hash = hash_content(content);
266            if current_hash == previous_hash {
267                // Content unchanged - converged!
268                return Ok(FixResult {
269                    rules_fixed: total_fixed,
270                    iterations,
271                    context_creations: total_ctx_creations,
272                    fixed_rule_names,
273                    converged: true,
274                });
275            }
276            previous_hash = current_hash;
277
278            // If no fixes were made in this iteration, we're done
279            if !any_fix_applied {
280                break;
281            }
282
283            // If all rules have been processed, we're done
284            if processed_rules.len() >= ordered_rules.len() {
285                break;
286            }
287        }
288
289        // If we reached here, either we hit max iterations or all rules processed
290        let converged = iterations < max_iterations;
291        Ok(FixResult {
292            rules_fixed: total_fixed,
293            iterations,
294            context_creations: total_ctx_creations,
295            fixed_rule_names,
296            converged,
297        })
298    }
299}
300
301#[cfg(test)]
302mod tests {
303    use super::*;
304    use crate::config::GlobalConfig;
305    use crate::rule::{LintError, LintResult, LintWarning, Rule, RuleCategory};
306
307    // Mock rule for testing
308    #[derive(Clone)]
309    struct MockRule {
310        name: &'static str,
311        warnings: Vec<LintWarning>,
312        fix_content: String,
313    }
314
315    impl Rule for MockRule {
316        fn name(&self) -> &'static str {
317            self.name
318        }
319
320        fn check(&self, _ctx: &LintContext) -> LintResult {
321            Ok(self.warnings.clone())
322        }
323
324        fn fix(&self, _ctx: &LintContext) -> Result<String, LintError> {
325            Ok(self.fix_content.clone())
326        }
327
328        fn description(&self) -> &'static str {
329            "Mock rule for testing"
330        }
331
332        fn category(&self) -> RuleCategory {
333            RuleCategory::Whitespace
334        }
335
336        fn as_any(&self) -> &dyn std::any::Any {
337            self
338        }
339    }
340
341    #[test]
342    fn test_dependency_ordering() {
343        let coordinator = FixCoordinator::new();
344
345        let rules: Vec<Box<dyn Rule>> = vec![
346            Box::new(MockRule {
347                name: "MD009",
348                warnings: vec![],
349                fix_content: "".to_string(),
350            }),
351            Box::new(MockRule {
352                name: "MD013",
353                warnings: vec![],
354                fix_content: "".to_string(),
355            }),
356            Box::new(MockRule {
357                name: "MD010",
358                warnings: vec![],
359                fix_content: "".to_string(),
360            }),
361            Box::new(MockRule {
362                name: "MD007",
363                warnings: vec![],
364                fix_content: "".to_string(),
365            }),
366        ];
367
368        let ordered = coordinator.get_optimal_order(&rules);
369        let ordered_names: Vec<&str> = ordered.iter().map(|r| r.name()).collect();
370
371        // MD010 should come before MD007 (dependency)
372        let md010_idx = ordered_names.iter().position(|&n| n == "MD010").unwrap();
373        let md007_idx = ordered_names.iter().position(|&n| n == "MD007").unwrap();
374        assert!(md010_idx < md007_idx, "MD010 should come before MD007");
375
376        // MD013 should come before MD009 (dependency)
377        let md013_idx = ordered_names.iter().position(|&n| n == "MD013").unwrap();
378        let md009_idx = ordered_names.iter().position(|&n| n == "MD009").unwrap();
379        assert!(md013_idx < md009_idx, "MD013 should come before MD009");
380    }
381
382    #[test]
383    fn test_single_iteration_fix() {
384        let coordinator = FixCoordinator::new();
385
386        let rules: Vec<Box<dyn Rule>> = vec![Box::new(MockRule {
387            name: "MD001",
388            warnings: vec![LintWarning {
389                line: 1,
390                column: 1,
391                end_line: 1,
392                end_column: 10,
393                message: "Test warning".to_string(),
394                rule_name: Some("MD001".to_string()),
395                severity: crate::rule::Severity::Error,
396                fix: None,
397            }],
398            fix_content: "fixed content".to_string(),
399        })];
400
401        let warnings = vec![LintWarning {
402            line: 1,
403            column: 1,
404            end_line: 1,
405            end_column: 10,
406            message: "Test warning".to_string(),
407            rule_name: Some("MD001".to_string()),
408            severity: crate::rule::Severity::Error,
409            fix: None,
410        }];
411
412        let mut content = "original content".to_string();
413        let config = Config {
414            global: GlobalConfig::default(),
415            per_file_ignores: HashMap::new(),
416            rules: Default::default(),
417            project_root: None,
418        };
419
420        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
421
422        assert!(result.is_ok());
423        let result = result.unwrap();
424        assert_eq!(result.rules_fixed, 1);
425        assert_eq!(result.iterations, 1);
426        assert_eq!(result.context_creations, 1);
427        assert!(result.converged);
428        assert_eq!(content, "fixed content");
429    }
430
431    #[test]
432    fn test_multiple_iteration_with_dependencies() {
433        let coordinator = FixCoordinator::new();
434
435        let rules: Vec<Box<dyn Rule>> = vec![
436            Box::new(MockRule {
437                name: "MD010", // Has dependents
438                warnings: vec![LintWarning {
439                    line: 1,
440                    column: 1,
441                    end_line: 1,
442                    end_column: 10,
443                    message: "Tabs".to_string(),
444                    rule_name: Some("MD010".to_string()),
445                    severity: crate::rule::Severity::Error,
446                    fix: None,
447                }],
448                fix_content: "content with spaces".to_string(),
449            }),
450            Box::new(MockRule {
451                name: "MD007", // Depends on MD010
452                warnings: vec![LintWarning {
453                    line: 1,
454                    column: 1,
455                    end_line: 1,
456                    end_column: 10,
457                    message: "Indentation".to_string(),
458                    rule_name: Some("MD007".to_string()),
459                    severity: crate::rule::Severity::Error,
460                    fix: None,
461                }],
462                fix_content: "content with spaces and proper indent".to_string(),
463            }),
464        ];
465
466        let warnings = vec![
467            LintWarning {
468                line: 1,
469                column: 1,
470                end_line: 1,
471                end_column: 10,
472                message: "Tabs".to_string(),
473                rule_name: Some("MD010".to_string()),
474                severity: crate::rule::Severity::Error,
475                fix: None,
476            },
477            LintWarning {
478                line: 1,
479                column: 1,
480                end_line: 1,
481                end_column: 10,
482                message: "Indentation".to_string(),
483                rule_name: Some("MD007".to_string()),
484                severity: crate::rule::Severity::Error,
485                fix: None,
486            },
487        ];
488
489        let mut content = "content with tabs".to_string();
490        let config = Config {
491            global: GlobalConfig::default(),
492            per_file_ignores: HashMap::new(),
493            rules: Default::default(),
494            project_root: None,
495        };
496
497        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
498
499        assert!(result.is_ok());
500        let result = result.unwrap();
501        assert_eq!(result.rules_fixed, 2);
502        assert_eq!(result.iterations, 2); // Should take 2 iterations due to dependency
503        assert!(result.context_creations >= 2);
504        assert!(result.converged);
505    }
506
507    #[test]
508    fn test_unfixable_rules_skipped() {
509        let coordinator = FixCoordinator::new();
510
511        let rules: Vec<Box<dyn Rule>> = vec![Box::new(MockRule {
512            name: "MD001",
513            warnings: vec![LintWarning {
514                line: 1,
515                column: 1,
516                end_line: 1,
517                end_column: 10,
518                message: "Test".to_string(),
519                rule_name: Some("MD001".to_string()),
520                severity: crate::rule::Severity::Error,
521                fix: None,
522            }],
523            fix_content: "fixed".to_string(),
524        })];
525
526        let warnings = vec![LintWarning {
527            line: 1,
528            column: 1,
529            end_line: 1,
530            end_column: 10,
531            message: "Test".to_string(),
532            rule_name: Some("MD001".to_string()),
533            severity: crate::rule::Severity::Error,
534            fix: None,
535        }];
536
537        let mut content = "original".to_string();
538        let mut config = Config {
539            global: GlobalConfig::default(),
540            per_file_ignores: HashMap::new(),
541            rules: Default::default(),
542            project_root: None,
543        };
544        config.global.unfixable = vec!["MD001".to_string()];
545
546        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
547
548        assert!(result.is_ok());
549        let result = result.unwrap();
550        assert_eq!(result.rules_fixed, 0);
551        assert!(result.converged);
552        assert_eq!(content, "original"); // Should not be changed
553    }
554
555    #[test]
556    fn test_max_iterations_limit() {
557        // This test ensures we don't loop infinitely
558        let coordinator = FixCoordinator::new();
559
560        // Create a rule that always changes content
561        #[derive(Clone)]
562        struct AlwaysChangeRule;
563        impl Rule for AlwaysChangeRule {
564            fn name(&self) -> &'static str {
565                "MD999"
566            }
567            fn check(&self, _: &LintContext) -> LintResult {
568                Ok(vec![LintWarning {
569                    line: 1,
570                    column: 1,
571                    end_line: 1,
572                    end_column: 10,
573                    message: "Always warns".to_string(),
574                    rule_name: Some("MD999".to_string()),
575                    severity: crate::rule::Severity::Error,
576                    fix: None,
577                }])
578            }
579            fn fix(&self, ctx: &LintContext) -> Result<String, LintError> {
580                Ok(format!("{}x", ctx.content))
581            }
582            fn description(&self) -> &'static str {
583                "Always changes"
584            }
585            fn category(&self) -> RuleCategory {
586                RuleCategory::Whitespace
587            }
588            fn as_any(&self) -> &dyn std::any::Any {
589                self
590            }
591        }
592
593        let rules: Vec<Box<dyn Rule>> = vec![Box::new(AlwaysChangeRule)];
594        let warnings = vec![LintWarning {
595            line: 1,
596            column: 1,
597            end_line: 1,
598            end_column: 10,
599            message: "Always warns".to_string(),
600            rule_name: Some("MD999".to_string()),
601            severity: crate::rule::Severity::Error,
602            fix: None,
603        }];
604
605        let mut content = "test".to_string();
606        let config = Config {
607            global: GlobalConfig::default(),
608            per_file_ignores: HashMap::new(),
609            rules: Default::default(),
610            project_root: None,
611        };
612
613        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 3);
614
615        assert!(result.is_ok());
616        let result = result.unwrap();
617        assert_eq!(result.iterations, 1); // Should stop after first successful fix
618        assert!(result.converged);
619    }
620
621    #[test]
622    fn test_empty_rules_and_warnings() {
623        let coordinator = FixCoordinator::new();
624        let rules: Vec<Box<dyn Rule>> = vec![];
625        let warnings: Vec<LintWarning> = vec![];
626
627        let mut content = "unchanged".to_string();
628        let config = Config {
629            global: GlobalConfig::default(),
630            per_file_ignores: HashMap::new(),
631            rules: Default::default(),
632            project_root: None,
633        };
634
635        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
636
637        assert!(result.is_ok());
638        let result = result.unwrap();
639        assert_eq!(result.rules_fixed, 0);
640        assert_eq!(result.iterations, 1);
641        assert_eq!(result.context_creations, 0);
642        assert!(result.converged);
643        assert_eq!(content, "unchanged");
644    }
645
646    #[test]
647    fn test_cyclic_dependencies_handled() {
648        // Test that cyclic dependencies don't cause infinite loops
649        let mut coordinator = FixCoordinator::new();
650
651        // Create a cycle: A -> B -> C -> A
652        coordinator.dependencies.insert("RuleA", vec!["RuleB"]);
653        coordinator.dependencies.insert("RuleB", vec!["RuleC"]);
654        coordinator.dependencies.insert("RuleC", vec!["RuleA"]);
655
656        let rules: Vec<Box<dyn Rule>> = vec![
657            Box::new(MockRule {
658                name: "RuleA",
659                warnings: vec![],
660                fix_content: "".to_string(),
661            }),
662            Box::new(MockRule {
663                name: "RuleB",
664                warnings: vec![],
665                fix_content: "".to_string(),
666            }),
667            Box::new(MockRule {
668                name: "RuleC",
669                warnings: vec![],
670                fix_content: "".to_string(),
671            }),
672        ];
673
674        // Should not panic or infinite loop
675        let ordered = coordinator.get_optimal_order(&rules);
676
677        // Should return all rules despite cycle
678        assert_eq!(ordered.len(), 3);
679    }
680}
rumdl_lib/fix_coordinator.rs

rumdl_lib/
fix_coordinator.rs
