Skip to main content

batuta/comply/rules/
makefile.rs

1//! Makefile Target Consistency Rule
2//!
3//! Ensures all PAIML stack projects have consistent Makefile targets.
4
5use crate::comply::rule::{
6    FixDetail, FixResult, RuleCategory, RuleResult, RuleViolation, StackComplianceRule, Suggestion,
7    ViolationLevel,
8};
9use std::collections::HashMap;
10use std::path::Path;
11
12/// Makefile target consistency rule
13#[derive(Debug)]
14pub struct MakefileRule {
15    /// Required targets with expected patterns
16    required_targets: HashMap<String, TargetSpec>,
17    /// Prohibited commands
18    prohibited_commands: Vec<String>,
19}
20
21#[derive(Debug, Clone)]
22struct TargetSpec {
23    pattern: Option<String>,
24    description: String,
25}
26
27impl Default for MakefileRule {
28    fn default() -> Self {
29        Self::new()
30    }
31}
32
33impl MakefileRule {
34    /// Create a new Makefile rule with default configuration
35    pub fn new() -> Self {
36        let mut required_targets = HashMap::new();
37
38        required_targets.insert(
39            "test-fast".to_string(),
40            TargetSpec {
41                pattern: Some("cargo nextest run --lib".to_string()),
42                description: "Fast unit tests".to_string(),
43            },
44        );
45
46        required_targets.insert(
47            "test".to_string(),
48            TargetSpec {
49                pattern: Some("cargo nextest run".to_string()),
50                description: "Standard tests".to_string(),
51            },
52        );
53
54        required_targets.insert(
55            "lint".to_string(),
56            TargetSpec {
57                pattern: Some("cargo clippy".to_string()),
58                description: "Clippy linting".to_string(),
59            },
60        );
61
62        required_targets.insert(
63            "fmt".to_string(),
64            TargetSpec {
65                pattern: Some("cargo fmt".to_string()),
66                description: "Format code".to_string(),
67            },
68        );
69
70        required_targets.insert(
71            "coverage".to_string(),
72            TargetSpec {
73                pattern: Some("cargo llvm-cov".to_string()),
74                description: "Coverage report".to_string(),
75            },
76        );
77
78        Self {
79            required_targets,
80            prohibited_commands: vec!["cargo tarpaulin".to_string(), "cargo-tarpaulin".to_string()],
81        }
82    }
83
84    fn check_required_targets(
85        &self,
86        targets: &HashMap<String, MakefileTarget>,
87        violations: &mut Vec<RuleViolation>,
88        suggestions: &mut Vec<Suggestion>,
89    ) {
90        for (target_name, spec) in &self.required_targets {
91            let Some(target) = targets.get(target_name) else {
92                violations.push(
93                    RuleViolation::new("MK-002", format!("Missing required target: {target_name}"))
94                        .with_severity(ViolationLevel::Error)
95                        .with_location("Makefile".to_string())
96                        .with_diff(format!("{target_name}: <command>"), "(not defined)".to_string())
97                        .fixable(),
98                );
99                continue;
100            };
101
102            if let Some(pattern) = &spec.pattern {
103                let has_pattern = target.commands.iter().any(|cmd| cmd.contains(pattern));
104                if !has_pattern {
105                    let msg = format!(
106                        "Target '{target_name}' should include '{pattern}' for {}",
107                        spec.description
108                    );
109                    suggestions.push(Suggestion::new(msg).with_location("Makefile".to_string()));
110                }
111            }
112
113            self.check_prohibited_in_target(target_name, &target.commands, violations);
114        }
115    }
116
117    fn check_prohibited_in_target(
118        &self,
119        target_name: &str,
120        cmds: &[String],
121        violations: &mut Vec<RuleViolation>,
122    ) {
123        for prohibited in &self.prohibited_commands {
124            if cmds.iter().any(|cmd| cmd.contains(prohibited)) {
125                let msg = format!("Target '{target_name}' uses prohibited command: {prohibited}");
126                let diff_left = format!("cargo llvm-cov (for {target_name})");
127                violations.push(
128                    RuleViolation::new("MK-003", msg)
129                        .with_severity(ViolationLevel::Critical)
130                        .with_location("Makefile".to_string())
131                        .with_diff(diff_left, prohibited.to_string()),
132                );
133            }
134        }
135    }
136
137    fn check_all_prohibited(
138        &self,
139        targets: &HashMap<String, MakefileTarget>,
140        violations: &mut Vec<RuleViolation>,
141    ) {
142        for target in targets.values() {
143            if self.required_targets.contains_key(&target.name) {
144                continue;
145            }
146            self.check_prohibited_in_target(&target.name, &target.commands, violations);
147        }
148    }
149
150    /// Parse a Makefile and extract targets
151    fn parse_makefile(&self, path: &Path) -> anyhow::Result<HashMap<String, MakefileTarget>> {
152        let content = std::fs::read_to_string(path)?;
153        let mut targets = HashMap::new();
154        let mut current_target: Option<String> = None;
155        let mut current_commands: Vec<String> = Vec::new();
156
157        for line in content.lines() {
158            // Skip comments and empty lines
159            if line.starts_with('#') || line.trim().is_empty() {
160                continue;
161            }
162
163            // Check for target definition (name: [dependencies])
164            if !line.starts_with('\t') && !line.starts_with(' ') && line.contains(':') {
165                // Save previous target
166                if let Some(name) = current_target.take() {
167                    targets.insert(
168                        name.clone(),
169                        MakefileTarget { name, commands: std::mem::take(&mut current_commands) },
170                    );
171                }
172
173                // Parse new target
174                let parts: Vec<&str> = line.splitn(2, ':').collect();
175                if !parts.is_empty() {
176                    let target_name = parts[0].trim();
177                    // Skip .PHONY and similar
178                    if !target_name.starts_with('.') {
179                        current_target = Some(target_name.to_string());
180                    }
181                }
182            } else if (line.starts_with('\t') || line.starts_with(' ')) && current_target.is_some()
183            {
184                // Command line for current target
185                let cmd = line.trim();
186                if !cmd.is_empty() {
187                    current_commands.push(cmd.to_string());
188                }
189            }
190        }
191
192        // Save last target
193        if let Some(name) = current_target {
194            targets.insert(name.clone(), MakefileTarget { name, commands: current_commands });
195        }
196
197        Ok(targets)
198    }
199}
200
201#[derive(Debug)]
202struct MakefileTarget {
203    name: String,
204    commands: Vec<String>,
205}
206
207impl StackComplianceRule for MakefileRule {
208    fn id(&self) -> &'static str {
209        "makefile-targets"
210    }
211
212    fn description(&self) -> &'static str {
213        "Ensures consistent Makefile targets across stack projects"
214    }
215
216    fn help(&self) -> Option<&str> {
217        Some(
218            "Required targets: test-fast, test, lint, fmt, coverage\n\
219             Prohibited commands: cargo tarpaulin",
220        )
221    }
222
223    fn category(&self) -> RuleCategory {
224        RuleCategory::Build
225    }
226
227    fn check(&self, project_path: &Path) -> anyhow::Result<RuleResult> {
228        let makefile_path = project_path.join("Makefile");
229
230        if !makefile_path.exists() {
231            return Ok(RuleResult::fail(vec![RuleViolation::new("MK-001", "Makefile not found")
232                .with_severity(ViolationLevel::Error)
233                .with_location(project_path.display().to_string())
234                .fixable()]));
235        }
236
237        let targets = self.parse_makefile(&makefile_path)?;
238        let mut violations = Vec::new();
239        let mut suggestions = Vec::new();
240
241        self.check_required_targets(&targets, &mut violations, &mut suggestions);
242        self.check_all_prohibited(&targets, &mut violations);
243
244        if violations.is_empty() {
245            if suggestions.is_empty() {
246                Ok(RuleResult::pass())
247            } else {
248                Ok(RuleResult::pass_with_suggestions(suggestions))
249            }
250        } else {
251            Ok(RuleResult::fail(violations))
252        }
253    }
254
255    fn can_fix(&self) -> bool {
256        true
257    }
258
259    fn fix(&self, project_path: &Path) -> anyhow::Result<FixResult> {
260        let makefile_path = project_path.join("Makefile");
261        let mut fixed = 0;
262        let mut details = Vec::new();
263
264        // Read existing content or start fresh
265        let mut content = if makefile_path.exists() {
266            std::fs::read_to_string(&makefile_path)?
267        } else {
268            ".PHONY: test-fast test lint fmt coverage build\n\n".to_string()
269        };
270
271        // Parse current targets
272        let existing_targets = if makefile_path.exists() {
273            self.parse_makefile(&makefile_path)?
274        } else {
275            HashMap::new()
276        };
277
278        // Add missing targets
279        for (target_name, spec) in &self.required_targets {
280            if !existing_targets.contains_key(target_name) {
281                let default_cmd = spec.pattern.as_deref().unwrap_or("@echo 'TODO'");
282                content.push_str(&format!("\n{0}:\n\t{1}\n", target_name, default_cmd));
283                fixed += 1;
284                details.push(FixDetail::Fixed {
285                    code: "MK-002".to_string(),
286                    description: format!("Added target '{}'", target_name),
287                });
288            }
289        }
290
291        // Write updated content
292        if fixed > 0 {
293            std::fs::write(&makefile_path, content)?;
294        }
295
296        Ok(FixResult::success(fixed).with_detail(FixDetail::Fixed {
297            code: "MK-000".to_string(),
298            description: format!("Updated Makefile with {} targets", fixed),
299        }))
300    }
301}
302
303#[cfg(test)]
304mod tests {
305    use super::*;
306    use tempfile::TempDir;
307
308    #[test]
309    fn test_makefile_rule_creation() {
310        let rule = MakefileRule::new();
311        assert_eq!(rule.id(), "makefile-targets");
312        assert!(rule.required_targets.contains_key("test-fast"));
313        assert!(rule.required_targets.contains_key("coverage"));
314    }
315
316    #[test]
317    fn test_missing_makefile() {
318        let temp = TempDir::new().unwrap();
319        let rule = MakefileRule::new();
320        let result = rule.check(temp.path()).unwrap();
321        assert!(!result.passed);
322        assert_eq!(result.violations[0].code, "MK-001");
323    }
324
325    #[test]
326    fn test_complete_makefile() {
327        let temp = TempDir::new().unwrap();
328        let makefile = temp.path().join("Makefile");
329
330        let content = r#"
331.PHONY: test-fast test lint fmt coverage
332
333test-fast:
334	cargo nextest run --lib
335
336test:
337	cargo nextest run
338
339lint:
340	cargo clippy -- -D warnings
341
342fmt:
343	cargo fmt --check
344
345coverage:
346	cargo llvm-cov --html
347"#;
348        std::fs::write(&makefile, content).unwrap();
349
350        let rule = MakefileRule::new();
351        let result = rule.check(temp.path()).unwrap();
352        assert!(result.passed, "Should pass: {:?}", result.violations);
353    }
354
355    #[test]
356    fn test_missing_target() {
357        let temp = TempDir::new().unwrap();
358        let makefile = temp.path().join("Makefile");
359
360        let content = r#"
361test:
362	cargo test
363
364lint:
365	cargo clippy
366"#;
367        std::fs::write(&makefile, content).unwrap();
368
369        let rule = MakefileRule::new();
370        let result = rule.check(temp.path()).unwrap();
371        assert!(!result.passed);
372        // Should have violations for test-fast, fmt, coverage
373        assert!(!result.violations.is_empty());
374    }
375
376    #[test]
377    fn test_prohibited_command() {
378        let temp = TempDir::new().unwrap();
379        let makefile = temp.path().join("Makefile");
380
381        let content = r#"
382coverage:
383	cargo tarpaulin --out Html
384"#;
385        std::fs::write(&makefile, content).unwrap();
386
387        let rule = MakefileRule::new();
388        let result = rule.check(temp.path()).unwrap();
389        assert!(!result.passed);
390        assert!(result.violations.iter().any(|v| v.code == "MK-003"));
391    }
392
393    #[test]
394    fn test_fix_creates_makefile() {
395        let temp = TempDir::new().unwrap();
396        let rule = MakefileRule::new();
397
398        // Verify no makefile exists
399        assert!(!temp.path().join("Makefile").exists());
400
401        let result = rule.fix(temp.path()).unwrap();
402        assert!(result.success);
403        assert!(temp.path().join("Makefile").exists());
404    }
405
406    #[test]
407    fn test_can_fix_returns_true() {
408        let rule = MakefileRule::new();
409        assert!(rule.can_fix());
410    }
411
412    #[test]
413    fn test_rule_metadata() {
414        let rule = MakefileRule::new();
415        assert_eq!(rule.id(), "makefile-targets");
416        assert!(!rule.description().is_empty());
417        assert_eq!(rule.category(), RuleCategory::Build);
418    }
419
420    #[test]
421    fn test_fix_with_existing_makefile() {
422        let temp = TempDir::new().unwrap();
423        let makefile = temp.path().join("Makefile");
424
425        // Create a minimal Makefile
426        let content = "test:\n\tcargo test\n";
427        std::fs::write(&makefile, content).unwrap();
428
429        let rule = MakefileRule::new();
430        let result = rule.fix(temp.path()).unwrap();
431
432        // Should succeed and add missing targets
433        assert!(result.success);
434        let new_content = std::fs::read_to_string(&makefile).unwrap();
435        assert!(new_content.contains("test-fast:"));
436    }
437
438    #[test]
439    fn test_prohibited_command_in_non_required_target() {
440        let temp = TempDir::new().unwrap();
441        let makefile = temp.path().join("Makefile");
442
443        let content = r#"
444custom-coverage:
445	cargo tarpaulin --out Html
446
447test-fast:
448	cargo nextest run --lib
449"#;
450        std::fs::write(&makefile, content).unwrap();
451
452        let rule = MakefileRule::new();
453        let result = rule.check(temp.path()).unwrap();
454        // Should fail because of prohibited command in custom-coverage
455        assert!(!result.passed);
456        assert!(result.violations.iter().any(|v| v.code == "MK-003"));
457    }
458
459    #[test]
460    fn test_target_without_expected_pattern() {
461        let temp = TempDir::new().unwrap();
462        let makefile = temp.path().join("Makefile");
463
464        // lint target without clippy
465        let content = r#"
466lint:
467	echo "linting"
468
469test-fast:
470	cargo nextest run --lib
471
472test:
473	cargo test
474
475fmt:
476	cargo fmt --check
477
478coverage:
479	cargo llvm-cov
480"#;
481        std::fs::write(&makefile, content).unwrap();
482
483        let rule = MakefileRule::new();
484        let result = rule.check(temp.path()).unwrap();
485        // Should pass but have suggestions
486        assert!(result.passed);
487        assert!(!result.suggestions.is_empty());
488    }
489
490    #[test]
491    fn test_default_trait() {
492        let rule = MakefileRule::default();
493        assert_eq!(rule.id(), "makefile-targets");
494    }
495}