From 9b25b917a764803e663b0155fab8cc779fd63198 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Mon, 13 Jan 2025 22:47:00 -0600 Subject: [PATCH] sqf: Add lint (S25) for count array comparisons --- libs/sqf/src/analyze/inspector/commands.rs | 37 ++++- libs/sqf/src/analyze/inspector/mod.rs | 9 ++ .../analyze/lints/s21_invalid_comparisons.rs | 4 +- .../src/analyze/lints/s25_count_array_comp.rs | 135 ++++++++++++++++++ libs/sqf/tests/inspector.rs | 5 +- libs/sqf/tests/inspector/test_1.sqf | 2 + libs/sqf/tests/lints.rs | 1 + libs/sqf/tests/lints/s25_count_array_comp.sqf | 2 + .../lints__simple_s25_count_array_comp.snap | 11 ++ 9 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 libs/sqf/src/analyze/lints/s25_count_array_comp.rs create mode 100644 libs/sqf/tests/lints/s25_count_array_comp.sqf create mode 100644 libs/sqf/tests/snapshots/lints__simple_s25_count_array_comp.snap diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index 86aefd08..9f4dabfa 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -2,7 +2,11 @@ use std::{collections::HashSet, ops::Range}; -use crate::{analyze::inspector::VarSource, parser::database::Database, Expression}; +use crate::{ + analyze::inspector::{Issue, VarSource}, + parser::database::Database, + Expression, +}; use super::{game_value::GameValue, SciptScope}; @@ -403,4 +407,35 @@ impl SciptScope { } return_value } + + pub fn cmd_eqx_count_lint( + &mut self, + lhs: &Box, + rhs: &Box, + source: &Range, + database: &Database, + equal_zero: bool, + ) { + let Expression::Number(float_ord::FloatOrd(0.0), _) = **rhs else { + return; + }; + let Expression::UnaryCommand(crate::UnaryCommand::Named(ref lhs_cmd), ref count_input, _) = + **lhs + else { + return; + }; + if lhs_cmd != "count" { + return; + } + let count_input_set = self.eval_expression(count_input, database); + if count_input_set.is_empty() + || !count_input_set + .iter() + .all(|arr| matches!(arr, GameValue::Array(_))) + { + return; + } + self.errors + .insert(Issue::CountArrayComparison(equal_zero, source.clone())); + } } diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 0313a736..2ae1b8c5 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -26,6 +26,7 @@ pub enum Issue { Unused(String, VarSource), Shadowed(String, Range), NotPrivate(String, Range), + CountArrayComparison(bool, Range), } #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -317,6 +318,14 @@ impl SciptScope { None } BinaryCommand::Else => Some(self.cmd_b_else(&lhs_set, &rhs_set)), + BinaryCommand::Eq => { + self.cmd_eqx_count_lint(&lhs, &rhs, source, database, true); + None + } + BinaryCommand::Greater | BinaryCommand::NotEq => { + self.cmd_eqx_count_lint(&lhs, &rhs, source, database, false); + None + } BinaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { "params" => Some(self.cmd_generic_params(&rhs_set)), "call" => { diff --git a/libs/sqf/src/analyze/lints/s21_invalid_comparisons.rs b/libs/sqf/src/analyze/lints/s21_invalid_comparisons.rs index 4ecbec95..8c905840 100644 --- a/libs/sqf/src/analyze/lints/s21_invalid_comparisons.rs +++ b/libs/sqf/src/analyze/lints/s21_invalid_comparisons.rs @@ -73,8 +73,8 @@ impl LintRunner for Runner { return Vec::new(); } - let comparisions = extract_comparisons(arg); - let flat = flatten_comparisons(comparisions); + let comparisons = extract_comparisons(arg); + let flat = flatten_comparisons(comparisons); let issues = find_issues(flat); issues .into_iter() diff --git a/libs/sqf/src/analyze/lints/s25_count_array_comp.rs b/libs/sqf/src/analyze/lints/s25_count_array_comp.rs new file mode 100644 index 00000000..c27c5dee --- /dev/null +++ b/libs/sqf/src/analyze/lints/s25_count_array_comp.rs @@ -0,0 +1,135 @@ +use crate::{ + analyze::{inspector::Issue, LintData}, + Statements, +}; +use hemtt_common::config::LintConfig; +use hemtt_workspace::{ + lint::{AnyLintRunner, Lint, LintRunner}, + reporting::{Code, Codes, Diagnostic, Processed, Severity}, +}; +use std::{ops::Range, sync::Arc}; + +crate::analyze::lint!(LintS25CountArrayComparison); + +impl Lint for LintS25CountArrayComparison { + fn ident(&self) -> &'static str { + "count_array_comp" + } + fn sort(&self) -> u32 { + 250 + } + fn description(&self) -> &'static str { + "Count Array Comp" + } + fn documentation(&self) -> &'static str { + r"### Example + +**Incorrect** +```sqf +count [] == 0 +``` + +### Explanation + +Checks for unoptimized `count array` checks." + } + fn default_config(&self) -> LintConfig { + LintConfig::help().with_enabled(true) + } + fn runners(&self) -> Vec>> { + vec![Box::new(Runner)] + } +} + +pub struct Runner; +impl LintRunner for Runner { + type Target = Statements; + fn run( + &self, + _project: Option<&hemtt_common::config::ProjectConfig>, + config: &hemtt_common::config::LintConfig, + processed: Option<&hemtt_workspace::reporting::Processed>, + target: &Statements, + _data: &LintData, + ) -> hemtt_workspace::reporting::Codes { + if target.issues().is_empty() { + return Vec::new(); + }; + let Some(processed) = processed else { + return Vec::new(); + }; + let mut errors: Codes = Vec::new(); + for issue in target.issues() { + if let Issue::CountArrayComparison(equal_zero, range) = issue { + errors.push(Arc::new(CodeS25CountArrayComp::new( + range.to_owned(), + equal_zero.to_owned(), + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS25CountArrayComp { + span: Range, + equal_zero: bool, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS25CountArrayComp { + fn ident(&self) -> &'static str { + "L-S25" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#count_array_comp") + } + /// Top message + fn message(&self) -> String { + format!("count array comparison") + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + if self.equal_zero { + Some("use `isEqualTo []`".into()) + } else { + Some("use `isNotEqualTo []`".into()) + } + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS25CountArrayComp { + #[must_use] + pub fn new( + span: Range, + equal_zero: bool, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + equal_zero, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index 2e5c630a..ca2f4b6e 100644 --- a/libs/sqf/tests/inspector.rs +++ b/libs/sqf/tests/inspector.rs @@ -37,7 +37,7 @@ mod tests { pub fn test_1() { let (_pro, sqf, _database) = get_statements("test_1.sqf"); let result = sqf.issues(); - assert_eq!(result.len(), 15); + assert_eq!(result.len(), 16); // Order not guarenteed assert!(result.iter().any(|i| { if let Issue::InvalidArgs(cmd, _) = i { @@ -144,6 +144,9 @@ mod tests { false } })); + assert!(result + .iter() + .any(|i| { matches!(i, Issue::CountArrayComparison(true, _)) })); } #[test] diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index 6505d61c..25204ff7 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -138,3 +138,5 @@ format _varP; [_test13] call some_func; // undef, is orphan }, player] call unknown_fnc_Usage; +private _test14 = str 12345678 splitString "5"; +if (count _test14 == 0) then { call b }; diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 530a0ae4..0b984337 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -43,6 +43,7 @@ lint!(s21_invalid_comparisons, true); lint!(s22_this_call, true); lint!(s23_reassign_reserved_variable, true); lint!(s24_marker_spam, true); +lint!(s25_count_array_comp, false); fn lint(file: &str, ignore_inspector: bool) -> String { let folder = std::path::PathBuf::from(ROOT); diff --git a/libs/sqf/tests/lints/s25_count_array_comp.sqf b/libs/sqf/tests/lints/s25_count_array_comp.sqf new file mode 100644 index 00000000..f755face --- /dev/null +++ b/libs/sqf/tests/lints/s25_count_array_comp.sqf @@ -0,0 +1,2 @@ +private _z = []; +if (count _z == 0) then { call b }; diff --git a/libs/sqf/tests/snapshots/lints__simple_s25_count_array_comp.snap b/libs/sqf/tests/snapshots/lints__simple_s25_count_array_comp.snap new file mode 100644 index 00000000..431a1034 --- /dev/null +++ b/libs/sqf/tests/snapshots/lints__simple_s25_count_array_comp.snap @@ -0,0 +1,11 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s25_count_array_comp), false)" +--- +help[L-S25]: count array comparison + ┌─ s25_count_array_comp.sqf:2:14 + │ +2 │ if (count _z == 0) then { call b }; + │ ^^ + │ + = note: use `isEqualTo []`