/home/noah/src/ruchy/src/lints/mod.rs
Line | Count | Source |
1 | | /// Custom lint rules for Ruchy code quality |
2 | | use crate::frontend::ast::{Expr, ExprKind}; |
3 | | use thiserror::Error; |
4 | | |
5 | | #[derive(Debug, Error)] |
6 | | pub enum LintViolation { |
7 | | #[error("{location}: {message} (severity: {severity:?})")] |
8 | | Violation { |
9 | | location: String, |
10 | | message: String, |
11 | | severity: Severity, |
12 | | suggestion: Option<String>, |
13 | | }, |
14 | | } |
15 | | |
16 | | #[derive(Debug, Clone, Copy, PartialEq, Eq)] |
17 | | pub enum Severity { |
18 | | Error, |
19 | | Warning, |
20 | | Info, |
21 | | } |
22 | | |
23 | | /// Trait for implementing lint rules |
24 | | pub trait LintRule: Send + Sync { |
25 | | fn name(&self) -> &str; |
26 | | fn check_expression(&self, expr: &Expr) -> Vec<LintViolation>; |
27 | | } |
28 | | |
29 | | /// Main linter that runs all rules |
30 | | pub struct RuchyLinter { |
31 | | rules: Vec<Box<dyn LintRule>>, |
32 | | } |
33 | | |
34 | | impl Default for RuchyLinter { |
35 | 0 | fn default() -> Self { |
36 | 0 | Self::new() |
37 | 0 | } |
38 | | } |
39 | | |
40 | | impl RuchyLinter { |
41 | 0 | pub fn new() -> Self { |
42 | 0 | let rules: Vec<Box<dyn LintRule>> = vec![ |
43 | 0 | Box::new(ComplexityRule::default()), |
44 | 0 | Box::new(NoDebugPrintRule), |
45 | | ]; |
46 | | |
47 | 0 | Self { rules } |
48 | 0 | } |
49 | | |
50 | 0 | pub fn add_rule(&mut self, rule: Box<dyn LintRule>) { |
51 | 0 | self.rules.push(rule); |
52 | 0 | } |
53 | | |
54 | 0 | pub fn lint(&self, expr: &Expr) -> Vec<LintViolation> { |
55 | 0 | let mut violations = Vec::new(); |
56 | | |
57 | 0 | for rule in &self.rules { |
58 | 0 | violations.extend(rule.check_expression(expr)); |
59 | 0 | } |
60 | | |
61 | 0 | violations |
62 | 0 | } |
63 | | } |
64 | | |
65 | | /// Rule: Cyclomatic complexity limit |
66 | | #[derive(Default)] |
67 | | struct ComplexityRule { |
68 | | max_complexity: usize, |
69 | | } |
70 | | |
71 | | impl ComplexityRule { |
72 | | #[allow(clippy::only_used_in_recursion)] |
73 | 0 | fn calculate_complexity(&self, expr: &Expr) -> usize { |
74 | 0 | match &expr.kind { |
75 | | ExprKind::If { |
76 | 0 | condition, |
77 | 0 | then_branch, |
78 | 0 | else_branch, |
79 | | } => { |
80 | 0 | 1 + self.calculate_complexity(condition) |
81 | 0 | + self.calculate_complexity(then_branch) |
82 | 0 | + else_branch |
83 | 0 | .as_ref() |
84 | 0 | .map_or(0, |e| self.calculate_complexity(e)) |
85 | | } |
86 | 0 | ExprKind::Match { expr, arms } => { |
87 | 0 | 1 + self.calculate_complexity(expr) |
88 | 0 | + arms |
89 | 0 | .iter() |
90 | 0 | .map(|arm| self.calculate_complexity(&arm.body)) |
91 | 0 | .sum::<usize>() |
92 | | } |
93 | 0 | ExprKind::While { condition, body } => { |
94 | 0 | 1 + self.calculate_complexity(condition) + self.calculate_complexity(body) |
95 | | } |
96 | 0 | ExprKind::For { iter, body, .. } => { |
97 | 0 | 1 + self.calculate_complexity(iter) + self.calculate_complexity(body) |
98 | | } |
99 | 0 | ExprKind::Binary { left, right, .. } => { |
100 | 0 | self.calculate_complexity(left) + self.calculate_complexity(right) |
101 | | } |
102 | 0 | _ => 0, |
103 | | } |
104 | 0 | } |
105 | | } |
106 | | |
107 | | impl LintRule for ComplexityRule { |
108 | 0 | fn name(&self) -> &'static str { |
109 | 0 | "complexity" |
110 | 0 | } |
111 | | |
112 | 0 | fn check_expression(&self, expr: &Expr) -> Vec<LintViolation> { |
113 | 0 | let mut violations = Vec::new(); |
114 | 0 | let max = if self.max_complexity == 0 { |
115 | 0 | 10 |
116 | | } else { |
117 | 0 | self.max_complexity |
118 | | }; |
119 | | |
120 | 0 | let complexity = self.calculate_complexity(expr); |
121 | 0 | if complexity > max { |
122 | 0 | violations.push(LintViolation::Violation { |
123 | 0 | location: format!("position {}", expr.span.start), |
124 | 0 | message: format!("Cyclomatic complexity is {complexity} (max: {max})"), |
125 | 0 | severity: Severity::Warning, |
126 | 0 | suggestion: Some("Consider breaking this into smaller functions".to_string()), |
127 | 0 | }); |
128 | 0 | } |
129 | | |
130 | 0 | violations |
131 | 0 | } |
132 | | } |
133 | | |
134 | | /// Rule: No debug print statements |
135 | | struct NoDebugPrintRule; |
136 | | |
137 | | impl LintRule for NoDebugPrintRule { |
138 | 0 | fn name(&self) -> &'static str { |
139 | 0 | "no_debug_print" |
140 | 0 | } |
141 | | |
142 | 0 | fn check_expression(&self, expr: &Expr) -> Vec<LintViolation> { |
143 | 0 | match &expr.kind { |
144 | 0 | ExprKind::Call { func, .. } => { |
145 | 0 | if let ExprKind::Identifier(name) = &func.kind { |
146 | 0 | if name == "dbg" || name == "debug_print" { |
147 | 0 | vec![LintViolation::Violation { |
148 | 0 | location: format!("position {}", expr.span.start), |
149 | 0 | message: "Debug print statement found".to_string(), |
150 | 0 | severity: Severity::Warning, |
151 | 0 | suggestion: Some( |
152 | 0 | "Remove debug statements before committing".to_string(), |
153 | 0 | ), |
154 | 0 | }] |
155 | | } else { |
156 | 0 | vec![] |
157 | | } |
158 | | } else { |
159 | 0 | vec![] |
160 | | } |
161 | | } |
162 | 0 | _ => vec![], |
163 | | } |
164 | 0 | } |
165 | | } |
166 | | |
167 | | #[cfg(test)] |
168 | | mod tests { |
169 | | use super::*; |
170 | | use crate::frontend::ast::{Expr, ExprKind, Span, Literal, BinaryOp}; |
171 | | |
172 | | fn make_test_expr(kind: ExprKind) -> Expr { |
173 | | Expr { |
174 | | kind, |
175 | | span: Span::new(0, 10), |
176 | | attributes: vec![], |
177 | | } |
178 | | } |
179 | | |
180 | | #[test] |
181 | | fn test_linter_creation() { |
182 | | let linter = RuchyLinter::new(); |
183 | | assert_eq!(linter.rules.len(), 2); |
184 | | } |
185 | | |
186 | | #[test] |
187 | | fn test_linter_default() { |
188 | | let linter = RuchyLinter::default(); |
189 | | assert_eq!(linter.rules.len(), 2); |
190 | | } |
191 | | |
192 | | #[test] |
193 | | fn test_add_custom_rule() { |
194 | | struct TestRule; |
195 | | impl LintRule for TestRule { |
196 | | fn name(&self) -> &'static str { |
197 | | "test" |
198 | | } |
199 | | fn check_expression(&self, _expr: &Expr) -> Vec<LintViolation> { |
200 | | vec![] |
201 | | } |
202 | | } |
203 | | |
204 | | let mut linter = RuchyLinter::new(); |
205 | | linter.add_rule(Box::new(TestRule)); |
206 | | assert_eq!(linter.rules.len(), 3); |
207 | | } |
208 | | |
209 | | #[test] |
210 | | fn test_severity_display() { |
211 | | assert_eq!(format!("{:?}", Severity::Error), "Error"); |
212 | | assert_eq!(format!("{:?}", Severity::Warning), "Warning"); |
213 | | assert_eq!(format!("{:?}", Severity::Info), "Info"); |
214 | | } |
215 | | |
216 | | #[test] |
217 | | fn test_severity_equality() { |
218 | | assert_eq!(Severity::Error, Severity::Error); |
219 | | assert_ne!(Severity::Error, Severity::Warning); |
220 | | } |
221 | | |
222 | | #[test] |
223 | | fn test_lint_violation_display() { |
224 | | let violation = LintViolation::Violation { |
225 | | location: "line 5".to_string(), |
226 | | message: "Test violation".to_string(), |
227 | | severity: Severity::Warning, |
228 | | suggestion: Some("Fix it".to_string()), |
229 | | }; |
230 | | |
231 | | let display = violation.to_string(); |
232 | | assert!(display.contains("line 5")); |
233 | | assert!(display.contains("Test violation")); |
234 | | assert!(display.contains("Warning")); |
235 | | } |
236 | | |
237 | | #[test] |
238 | | fn test_lint_violation_without_suggestion() { |
239 | | let violation = LintViolation::Violation { |
240 | | location: "position 10".to_string(), |
241 | | message: "Error found".to_string(), |
242 | | severity: Severity::Error, |
243 | | suggestion: None, |
244 | | }; |
245 | | |
246 | | let display = violation.to_string(); |
247 | | assert!(display.contains("position 10")); |
248 | | assert!(display.contains("Error found")); |
249 | | assert!(display.contains("Error")); |
250 | | } |
251 | | |
252 | | #[test] |
253 | | fn test_complexity_rule_simple() { |
254 | | let rule = ComplexityRule::default(); |
255 | | let expr = make_test_expr(ExprKind::Literal(Literal::Integer(42))); |
256 | | let violations = rule.check_expression(&expr); |
257 | | assert!(violations.is_empty()); |
258 | | } |
259 | | |
260 | | #[test] |
261 | | fn test_complexity_rule_name() { |
262 | | let rule = ComplexityRule::default(); |
263 | | assert_eq!(rule.name(), "complexity"); |
264 | | } |
265 | | |
266 | | #[test] |
267 | | fn test_complexity_rule_if_statement() { |
268 | | let rule = ComplexityRule { max_complexity: 0 }; // Will use default of 10 |
269 | | |
270 | | // Create a simple if statement |
271 | | let if_expr = make_test_expr(ExprKind::If { |
272 | | condition: Box::new(make_test_expr(ExprKind::Literal(Literal::Bool(true)))), |
273 | | then_branch: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(1)))), |
274 | | else_branch: None, |
275 | | }); |
276 | | |
277 | | let violations = rule.check_expression(&if_expr); |
278 | | assert!(violations.is_empty()); // Complexity is 1, under limit of 10 |
279 | | } |
280 | | |
281 | | #[test] |
282 | | fn test_complexity_rule_nested_if_exceeds_limit() { |
283 | | let rule = ComplexityRule { max_complexity: 1 }; |
284 | | |
285 | | // Create nested if statements to exceed complexity of 1 |
286 | | let inner_if = make_test_expr(ExprKind::If { |
287 | | condition: Box::new(make_test_expr(ExprKind::Literal(Literal::Bool(true)))), |
288 | | then_branch: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(1)))), |
289 | | else_branch: None, |
290 | | }); |
291 | | |
292 | | let outer_if = make_test_expr(ExprKind::If { |
293 | | condition: Box::new(make_test_expr(ExprKind::Literal(Literal::Bool(false)))), |
294 | | then_branch: Box::new(inner_if), |
295 | | else_branch: Some(Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(2))))), |
296 | | }); |
297 | | |
298 | | let violations = rule.check_expression(&outer_if); |
299 | | assert!(!violations.is_empty()); |
300 | | assert!(violations[0].to_string().contains("Cyclomatic complexity")); |
301 | | } |
302 | | |
303 | | #[test] |
304 | | fn test_complexity_rule_while_loop() { |
305 | | let rule = ComplexityRule { max_complexity: 5 }; |
306 | | |
307 | | let while_expr = make_test_expr(ExprKind::While { |
308 | | condition: Box::new(make_test_expr(ExprKind::Literal(Literal::Bool(true)))), |
309 | | body: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(42)))), |
310 | | }); |
311 | | |
312 | | let violations = rule.check_expression(&while_expr); |
313 | | assert!(violations.is_empty()); // Complexity is 1, under limit |
314 | | } |
315 | | |
316 | | #[test] |
317 | | fn test_complexity_rule_for_loop() { |
318 | | let rule = ComplexityRule { max_complexity: 5 }; |
319 | | |
320 | | let for_expr = make_test_expr(ExprKind::For { |
321 | | var: "i".to_string(), |
322 | | pattern: None, |
323 | | iter: Box::new(make_test_expr(ExprKind::Range { |
324 | | start: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(0)))), |
325 | | end: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(10)))), |
326 | | inclusive: false, |
327 | | })), |
328 | | body: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(42)))), |
329 | | }); |
330 | | |
331 | | let violations = rule.check_expression(&for_expr); |
332 | | assert!(violations.is_empty()); // Complexity is 1, under limit |
333 | | } |
334 | | |
335 | | #[test] |
336 | | fn test_complexity_rule_binary_operation() { |
337 | | let rule = ComplexityRule { max_complexity: 5 }; |
338 | | |
339 | | let binary_expr = make_test_expr(ExprKind::Binary { |
340 | | left: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(1)))), |
341 | | op: BinaryOp::Add, |
342 | | right: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(2)))), |
343 | | }); |
344 | | |
345 | | let violations = rule.check_expression(&binary_expr); |
346 | | assert!(violations.is_empty()); // Binary operations don't add complexity |
347 | | } |
348 | | |
349 | | #[test] |
350 | | fn test_no_debug_print_rule() { |
351 | | let rule = NoDebugPrintRule; |
352 | | |
353 | | // Test normal function call - no violation |
354 | | let normal_call = make_test_expr(ExprKind::Call { |
355 | | func: Box::new(make_test_expr(ExprKind::Identifier("println".to_string()))), |
356 | | args: vec![], |
357 | | }); |
358 | | assert!(rule.check_expression(&normal_call).is_empty()); |
359 | | |
360 | | // Test debug print - should have violation |
361 | | let debug_call = make_test_expr(ExprKind::Call { |
362 | | func: Box::new(make_test_expr(ExprKind::Identifier("dbg".to_string()))), |
363 | | args: vec![], |
364 | | }); |
365 | | let violations = rule.check_expression(&debug_call); |
366 | | assert_eq!(violations.len(), 1); |
367 | | assert!(violations[0].to_string().contains("Debug print")); |
368 | | |
369 | | // Test debug_print - should have violation |
370 | | let debug_print = make_test_expr(ExprKind::Call { |
371 | | func: Box::new(make_test_expr(ExprKind::Identifier("debug_print".to_string()))), |
372 | | args: vec![], |
373 | | }); |
374 | | let violations = rule.check_expression(&debug_print); |
375 | | assert_eq!(violations.len(), 1); |
376 | | } |
377 | | |
378 | | #[test] |
379 | | fn test_no_debug_print_rule_name() { |
380 | | let rule = NoDebugPrintRule; |
381 | | assert_eq!(rule.name(), "no_debug_print"); |
382 | | } |
383 | | |
384 | | #[test] |
385 | | fn test_linter_runs_all_rules() { |
386 | | let linter = RuchyLinter::new(); |
387 | | |
388 | | // Create expression that violates debug print rule |
389 | | let expr = make_test_expr(ExprKind::Call { |
390 | | func: Box::new(make_test_expr(ExprKind::Identifier("dbg".to_string()))), |
391 | | args: vec![], |
392 | | }); |
393 | | |
394 | | let violations = linter.lint(&expr); |
395 | | assert!(!violations.is_empty()); |
396 | | } |
397 | | |
398 | | #[test] |
399 | | fn test_linter_no_violations() { |
400 | | let linter = RuchyLinter::new(); |
401 | | |
402 | | // Create simple expression with no violations |
403 | | let expr = make_test_expr(ExprKind::Literal(Literal::Integer(42))); |
404 | | |
405 | | let violations = linter.lint(&expr); |
406 | | assert!(violations.is_empty()); |
407 | | } |
408 | | |
409 | | #[test] |
410 | | fn test_complexity_calculation_match() { |
411 | | let rule = ComplexityRule { max_complexity: 10 }; |
412 | | |
413 | | // Match expressions add complexity |
414 | | use crate::frontend::ast::{MatchArm, Pattern}; |
415 | | |
416 | | let arms = vec![ |
417 | | MatchArm { |
418 | | pattern: Pattern::Literal(Literal::Integer(1)), |
419 | | guard: None, |
420 | | body: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(10)))), |
421 | | span: Span::new(0, 10), |
422 | | }, |
423 | | MatchArm { |
424 | | pattern: Pattern::Literal(Literal::Integer(2)), |
425 | | guard: None, |
426 | | body: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(20)))), |
427 | | span: Span::new(0, 10), |
428 | | }, |
429 | | ]; |
430 | | |
431 | | let match_expr = make_test_expr(ExprKind::Match { |
432 | | expr: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(42)))), |
433 | | arms, |
434 | | }); |
435 | | |
436 | | let violations = rule.check_expression(&match_expr); |
437 | | assert!(violations.is_empty()); // Under complexity limit |
438 | | } |
439 | | |
440 | | #[test] |
441 | | fn test_complexity_rule_with_custom_max() { |
442 | | let rule = ComplexityRule { max_complexity: 3 }; |
443 | | |
444 | | // Simple literal should not trigger |
445 | | let expr = make_test_expr(ExprKind::Literal(Literal::Integer(42))); |
446 | | let violations = rule.check_expression(&expr); |
447 | | assert!(violations.is_empty()); |
448 | | } |
449 | | |
450 | | #[test] |
451 | | fn test_no_debug_print_rule_non_call_expression() { |
452 | | let rule = NoDebugPrintRule; |
453 | | |
454 | | // Test with non-call expression |
455 | | let expr = make_test_expr(ExprKind::Literal(Literal::String("test".to_string()))); |
456 | | let violations = rule.check_expression(&expr); |
457 | | assert!(violations.is_empty()); |
458 | | } |
459 | | |
460 | | #[test] |
461 | | fn test_no_debug_print_rule_non_identifier_function() { |
462 | | let rule = NoDebugPrintRule; |
463 | | |
464 | | // Test with non-identifier function (e.g., lambda call) |
465 | | let expr = make_test_expr(ExprKind::Call { |
466 | | func: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(42)))), |
467 | | args: vec![], |
468 | | }); |
469 | | |
470 | | let violations = rule.check_expression(&expr); |
471 | | assert!(violations.is_empty()); |
472 | | } |
473 | | |
474 | | #[test] |
475 | | fn test_lint_violation_debug_formatting() { |
476 | | let violation = LintViolation::Violation { |
477 | | location: "test.ruchy:10:5".to_string(), |
478 | | message: "Complex expression detected".to_string(), |
479 | | severity: Severity::Info, |
480 | | suggestion: None, |
481 | | }; |
482 | | |
483 | | let debug_str = format!("{violation:?}"); |
484 | | assert!(debug_str.contains("Violation")); |
485 | | assert!(debug_str.contains("test.ruchy:10:5")); |
486 | | } |
487 | | |
488 | | #[test] |
489 | | fn test_severity_clone() { |
490 | | let sev1 = Severity::Warning; |
491 | | let sev2 = sev1; |
492 | | assert_eq!(sev1, sev2); |
493 | | } |
494 | | |
495 | | #[test] |
496 | | fn test_complexity_rule_if_with_else() { |
497 | | let rule = ComplexityRule { max_complexity: 5 }; |
498 | | |
499 | | let if_expr = make_test_expr(ExprKind::If { |
500 | | condition: Box::new(make_test_expr(ExprKind::Literal(Literal::Bool(true)))), |
501 | | then_branch: Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(1)))), |
502 | | else_branch: Some(Box::new(make_test_expr(ExprKind::Literal(Literal::Integer(2))))), |
503 | | }); |
504 | | |
505 | | let violations = rule.check_expression(&if_expr); |
506 | | assert!(violations.is_empty()); |
507 | | } |
508 | | |
509 | | #[test] |
510 | | fn test_multiple_violations_from_linter() { |
511 | | let mut linter = RuchyLinter::new(); |
512 | | |
513 | | // Add another rule that always fails |
514 | | struct AlwaysFailRule; |
515 | | impl LintRule for AlwaysFailRule { |
516 | | fn name(&self) -> &'static str { |
517 | | "always_fail" |
518 | | } |
519 | | fn check_expression(&self, expr: &Expr) -> Vec<LintViolation> { |
520 | | vec![LintViolation::Violation { |
521 | | location: format!("position {}", expr.span.start), |
522 | | message: "Always fails".to_string(), |
523 | | severity: Severity::Error, |
524 | | suggestion: None, |
525 | | }] |
526 | | } |
527 | | } |
528 | | |
529 | | linter.add_rule(Box::new(AlwaysFailRule)); |
530 | | |
531 | | // Create expression that also violates debug print rule |
532 | | let expr = make_test_expr(ExprKind::Call { |
533 | | func: Box::new(make_test_expr(ExprKind::Identifier("dbg".to_string()))), |
534 | | args: vec![], |
535 | | }); |
536 | | |
537 | | let violations = linter.lint(&expr); |
538 | | assert!(violations.len() >= 2); // At least 2 violations |
539 | | } |
540 | | } |