diff --git a/libs/sqf/src/analyze/lints/s52_duplicate_case.rs b/libs/sqf/src/analyze/lints/s52_duplicate_case.rs new file mode 100644 index 00000000..dff3b8e0 --- /dev/null +++ b/libs/sqf/src/analyze/lints/s52_duplicate_case.rs @@ -0,0 +1,222 @@ +use std::{ops::Range, sync::Arc}; + +use hemtt_common::config::LintConfig; +use hemtt_workspace::{ + lint::{AnyLintRunner, Lint, LintRunner}, + reporting::{Code, Codes, Diagnostic, Label, Processed, Severity}, + WorkspacePath, +}; + +use crate::{analyze::LintData, BinaryCommand, Expression, Statement, UnaryCommand}; + +crate::analyze::lint!(LintS52DuplicateCase); + +impl Lint for LintS52DuplicateCase { + fn ident(&self) -> &'static str { + "duplicate_case" + } + + fn sort(&self) -> u32 { + 520 + } + + fn description(&self) -> &'static str { + "Checks for duplicate cases in switch statements" + } + + fn documentation(&self) -> &'static str { + r#"### Example + +**Incorrect** +```sqf +switch (_value) do { + case 1: { "one" }; + case 2: { "two" }; + case 1: { "one again" }; +}; +``` + +**Correct** +```sqf +switch (_value) do { + case 1: { "one" }; + case 2: { "two" }; + case 3: { "three" }; +}; +``` + +### Explanation + +Having duplicate case labels in a switch statement is likely a mistake. Only the first case will be executed, and the duplicate case will never be reached. +"# + } + + fn default_config(&self) -> LintConfig { + LintConfig::help() + } + + fn runners(&self) -> Vec>> { + vec![Box::new(Runner)] + } +} + +struct Runner; +impl LintRunner for Runner { + type Target = crate::Expression; + + fn run( + &self, + _project: Option<&hemtt_common::config::ProjectConfig>, + config: &LintConfig, + processed: Option<&hemtt_workspace::reporting::Processed>, + _runtime: &hemtt_common::config::RuntimeArguments, + target: &Self::Target, + _data: &LintData, + ) -> Codes { + let Some(processed) = processed else { + return Vec::new(); + }; + + // Look for switch (expr) do { ... } + let Expression::BinaryCommand(BinaryCommand::Named(cmd), lhs, rhs, _) = target else { + return Vec::new(); + }; + + if !cmd.eq_ignore_ascii_case("do") { + return Vec::new(); + } + + // Check if this is a switch statement + let Expression::UnaryCommand(UnaryCommand::Named(unary), _, _) = lhs.as_ref() else { + return Vec::new(); + }; + + if !unary.eq_ignore_ascii_case("switch") { + return Vec::new(); + } + + // The right side should be a Code block containing case statements + let Expression::Code(body) = rhs.as_ref() else { + return Vec::new(); + }; + + // Check if this is a switch statement by looking for case expressions + let mut case_values: Vec<(String, Range)> = Vec::new(); + let mut codes: Codes = Vec::new(); + + for body_statement in body.content() { + let Statement::Expression(case_expr, _) = body_statement else { + continue; + }; + + let case = if let Expression::BinaryCommand(BinaryCommand::Associate, left, _right, _) = case_expr { + left + } else { + case_expr + }; + if let Expression::UnaryCommand(UnaryCommand::Named(name), value_expr, _) = case + && name.eq_ignore_ascii_case("case") { + let case_source = value_expr.source(false); + let case_span = value_expr.span(); + + for (existing_value, existing_span) in &case_values { + if existing_value == &case_source { + codes.push(Arc::new(CodeS52DuplicateCase::new( + case_span.clone(), + case_source.clone(), + existing_span.clone(), + processed, + config.severity(), + ))); + } + } + + case_values.push((case_source, case_span)); + } + } + + codes + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS52DuplicateCase { + span: Range, + value: String, + first_span: Range, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS52DuplicateCase { + fn ident(&self) -> &'static str { + "L-S52" + } + + fn link(&self) -> Option<&str> { + Some("/lints/sqf.html#duplicate_case") + } + + fn severity(&self) -> Severity { + self.severity + } + + fn message(&self) -> String { + format!("Duplicate case `{}` in switch statement", self.value) + } + + fn label_message(&self) -> String { + "duplicate case".to_string() + } + + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS52DuplicateCase { + #[must_use] + pub fn new( + span: Range, + value: String, + first_span: Range, + processed: &Processed, + severity: Severity, + ) -> Self { + Self { + span, + value, + first_span, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + + fn generate_processed(mut self, processed: &Processed) -> Self { + let Some(mut diag) = Diagnostic::from_code_processed(&self, self.span.clone(), processed) + else { + return self; + }; + + // Try to get info about the first span + if let Some((path, span)) = get_span_info(self.first_span.clone(), processed) { + diag = diag.with_label( + Label::secondary(path, span) + .with_message("first case here"), + ); + } + self.diagnostic = Some(diag); + self + } +} + +fn get_span_info(span: Range, processed: &Processed) -> Option<(WorkspacePath, Range)> { + let map_start = processed.mapping(span.start)?; + let map_end = processed.mapping(span.end)?; + let map_file = processed.source(map_start.source())?; + Some(( + map_file.0.clone(), + map_start.original_start()..map_end.original_end(), + )) +} diff --git a/libs/sqf/src/lib.rs b/libs/sqf/src/lib.rs index bd7e4a0c..2cf7df1a 100644 --- a/libs/sqf/src/lib.rs +++ b/libs/sqf/src/lib.rs @@ -442,6 +442,7 @@ pub enum BinaryCommand { LessEq, /// `>>` ConfigPath, + /// `:` Associate, Else, Add, diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 0fbfafec..944250cd 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -87,6 +87,7 @@ lint!(s48_is_equal_type_all, true); lint!(s49_count_type, true); lint!(s50_count_side, true); lint!(s51_push_back_unique, true); +lint!(s52_duplicate_case, true); #[test] fn test_s29_function_undefined() { diff --git a/libs/sqf/tests/lints/s52_duplicate_case.sqf b/libs/sqf/tests/lints/s52_duplicate_case.sqf new file mode 100644 index 00000000..76c82e47 --- /dev/null +++ b/libs/sqf/tests/lints/s52_duplicate_case.sqf @@ -0,0 +1,43 @@ +// Test duplicate case detection + +// Simple duplicate +switch (_value) do { + case 1: { "one" }; + case 2: { "two" }; + case 1: { "one again" }; +}; + +// Multiple duplicates +switch (_x) do { + case "a": { 1 }; + case "b": { 2 }; + case "a": { 3 }; + case "b": { 4 }; +}; + +// Variable case (should be detected) +switch (_var) do { + case _distBottom: { [_x, 0] }; + case _distRight: { [_worldSize, _y] }; + case _distBottom: { [_x, _worldSize] }; +}; + +// Default cases are OK (not considered cases for duplication) +switch (_value) do { + case 1: { "one" }; + case 2: { "two" }; + default { "other" }; +}; + +// Single case is OK +switch (_value) do { + case 1: { "one" }; +}; + +// multiple cases +switch test do { + case 1; + case 2: { "X" }; + case 1: { "Y"}; +}; + diff --git a/libs/sqf/tests/snapshots/lints__simple_s52_duplicate_case.snap b/libs/sqf/tests/snapshots/lints__simple_s52_duplicate_case.snap new file mode 100644 index 00000000..5a4c99ea --- /dev/null +++ b/libs/sqf/tests/snapshots/lints__simple_s52_duplicate_case.snap @@ -0,0 +1,52 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s52_duplicate_case), true).0" +--- +help[L-S52]: Duplicate case `"a"` in switch statement + ┌─ s52_duplicate_case.sqf:14:10 + │ +12 │ case "a": { 1 }; + │ ---- first case here +13 │ case "b": { 2 }; +14 │ case "a": { 3 }; + │ ^^^ duplicate case + + +help[L-S52]: Duplicate case `"b"` in switch statement + ┌─ s52_duplicate_case.sqf:15:10 + │ +13 │ case "b": { 2 }; + │ ---- first case here +14 │ case "a": { 3 }; +15 │ case "b": { 4 }; + │ ^^^ duplicate case + + +help[L-S52]: Duplicate case `1` in switch statement + ┌─ s52_duplicate_case.sqf:7:10 + │ +5 │ case 1: { "one" }; + │ -- first case here +6 │ case 2: { "two" }; +7 │ case 1: { "one again" }; + │ ^ duplicate case + + +help[L-S52]: Duplicate case `1` in switch statement + ┌─ s52_duplicate_case.sqf:41:10 + │ +39 │ case 1; + │ -- first case here +40 │ case 2: { "X" }; +41 │ case 1: { "Y"}; + │ ^ duplicate case + + +help[L-S52]: Duplicate case `_distBottom` in switch statement + ┌─ s52_duplicate_case.sqf:22:10 + │ +20 │ case _distBottom: { [_x, 0] }; + │ ------------ first case here +21 │ case _distRight: { [_worldSize, _y] }; +22 │ case _distBottom: { [_x, _worldSize] }; + │ ^^^^^^^^^^^ duplicate case