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        // MD070 (nested fence collision) MUST run before:
76        // - MD040 (code language) - MD070 changes block structure, making orphan fences into content
77        // - MD031 (blanks around fences) - same reason
78        dependencies.insert("MD070", vec!["MD040", "MD031"]);
79
80        Self { dependencies }
81    }
82
83    /// Get the optimal order for running rules based on dependencies
84    pub fn get_optimal_order<'a>(&self, rules: &'a [Box<dyn Rule>]) -> Vec<&'a dyn Rule> {
85        // Build a map of rule names to rules for quick lookup
86        let rule_map: HashMap<&str, &dyn Rule> = rules.iter().map(|r| (r.name(), r.as_ref())).collect();
87
88        // Build reverse dependencies (rule -> rules it depends on)
89        let mut reverse_deps: HashMap<&str, HashSet<&str>> = HashMap::new();
90        for (prereq, dependents) in &self.dependencies {
91            for dependent in dependents {
92                reverse_deps.entry(dependent).or_default().insert(prereq);
93            }
94        }
95
96        // Perform topological sort
97        let mut sorted = Vec::new();
98        let mut visited = HashSet::new();
99        let mut visiting = HashSet::new();
100
101        fn visit<'a>(
102            rule_name: &str,
103            rule_map: &HashMap<&str, &'a dyn Rule>,
104            reverse_deps: &HashMap<&str, HashSet<&str>>,
105            visited: &mut HashSet<String>,
106            visiting: &mut HashSet<String>,
107            sorted: &mut Vec<&'a dyn Rule>,
108        ) {
109            if visited.contains(rule_name) {
110                return;
111            }
112
113            if visiting.contains(rule_name) {
114                // Cycle detected, but we'll just skip it
115                return;
116            }
117
118            visiting.insert(rule_name.to_string());
119
120            // Visit dependencies first
121            if let Some(deps) = reverse_deps.get(rule_name) {
122                for dep in deps {
123                    if rule_map.contains_key(dep) {
124                        visit(dep, rule_map, reverse_deps, visited, visiting, sorted);
125                    }
126                }
127            }
128
129            visiting.remove(rule_name);
130            visited.insert(rule_name.to_string());
131
132            // Add this rule to sorted list
133            if let Some(&rule) = rule_map.get(rule_name) {
134                sorted.push(rule);
135            }
136        }
137
138        // Visit all rules
139        for rule in rules {
140            visit(
141                rule.name(),
142                &rule_map,
143                &reverse_deps,
144                &mut visited,
145                &mut visiting,
146                &mut sorted,
147            );
148        }
149
150        // Add any rules not in dependency graph
151        for rule in rules {
152            if !sorted.iter().any(|r| r.name() == rule.name()) {
153                sorted.push(rule.as_ref());
154            }
155        }
156
157        sorted
158    }
159
160    /// Apply fixes iteratively until no more fixes are needed or max iterations reached
161    pub fn apply_fixes_iterative(
162        &self,
163        rules: &[Box<dyn Rule>],
164        all_warnings: &[LintWarning],
165        content: &mut String,
166        config: &Config,
167        max_iterations: usize,
168    ) -> Result<FixResult, String> {
169        // Use the minimum of max_iterations parameter and MAX_ITERATIONS constant
170        let max_iterations = max_iterations.min(MAX_ITERATIONS);
171
172        // Get optimal rule order
173        let ordered_rules = self.get_optimal_order(rules);
174
175        // Group warnings by rule for quick lookup
176        let mut warnings_by_rule: HashMap<&str, Vec<&LintWarning>> = HashMap::new();
177        for warning in all_warnings {
178            if let Some(ref rule_name) = warning.rule_name {
179                warnings_by_rule.entry(rule_name.as_str()).or_default().push(warning);
180            }
181        }
182
183        let mut total_fixed = 0;
184        let mut total_ctx_creations = 0;
185        let mut iterations = 0;
186        let mut previous_hash = hash_content(content);
187
188        // Keep track of which rules have been processed successfully
189        let mut processed_rules = HashSet::new();
190
191        // Track which rules actually applied fixes
192        let mut fixed_rule_names = HashSet::new();
193
194        // Keep applying fixes until content stabilizes
195        while iterations < max_iterations {
196            iterations += 1;
197
198            let mut fixes_in_iteration = 0;
199            let mut any_fix_applied = false;
200
201            // Process one rule at a time with its own context
202            for rule in &ordered_rules {
203                // Skip rules we've already successfully processed
204                if processed_rules.contains(rule.name()) {
205                    continue;
206                }
207
208                // Only process rules that had warnings
209                if !warnings_by_rule.contains_key(rule.name()) {
210                    processed_rules.insert(rule.name());
211                    continue;
212                }
213
214                // Check if rule is disabled
215                if config
216                    .global
217                    .unfixable
218                    .iter()
219                    .any(|r| r.eq_ignore_ascii_case(rule.name()))
220                {
221                    processed_rules.insert(rule.name());
222                    continue;
223                }
224
225                if !config.global.fixable.is_empty()
226                    && !config
227                        .global
228                        .fixable
229                        .iter()
230                        .any(|r| r.eq_ignore_ascii_case(rule.name()))
231                {
232                    processed_rules.insert(rule.name());
233                    continue;
234                }
235
236                // Create context for this specific rule
237                let ctx = LintContext::new(content, config.markdown_flavor(), None);
238                total_ctx_creations += 1;
239
240                // Apply fix
241                match rule.fix(&ctx) {
242                    Ok(fixed_content) => {
243                        if fixed_content != *content {
244                            *content = fixed_content;
245                            fixes_in_iteration += 1;
246                            any_fix_applied = true;
247                            processed_rules.insert(rule.name());
248                            fixed_rule_names.insert(rule.name().to_string());
249
250                            // If this rule has dependents, break to start fresh iteration
251                            if self.dependencies.contains_key(rule.name()) {
252                                break;
253                            }
254                            // Otherwise continue with the next rule
255                        } else {
256                            // No changes from this rule, mark as processed
257                            processed_rules.insert(rule.name());
258                        }
259                    }
260                    Err(_) => {
261                        // Error applying fix, mark as processed to avoid retrying
262                        processed_rules.insert(rule.name());
263                    }
264                }
265            }
266
267            total_fixed += fixes_in_iteration;
268
269            // Check if content has stabilized (hash-based convergence)
270            let current_hash = hash_content(content);
271            if current_hash == previous_hash {
272                // Content unchanged - converged!
273                return Ok(FixResult {
274                    rules_fixed: total_fixed,
275                    iterations,
276                    context_creations: total_ctx_creations,
277                    fixed_rule_names,
278                    converged: true,
279                });
280            }
281            previous_hash = current_hash;
282
283            // If no fixes were made in this iteration, we're done
284            if !any_fix_applied {
285                break;
286            }
287
288            // If all rules have been processed, we're done
289            if processed_rules.len() >= ordered_rules.len() {
290                break;
291            }
292        }
293
294        // If we reached here, either we hit max iterations or all rules processed
295        let converged = iterations < max_iterations;
296        Ok(FixResult {
297            rules_fixed: total_fixed,
298            iterations,
299            context_creations: total_ctx_creations,
300            fixed_rule_names,
301            converged,
302        })
303    }
304}
305
306#[cfg(test)]
307mod tests {
308    use super::*;
309    use crate::config::GlobalConfig;
310    use crate::rule::{LintError, LintResult, LintWarning, Rule, RuleCategory};
311
312    // Mock rule for testing
313    #[derive(Clone)]
314    struct MockRule {
315        name: &'static str,
316        warnings: Vec<LintWarning>,
317        fix_content: String,
318    }
319
320    impl Rule for MockRule {
321        fn name(&self) -> &'static str {
322            self.name
323        }
324
325        fn check(&self, _ctx: &LintContext) -> LintResult {
326            Ok(self.warnings.clone())
327        }
328
329        fn fix(&self, _ctx: &LintContext) -> Result<String, LintError> {
330            Ok(self.fix_content.clone())
331        }
332
333        fn description(&self) -> &'static str {
334            "Mock rule for testing"
335        }
336
337        fn category(&self) -> RuleCategory {
338            RuleCategory::Whitespace
339        }
340
341        fn as_any(&self) -> &dyn std::any::Any {
342            self
343        }
344    }
345
346    #[test]
347    fn test_dependency_ordering() {
348        let coordinator = FixCoordinator::new();
349
350        let rules: Vec<Box<dyn Rule>> = vec![
351            Box::new(MockRule {
352                name: "MD009",
353                warnings: vec![],
354                fix_content: "".to_string(),
355            }),
356            Box::new(MockRule {
357                name: "MD013",
358                warnings: vec![],
359                fix_content: "".to_string(),
360            }),
361            Box::new(MockRule {
362                name: "MD010",
363                warnings: vec![],
364                fix_content: "".to_string(),
365            }),
366            Box::new(MockRule {
367                name: "MD007",
368                warnings: vec![],
369                fix_content: "".to_string(),
370            }),
371        ];
372
373        let ordered = coordinator.get_optimal_order(&rules);
374        let ordered_names: Vec<&str> = ordered.iter().map(|r| r.name()).collect();
375
376        // MD010 should come before MD007 (dependency)
377        let md010_idx = ordered_names.iter().position(|&n| n == "MD010").unwrap();
378        let md007_idx = ordered_names.iter().position(|&n| n == "MD007").unwrap();
379        assert!(md010_idx < md007_idx, "MD010 should come before MD007");
380
381        // MD013 should come before MD009 (dependency)
382        let md013_idx = ordered_names.iter().position(|&n| n == "MD013").unwrap();
383        let md009_idx = ordered_names.iter().position(|&n| n == "MD009").unwrap();
384        assert!(md013_idx < md009_idx, "MD013 should come before MD009");
385    }
386
387    #[test]
388    fn test_single_iteration_fix() {
389        let coordinator = FixCoordinator::new();
390
391        let rules: Vec<Box<dyn Rule>> = vec![Box::new(MockRule {
392            name: "MD001",
393            warnings: vec![LintWarning {
394                line: 1,
395                column: 1,
396                end_line: 1,
397                end_column: 10,
398                message: "Test warning".to_string(),
399                rule_name: Some("MD001".to_string()),
400                severity: crate::rule::Severity::Error,
401                fix: None,
402            }],
403            fix_content: "fixed content".to_string(),
404        })];
405
406        let warnings = vec![LintWarning {
407            line: 1,
408            column: 1,
409            end_line: 1,
410            end_column: 10,
411            message: "Test warning".to_string(),
412            rule_name: Some("MD001".to_string()),
413            severity: crate::rule::Severity::Error,
414            fix: None,
415        }];
416
417        let mut content = "original content".to_string();
418        let config = Config {
419            global: GlobalConfig::default(),
420            per_file_ignores: HashMap::new(),
421            rules: Default::default(),
422            project_root: None,
423        };
424
425        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
426
427        assert!(result.is_ok());
428        let result = result.unwrap();
429        assert_eq!(result.rules_fixed, 1);
430        assert_eq!(result.iterations, 1);
431        assert_eq!(result.context_creations, 1);
432        assert!(result.converged);
433        assert_eq!(content, "fixed content");
434    }
435
436    #[test]
437    fn test_multiple_iteration_with_dependencies() {
438        let coordinator = FixCoordinator::new();
439
440        let rules: Vec<Box<dyn Rule>> = vec![
441            Box::new(MockRule {
442                name: "MD010", // Has dependents
443                warnings: vec![LintWarning {
444                    line: 1,
445                    column: 1,
446                    end_line: 1,
447                    end_column: 10,
448                    message: "Tabs".to_string(),
449                    rule_name: Some("MD010".to_string()),
450                    severity: crate::rule::Severity::Error,
451                    fix: None,
452                }],
453                fix_content: "content with spaces".to_string(),
454            }),
455            Box::new(MockRule {
456                name: "MD007", // Depends on MD010
457                warnings: vec![LintWarning {
458                    line: 1,
459                    column: 1,
460                    end_line: 1,
461                    end_column: 10,
462                    message: "Indentation".to_string(),
463                    rule_name: Some("MD007".to_string()),
464                    severity: crate::rule::Severity::Error,
465                    fix: None,
466                }],
467                fix_content: "content with spaces and proper indent".to_string(),
468            }),
469        ];
470
471        let warnings = vec![
472            LintWarning {
473                line: 1,
474                column: 1,
475                end_line: 1,
476                end_column: 10,
477                message: "Tabs".to_string(),
478                rule_name: Some("MD010".to_string()),
479                severity: crate::rule::Severity::Error,
480                fix: None,
481            },
482            LintWarning {
483                line: 1,
484                column: 1,
485                end_line: 1,
486                end_column: 10,
487                message: "Indentation".to_string(),
488                rule_name: Some("MD007".to_string()),
489                severity: crate::rule::Severity::Error,
490                fix: None,
491            },
492        ];
493
494        let mut content = "content with tabs".to_string();
495        let config = Config {
496            global: GlobalConfig::default(),
497            per_file_ignores: HashMap::new(),
498            rules: Default::default(),
499            project_root: None,
500        };
501
502        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
503
504        assert!(result.is_ok());
505        let result = result.unwrap();
506        assert_eq!(result.rules_fixed, 2);
507        assert_eq!(result.iterations, 2); // Should take 2 iterations due to dependency
508        assert!(result.context_creations >= 2);
509        assert!(result.converged);
510    }
511
512    #[test]
513    fn test_unfixable_rules_skipped() {
514        let coordinator = FixCoordinator::new();
515
516        let rules: Vec<Box<dyn Rule>> = vec![Box::new(MockRule {
517            name: "MD001",
518            warnings: vec![LintWarning {
519                line: 1,
520                column: 1,
521                end_line: 1,
522                end_column: 10,
523                message: "Test".to_string(),
524                rule_name: Some("MD001".to_string()),
525                severity: crate::rule::Severity::Error,
526                fix: None,
527            }],
528            fix_content: "fixed".to_string(),
529        })];
530
531        let warnings = vec![LintWarning {
532            line: 1,
533            column: 1,
534            end_line: 1,
535            end_column: 10,
536            message: "Test".to_string(),
537            rule_name: Some("MD001".to_string()),
538            severity: crate::rule::Severity::Error,
539            fix: None,
540        }];
541
542        let mut content = "original".to_string();
543        let mut config = Config {
544            global: GlobalConfig::default(),
545            per_file_ignores: HashMap::new(),
546            rules: Default::default(),
547            project_root: None,
548        };
549        config.global.unfixable = vec!["MD001".to_string()];
550
551        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 5);
552
553        assert!(result.is_ok());
554        let result = result.unwrap();
555        assert_eq!(result.rules_fixed, 0);
556        assert!(result.converged);
557        assert_eq!(content, "original"); // Should not be changed
558    }
559
560    #[test]
561    fn test_max_iterations_limit() {
562        // This test ensures we don't loop infinitely
563        let coordinator = FixCoordinator::new();
564
565        // Create a rule that always changes content
566        #[derive(Clone)]
567        struct AlwaysChangeRule;
568        impl Rule for AlwaysChangeRule {
569            fn name(&self) -> &'static str {
570                "MD999"
571            }
572            fn check(&self, _: &LintContext) -> LintResult {
573                Ok(vec![LintWarning {
574                    line: 1,
575                    column: 1,
576                    end_line: 1,
577                    end_column: 10,
578                    message: "Always warns".to_string(),
579                    rule_name: Some("MD999".to_string()),
580                    severity: crate::rule::Severity::Error,
581                    fix: None,
582                }])
583            }
584            fn fix(&self, ctx: &LintContext) -> Result<String, LintError> {
585                Ok(format!("{}x", ctx.content))
586            }
587            fn description(&self) -> &'static str {
588                "Always changes"
589            }
590            fn category(&self) -> RuleCategory {
591                RuleCategory::Whitespace
592            }
593            fn as_any(&self) -> &dyn std::any::Any {
594                self
595            }
596        }
597
598        let rules: Vec<Box<dyn Rule>> = vec![Box::new(AlwaysChangeRule)];
599        let warnings = vec![LintWarning {
600            line: 1,
601            column: 1,
602            end_line: 1,
603            end_column: 10,
604            message: "Always warns".to_string(),
605            rule_name: Some("MD999".to_string()),
606            severity: crate::rule::Severity::Error,
607            fix: None,
608        }];
609
610        let mut content = "test".to_string();
611        let config = Config {
612            global: GlobalConfig::default(),
613            per_file_ignores: HashMap::new(),
614            rules: Default::default(),
615            project_root: None,
616        };
617
618        let result = coordinator.apply_fixes_iterative(&rules, &warnings, &mut content, &config, 3);
619
620        assert!(result.is_ok());
621        let result = result.unwrap();
622        assert_eq!(result.iterations, 1); // Should stop after first successful fix
623        assert!(result.converged);
624    }
625
626    #[test]
627    fn test_empty_rules_and_warnings() {
628        let coordinator = FixCoordinator::new();
629        let rules: Vec<Box<dyn Rule>> = vec![];
630        let warnings: Vec<LintWarning> = vec![];
631
632        let mut content = "unchanged".to_string();
633        let config = Config {
634            global: GlobalConfig::default(),
635            per_file_ignores: HashMap::new(),
636            rules: 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
