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            rule_severities: Default::default(),
418            project_root: None,
419        };
420
421        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
422
423        assert!(result.is_ok());
424        let result = result.unwrap();
425        assert_eq!(result.rules_fixed, 1);
426        assert_eq!(result.iterations, 1);
427        assert_eq!(result.context_creations, 1);
428        assert!(result.converged);
429        assert_eq!(content, "fixed content");
430    }
431
432    #[test]
433    fn test_multiple_iteration_with_dependencies() {
434        let coordinator = FixCoordinator::new();
435
436        let rules: Vec<Box<dyn Rule>> = vec![
437            Box::new(MockRule {
438                name: "MD010", // Has dependents
439                warnings: vec![LintWarning {
440                    line: 1,
441                    column: 1,
442                    end_line: 1,
443                    end_column: 10,
444                    message: "Tabs".to_string(),
445                    rule_name: Some("MD010".to_string()),
446                    severity: crate::rule::Severity::Error,
447                    fix: None,
448                }],
449                fix_content: "content with spaces".to_string(),
450            }),
451            Box::new(MockRule {
452                name: "MD007", // Depends on MD010
453                warnings: vec![LintWarning {
454                    line: 1,
455                    column: 1,
456                    end_line: 1,
457                    end_column: 10,
458                    message: "Indentation".to_string(),
459                    rule_name: Some("MD007".to_string()),
460                    severity: crate::rule::Severity::Error,
461                    fix: None,
462                }],
463                fix_content: "content with spaces and proper indent".to_string(),
464            }),
465        ];
466
467        let warnings = vec![
468            LintWarning {
469                line: 1,
470                column: 1,
471                end_line: 1,
472                end_column: 10,
473                message: "Tabs".to_string(),
474                rule_name: Some("MD010".to_string()),
475                severity: crate::rule::Severity::Error,
476                fix: None,
477            },
478            LintWarning {
479                line: 1,
480                column: 1,
481                end_line: 1,
482                end_column: 10,
483                message: "Indentation".to_string(),
484                rule_name: Some("MD007".to_string()),
485                severity: crate::rule::Severity::Error,
486                fix: None,
487            },
488        ];
489
490        let mut content = "content with tabs".to_string();
491        let config = Config {
492            global: GlobalConfig::default(),
493            per_file_ignores: HashMap::new(),
494            rules: Default::default(),
495            rule_severities: Default::default(),
496            project_root: None,
497        };
498
499        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
500
501        assert!(result.is_ok());
502        let result = result.unwrap();
503        assert_eq!(result.rules_fixed, 2);
504        assert_eq!(result.iterations, 2); // Should take 2 iterations due to dependency
505        assert!(result.context_creations >= 2);
506        assert!(result.converged);
507    }
508
509    #[test]
510    fn test_unfixable_rules_skipped() {
511        let coordinator = FixCoordinator::new();
512
513        let rules: Vec<Box<dyn Rule>> = vec![Box::new(MockRule {
514            name: "MD001",
515            warnings: vec![LintWarning {
516                line: 1,
517                column: 1,
518                end_line: 1,
519                end_column: 10,
520                message: "Test".to_string(),
521                rule_name: Some("MD001".to_string()),
522                severity: crate::rule::Severity::Error,
523                fix: None,
524            }],
525            fix_content: "fixed".to_string(),
526        })];
527
528        let warnings = vec![LintWarning {
529            line: 1,
530            column: 1,
531            end_line: 1,
532            end_column: 10,
533            message: "Test".to_string(),
534            rule_name: Some("MD001".to_string()),
535            severity: crate::rule::Severity::Error,
536            fix: None,
537        }];
538
539        let mut content = "original".to_string();
540        let mut config = Config {
541            global: GlobalConfig::default(),
542            per_file_ignores: HashMap::new(),
543            rules: Default::default(),
544            rule_severities: Default::default(),
545            project_root: None,
546        };
547        config.global.unfixable = vec!["MD001".to_string()];
548
549        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
550
551        assert!(result.is_ok());
552        let result = result.unwrap();
553        assert_eq!(result.rules_fixed, 0);
554        assert!(result.converged);
555        assert_eq!(content, "original"); // Should not be changed
556    }
557
558    #[test]
559    fn test_max_iterations_limit() {
560        // This test ensures we don't loop infinitely
561        let coordinator = FixCoordinator::new();
562
563        // Create a rule that always changes content
564        #[derive(Clone)]
565        struct AlwaysChangeRule;
566        impl Rule for AlwaysChangeRule {
567            fn name(&self) -> &'static str {
568                "MD999"
569            }
570            fn check(&self, _: &LintContext) -> LintResult {
571                Ok(vec![LintWarning {
572                    line: 1,
573                    column: 1,
574                    end_line: 1,
575                    end_column: 10,
576                    message: "Always warns".to_string(),
577                    rule_name: Some("MD999".to_string()),
578                    severity: crate::rule::Severity::Error,
579                    fix: None,
580                }])
581            }
582            fn fix(&self, ctx: &LintContext) -> Result<String, LintError> {
583                Ok(format!("{}x", ctx.content))
584            }
585            fn description(&self) -> &'static str {
586                "Always changes"
587            }
588            fn category(&self) -> RuleCategory {
589                RuleCategory::Whitespace
590            }
591            fn as_any(&self) -> &dyn std::any::Any {
592                self
593            }
594        }
595
596        let rules: Vec<Box<dyn Rule>> = vec![Box::new(AlwaysChangeRule)];
597        let warnings = vec![LintWarning {
598            line: 1,
599            column: 1,
600            end_line: 1,
601            end_column: 10,
602            message: "Always warns".to_string(),
603            rule_name: Some("MD999".to_string()),
604            severity: crate::rule::Severity::Error,
605            fix: None,
606        }];
607
608        let mut content = "test".to_string();
609        let config = Config {
610            global: GlobalConfig::default(),
611            per_file_ignores: HashMap::new(),
612            rules: Default::default(),
613            rule_severities: Default::default(),
614            project_root: None,
615        };
616
617        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 3);
618
619        assert!(result.is_ok());
620        let result = result.unwrap();
621        assert_eq!(result.iterations, 1); // Should stop after first successful fix
622        assert!(result.converged);
623    }
624
625    #[test]
626    fn test_empty_rules_and_warnings() {
627        let coordinator = FixCoordinator::new();
628        let rules: Vec<Box<dyn Rule>> = vec![];
629        let warnings: Vec<LintWarning> = vec![];
630
631        let mut content = "unchanged".to_string();
632        let config = Config {
633            global: GlobalConfig::default(),
634            per_file_ignores: HashMap::new(),
635            rules: Default::default(),
636            rule_severities: Default::default(),
637            project_root: None,
638        };
639
640        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
641
642        assert!(result.is_ok());
643        let result = result.unwrap();
644        assert_eq!(result.rules_fixed, 0);
645        assert_eq!(result.iterations, 1);
646        assert_eq!(result.context_creations, 0);
647        assert!(result.converged);
648        assert_eq!(content, "unchanged");
649    }
650
651    #[test]
652    fn test_cyclic_dependencies_handled() {
653        // Test that cyclic dependencies don't cause infinite loops
654        let mut coordinator = FixCoordinator::new();
655
656        // Create a cycle: A -> B -> C -> A
657        coordinator.dependencies.insert("RuleA", vec!["RuleB"]);
658        coordinator.dependencies.insert("RuleB", vec!["RuleC"]);
659        coordinator.dependencies.insert("RuleC", vec!["RuleA"]);
660
661        let rules: Vec<Box<dyn Rule>> = vec![
662            Box::new(MockRule {
663                name: "RuleA",
664                warnings: vec![],
665                fix_content: "".to_string(),
666            }),
667            Box::new(MockRule {
668                name: "RuleB",
669                warnings: vec![],
670                fix_content: "".to_string(),
671            }),
672            Box::new(MockRule {
673                name: "RuleC",
674                warnings: vec![],
675                fix_content: "".to_string(),
676            }),
677        ];
678
679        // Should not panic or infinite loop
680        let ordered = coordinator.get_optimal_order(&rules);
681
682        // Should return all rules despite cycle
683        assert_eq!(ordered.len(), 3);
684    }
685}
rumdl_lib/fix_coordinator.rs

rumdl_lib/
fix_coordinator.rs
