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

rumdl_lib/
fix_coordinator.rs
