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

rumdl_lib/
fix_coordinator.rs
