rumdl_lib/
fix_coordinator.rs

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

rumdl_lib/
fix_coordinator.rs
