diff --git a/libs/sqf/src/analyze/lints/s18_in_vehicle_check.rs b/libs/sqf/src/analyze/lints/s18_in_vehicle_check.rs index 43b43781..9a25bcd0 100644 --- a/libs/sqf/src/analyze/lints/s18_in_vehicle_check.rs +++ b/libs/sqf/src/analyze/lints/s18_in_vehicle_check.rs @@ -150,12 +150,12 @@ impl Code for CodeS18InVehicleCheck { impl CodeS18InVehicleCheck { #[must_use] - pub fn new(span: Range, processed: &Processed, severity: Severity, var: String, negated: bool) -> Self { + pub fn new(span: Range, processed: &Processed, severity: Severity, ident: String, negated: bool) -> Self { Self { span, severity, diagnostic: None, - ident: var, + ident, negated, } .generate_processed(processed) diff --git a/libs/sqf/src/analyze/lints/s22_this_call.rs b/libs/sqf/src/analyze/lints/s22_this_call.rs index 82316e54..43d2db3b 100644 --- a/libs/sqf/src/analyze/lints/s22_this_call.rs +++ b/libs/sqf/src/analyze/lints/s22_this_call.rs @@ -8,9 +8,9 @@ use hemtt_workspace::{ use crate::{analyze::SqfLintData, BinaryCommand, Expression}; -crate::lint!(LintS22InVehicleCheck); +crate::lint!(LintS22ThisCall); -impl Lint for LintS22InVehicleCheck { +impl Lint for LintS22ThisCall { fn ident(&self) -> &str { "this_call" } diff --git a/libs/sqf/src/analyze/lints/s23_reassign_reserved_variable.rs b/libs/sqf/src/analyze/lints/s23_reassign_reserved_variable.rs new file mode 100644 index 00000000..4de5c2ae --- /dev/null +++ b/libs/sqf/src/analyze/lints/s23_reassign_reserved_variable.rs @@ -0,0 +1,299 @@ +use std::{collections::HashMap, 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::SqfLintData, BinaryCommand, Expression, Statement, UnaryCommand}; + +crate::lint!(LintS23ReassignReservedVariable); + +impl Lint for LintS23ReassignReservedVariable { + fn ident(&self) -> &str { + "reasign_reserved_variable" + } + + fn sort(&self) -> u32 { + 230 + } + + fn description(&self) -> &str { + "Prevents reassigning reserved variables" + } + + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +call { + _this = 1; +}; +``` + +```sqf +{ + private _forEachIndex = random 5; +} forEach allUnits; + +### Explanation + +Reassigning reserved variables can lead to unintentional behavior. +" + } + + fn default_config(&self) -> LintConfig { + LintConfig::error() + } + + fn runners(&self) -> Vec>> { + vec![Box::new(StatementsRunner {}), Box::new(ExpressionRunner {})] + } +} + +static RESERVED: [&str; 8] = [ + "this","_this","_forEachIndex","_exception","_thisScript","_thisFSM","thisList","thisTrigger", +]; + +struct StatementsRunner {} +impl LintRunner for StatementsRunner { + type Target = crate::Statements; + + #[allow(clippy::significant_drop_tightening)] + fn run( + &self, + _project: Option<&hemtt_common::config::ProjectConfig>, + config: &LintConfig, + processed: Option<&hemtt_workspace::reporting::Processed>, + target: &Self::Target, + _data: &SqfLintData, + ) -> Codes { + let Some(processed) = processed else { + return Vec::new(); + }; + + let mut just_saved: Option<(String, String, Range)> = None; + let mut codes: Codes = Vec::new(); + let mut need_to_restore: HashMap, Range)> = HashMap::new(); + + for statement in target.content() { + let (Statement::AssignGlobal(var, exp, span) | Statement::AssignLocal(var, exp, span)) = statement else { + just_saved.take(); + continue + }; + + if let Some((saved, original, saved_span)) = just_saved.as_ref() { + if saved == var { + need_to_restore.insert(original.to_string(), (saved.to_string(), span.clone(), saved_span.clone())); + just_saved.take(); + continue + } + } + + if let Some((saved, original_save, saved_span)) = need_to_restore.get(var) { + codes.push(Arc::new(CodeS23ReassignReservedVariable::new(Variant::SavedWhileSaved(var.to_string(), span.clone(), original_save.clone(), saved.clone(), saved_span.clone()), processed, config.severity()))); + } + + if let Expression::Variable(restoring, _) = exp { + if need_to_restore.remove(restoring).is_some() { + continue + } + } + + if RESERVED.contains(&var.as_str()) { + codes.push(Arc::new(CodeS23ReassignReservedVariable::new(Variant::Overwrite(var.to_string(), span.clone()), processed, config.severity()))); + } else if let Expression::Variable(saved, new_saved_span) = exp { + if RESERVED.contains(&saved.as_str()) { + just_saved.replace((saved.to_string(), var.to_string(), new_saved_span.clone())); + continue + } + } + } + + for (saved, (original, span, saved_span)) in need_to_restore { + codes.push(Arc::new(CodeS23ReassignReservedVariable::new(Variant::NeverRestored((saved, span.clone()), (original, saved_span.clone())), processed, config.severity()))); + } + + codes + } +} + +struct ExpressionRunner {} +impl LintRunner for ExpressionRunner { + type Target = crate::Expression; + + #[allow(clippy::significant_drop_tightening)] + fn run( + &self, + _project: Option<&hemtt_common::config::ProjectConfig>, + config: &LintConfig, + processed: Option<&hemtt_workspace::reporting::Processed>, + target: &Self::Target, + _data: &SqfLintData, + ) -> Codes { + let Some(processed) = processed else { + return Vec::new(); + }; + + let exp = match target { + Expression::UnaryCommand(UnaryCommand::Named(cmd), exp, _) | + Expression::BinaryCommand(BinaryCommand::Named(cmd), _, exp, _) if cmd.to_lowercase() == "params" => exp, + _ => return Vec::new(), + }; + + if let Expression::Array(values, _) | Expression::ConsumeableArray(values, _) = &**exp { + for param in values { + let (name, span) = match ¶m { + Expression::String(name, span, _) => (name, span), + Expression::Array(values, _) => { + if let Some(Expression::String(name, span, _)) = values.first() { + (name, span) + } else { + continue + } + } + _ => continue, + }; + if RESERVED.contains(&&**name) { + return vec![Arc::new(CodeS23ReassignReservedVariable::new(Variant::Overwrite(name.to_string(), span.clone()), processed, config.severity()))]; + } + } + } + + Vec::new() + } +} + +pub enum Variant { + Overwrite(String, Range), + NeverRestored((String, Range),(String, Range)), + SavedWhileSaved(String, Range, Range, String, Range), +} + +impl Variant { + #[must_use] + pub fn span(&self) -> Range { + match self { + Self::Overwrite(_, span) | + Self::NeverRestored((_, span), _) | + Self::SavedWhileSaved(_, span, _, _, _) => span.clone(), + } + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS23ReassignReservedVariable { + variant: Variant, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS23ReassignReservedVariable { + fn ident(&self) -> &'static str { + "L-S23" + } + + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#reasign_reserved_variable") + } + + fn severity(&self) -> Severity { + self.severity + } + + fn message(&self) -> String { + match &self.variant { + Variant::Overwrite(var, _) => format!("Reassigning reserved variable `{var}`"), + Variant::NeverRestored((saved, _), (original, _)) => format!("Reserved variable `{original}` was never restored after being saved to `{saved}`"), + Variant::SavedWhileSaved(saved, _, _, original, _) => format!("Holder variable `{saved}` is overwritten before restoring `{original}`"), + } + } + + fn label_message(&self) -> String { + String::new() + } + + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS23ReassignReservedVariable { + #[must_use] + pub fn new(variant: Variant, processed: &Processed, severity: Severity) -> Self { + Self { + variant, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + + fn generate_processed(mut self, processed: &Processed) -> Self { + let Some(mut diag) = Diagnostic::new_for_processed(&self, self.variant.span(), processed) else { + return self + }; + diag = diag.clear_labels(); + match &self.variant { + Variant::Overwrite(var, span) => { + let Some(info) = get_span_info(span.clone(), processed) else { + return self; + }; + diag = diag.with_label(Label::primary( + info.0, + info.1, + ).with_message(format!("`{var}` is reserved"))); + } + Variant::NeverRestored((saved, saved_span), (original, original_span)) => { + let Some(saved_info) = get_span_info(saved_span.clone(), processed) else { + return self; + }; + let Some(original_info) = get_span_info(original_span.clone(), processed) else { + return self; + }; + diag = diag.with_label(Label::secondary( + saved_info.0, + saved_info.1, + ).with_message(format!("`{original}` is modified here"))).with_label(Label::primary( + original_info.0, + original_info.1, + ).with_message(format!("`{saved}` is never restored to `{original}`"))); + } + Variant::SavedWhileSaved(saved, saved_span, changed_span, original, original_span) => { + let Some(saved_info) = get_span_info(saved_span.clone(), processed) else { + return self; + }; + let Some(changed_span) = get_span_info(changed_span.clone(), processed) else { + return self; + }; + let Some(original_info) = get_span_info(original_span.clone(), processed) else { + return self; + }; + diag = diag.with_label(Label::primary( + saved_info.0, + saved_info.1, + ).with_message(format!("`{saved}` is overwritten before restoring `{original}`"))).with_label(Label::secondary( + changed_span.0, + changed_span.1, + ).with_message(format!("`{original}` is changed"))).with_label(Label::secondary( + original_info.0, + original_info.1, + ).with_message(format!("`{saved}` saves the state of {original}"))); + } + } + 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_start(), + )) +} diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 37a8826d..cb0b0a79 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -87,3 +87,4 @@ analyze!(s18_in_vehicle_check); analyze!(s20_bool_static_comparison); analyze!(s21_invalid_comparisons); analyze!(s22_this_call); +analyze!(s23_reassign_reserved_variable); diff --git a/libs/sqf/tests/lints/s23_reassign_reserved_variable/source.sqf b/libs/sqf/tests/lints/s23_reassign_reserved_variable/source.sqf new file mode 100644 index 00000000..4ac55b85 --- /dev/null +++ b/libs/sqf/tests/lints/s23_reassign_reserved_variable/source.sqf @@ -0,0 +1,33 @@ +// error for overwriting +call { + _this = 123; +}; + +// error for overwriting +call { + params ["_this"]; +}; + +// error for not restoring +call { + private _savedThis = _this; + _this = 123; + call _my_fnc; +}; + +// error for saving while still saved +call { + private _savedThis = _this; + _this = 123; + private _savedThis = 1234; + call _my_fnc; + _this = _savedThis; +}; + +// no error +call { + private _savedThis = _this; + _this = 123; + call _my_fnc; + _this = _savedThis; +}; diff --git a/libs/sqf/tests/lints/s23_reassign_reserved_variable/stdout.ansi b/libs/sqf/tests/lints/s23_reassign_reserved_variable/stdout.ansi new file mode 100644 index 00000000..d176340d --- /dev/null +++ b/libs/sqf/tests/lints/s23_reassign_reserved_variable/stdout.ansi @@ -0,0 +1,33 @@ +error[L-S23]: Reassigning reserved variable `_this` + ┌─ source.sqf:3:5 + │ +3 │ _this = 123; + │ ^^^^^^^^^^^ `_this` is reserved + + +error[L-S23]: Reassigning reserved variable `_this` + ┌─ source.sqf:8:13 + │ +8 │ params ["_this"]; + │ ^^^^^^^ `_this` is reserved + + +error[L-S23]: Reserved variable `_this` was never restored after being saved to `_savedThis` + ┌─ source.sqf:13:26 + │ +13 │ private _savedThis = _this; + │ ^^^^^ `_savedThis` is never restored to `_this` +14 │ _this = 123; + │ ----------- `_this` is modified here + + +error[L-S23]: Holder variable `_savedThis` is overwritten before restoring `_this` + ┌─ source.sqf:22:5 + │ +20 │ private _savedThis = _this; + │ ----- `_savedThis` saves the state of _this +21 │ _this = 123; + │ ----------- `_this` is changed +22 │ private _savedThis = 1234; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ `_savedThis` is overwritten before restoring `_this` +