From c6486885be57095a4e5e6a913b98fa7e403a255e Mon Sep 17 00:00:00 2001 From: BrettMayson Date: Sat, 19 Oct 2024 17:43:27 -0600 Subject: [PATCH] sqf: prevent marker spam (#806) --- libs/sqf/src/analyze/lints/s24_marker_spam.rs | 208 ++++++++++++++++++ libs/sqf/tests/lints.rs | 1 + .../tests/lints/s24_marker_spam/source.sqf | 41 ++++ .../tests/lints/s24_marker_spam/stdout.ansi | 53 +++++ 4 files changed, 303 insertions(+) create mode 100644 libs/sqf/src/analyze/lints/s24_marker_spam.rs create mode 100644 libs/sqf/tests/lints/s24_marker_spam/source.sqf create mode 100644 libs/sqf/tests/lints/s24_marker_spam/stdout.ansi diff --git a/libs/sqf/src/analyze/lints/s24_marker_spam.rs b/libs/sqf/src/analyze/lints/s24_marker_spam.rs new file mode 100644 index 00000000..d192bc71 --- /dev/null +++ b/libs/sqf/src/analyze/lints/s24_marker_spam.rs @@ -0,0 +1,208 @@ +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}; + +crate::lint!(LintS24MarkerSpam); + +impl Lint for LintS24MarkerSpam { + fn ident(&self) -> &str { + "marker_update_spam" + } + + fn sort(&self) -> u32 { + 240 + } + + fn description(&self) -> &str { + "Checks for repeated calls to global marker updates" + } + + fn documentation(&self) -> &str { + r#"### Example + +**Incorrect** +```sqf +"my_marker" setMarkerAlpha 0.5; +"my_marker" setMarkerDir 90; +"my_marker" setMarkerSize [100, 200]; +"my_marker" setMarkerShape "RECTANGLE"; +``` +**Correct** +```sqf +"my_marker" setMarkerAlphaLocal 0.5; +"my_marker" setMarkerDirLocal 90; +"my_marker" setMarkerSizeLocal [100, 200]; +"my_marker" setMarkerShape "RECTANGLE"; +``` + +### Explanation + +The `setMarker*` commands send the entire state of the marker to all clients. This can be very expensive if done repeatedly. +Using the `setMarker*Local` on all calls except the last one will reduce the amount of data sent over the network. +"# + } + + fn default_config(&self) -> LintConfig { + LintConfig::help() + } + + fn runners(&self) -> Vec>> { + vec![Box::new(Runner)] + } +} + +struct Runner; +impl LintRunner for Runner { + type Target = crate::Statements; + + 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 codes: Codes = Vec::new(); + + let mut pending: HashMap)>> = HashMap::new(); + + for statement in target.content() { + match statement { + Statement::Expression(Expression::BinaryCommand(BinaryCommand::Named(cmd), name, _, cmd_span), _) if is_marker_global(cmd) => { + let Some(name) = marker_name(name) else { + continue; + }; + pending.entry(name.clone()).or_default().push((cmd.to_string(), cmd_span.clone())); + } + Statement::Expression(_, _) => {} + Statement::AssignGlobal(name, _, _) | Statement::AssignLocal(name, _, _) => { + if let Some(existing) = pending.remove(name) { + if existing.len() > 1 { + codes.push(Arc::new(CodeS24MarkerSpam::new(existing, processed, config.severity()))); + } + } + } + } + } + + for (_, calls) in pending { + if calls.len() > 1 { + codes.push(Arc::new(CodeS24MarkerSpam::new(calls, processed, config.severity()))); + } + } + + codes + } +} + +fn is_marker_global(cmd: &str) -> bool { + cmd.starts_with("setMarker") && !cmd.ends_with("Local") +} + +fn marker_name(var: &Expression) -> Option { + match var { + Expression::Variable(name, _) => Some(name.clone()), + Expression::String(name, _, _) => Some(name.to_string()), + _ => None, + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS24MarkerSpam { + calls: Vec<(String, Range)>, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS24MarkerSpam { + fn ident(&self) -> &'static str { + "L-S24" + } + + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#marker_update_spam") + } + + fn severity(&self) -> Severity { + self.severity + } + + fn message(&self) -> String { + "Repeated calls to global marker updates".to_string() + } + + fn label_message(&self) -> String { + String::new() + } + + fn note(&self) -> Option { + Some("Global marker commands update the entire state of the marker".to_string()) + } + + fn help(&self) -> Option { + Some("Using `setMarker*Local` on all except the last call reduces network updates".to_string()) + } + + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS24MarkerSpam { + #[must_use] + pub fn new(calls: Vec<(String, Range)>, processed: &Processed, severity: Severity) -> Self { + Self { + calls, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + + fn generate_processed(mut self, processed: &Processed) -> Self { + let Some(mut diag) = Diagnostic::new_for_processed(&self, self.calls.first().expect("at least one call").1.clone(), processed) else { + return self; + }; + diag = diag.clear_labels(); + let last = self.calls.last().expect("at least one call"); + let Some(info) = get_span_info(&last.1, processed) else { + return self; + }; + diag = diag.with_label(Label::secondary( + info.0, + info.1, + ).with_message("Last marker update, should remain global")); + for (cmd, span) in self.calls.iter().rev().skip(1) { + let Some(info) = get_span_info(span, processed) else { + continue; + }; + diag = diag.with_label(Label::primary( + info.0, + info.1, + ).with_message(format!("Use {cmd}Local"))); + } + 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 cb0b0a79..900b826a 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -88,3 +88,4 @@ analyze!(s20_bool_static_comparison); analyze!(s21_invalid_comparisons); analyze!(s22_this_call); analyze!(s23_reassign_reserved_variable); +analyze!(s24_marker_spam); diff --git a/libs/sqf/tests/lints/s24_marker_spam/source.sqf b/libs/sqf/tests/lints/s24_marker_spam/source.sqf new file mode 100644 index 00000000..03ac87c0 --- /dev/null +++ b/libs/sqf/tests/lints/s24_marker_spam/source.sqf @@ -0,0 +1,41 @@ +// Message on all 4 +call { + private _marker = "m1"; + _marker setMarkerShape "ICON"; + _marker setMarkerType "hd_dot"; + _marker setMarkerColor "ColorRed"; + _marker setMarkerSize [1, 1]; +}; + +// Message on all 3 +call { + "m1" setMarkerShape "ICON"; + "m1" setMarkerType "hd_dot"; + "m1" setMarkerSize [1, 1]; +}; + +// 2 sets of 2 +call { + private _marker = "m1"; + _marker setMarkerShape "ICON"; + _marker setMarkerType "hd_dot"; + private _marker = "m2"; + _marker setMarkerColor "ColorRed"; + _marker setMarkerSize [1, 1]; +}; + +// no message +call { + private _marker = "m1"; + _marker setMarkerShapeLocal "ICON"; + _marker setMarkerTypeLocal "hd_dot"; + _marker setMarkerColorLocal "ColorRed"; + _marker setMarkerSize [1, 1]; +}; + + +// no message +call { + "m1" setMarkerShape "ICON"; + "m2" setMarkerType "hd_dot"; +}; diff --git a/libs/sqf/tests/lints/s24_marker_spam/stdout.ansi b/libs/sqf/tests/lints/s24_marker_spam/stdout.ansi new file mode 100644 index 00000000..81ca6115 --- /dev/null +++ b/libs/sqf/tests/lints/s24_marker_spam/stdout.ansi @@ -0,0 +1,53 @@ +help[L-S24]: Repeated calls to global marker updates + ┌─ source.sqf:4:13 + │ +4 │ _marker setMarkerShape "ICON"; + │ ^^^^^^^^^^^^^^ Use setMarkerShapeLocal +5 │ _marker setMarkerType "hd_dot"; + │ ^^^^^^^^^^^^^ Use setMarkerTypeLocal +6 │ _marker setMarkerColor "ColorRed"; + │ ^^^^^^^^^^^^^^ Use setMarkerColorLocal +7 │ _marker setMarkerSize [1, 1]; + │ ------------- Last marker update, should remain global + │ + = note: Global marker commands update the entire state of the marker + = help: Using `setMarker*Local` on all except the last call reduces network updates + + +help[L-S24]: Repeated calls to global marker updates + ┌─ source.sqf:12:10 + │ +12 │ "m1" setMarkerShape "ICON"; + │ ^^^^^^^^^^^^^^ Use setMarkerShapeLocal +13 │ "m1" setMarkerType "hd_dot"; + │ ^^^^^^^^^^^^^ Use setMarkerTypeLocal +14 │ "m1" setMarkerSize [1, 1]; + │ ------------- Last marker update, should remain global + │ + = note: Global marker commands update the entire state of the marker + = help: Using `setMarker*Local` on all except the last call reduces network updates + + +help[L-S24]: Repeated calls to global marker updates + ┌─ source.sqf:20:13 + │ +20 │ _marker setMarkerShape "ICON"; + │ ^^^^^^^^^^^^^^ Use setMarkerShapeLocal +21 │ _marker setMarkerType "hd_dot"; + │ ------------- Last marker update, should remain global + │ + = note: Global marker commands update the entire state of the marker + = help: Using `setMarker*Local` on all except the last call reduces network updates + + +help[L-S24]: Repeated calls to global marker updates + ┌─ source.sqf:23:13 + │ +23 │ _marker setMarkerColor "ColorRed"; + │ ^^^^^^^^^^^^^^ Use setMarkerColorLocal +24 │ _marker setMarkerSize [1, 1]; + │ ------------- Last marker update, should remain global + │ + = note: Global marker commands update the entire state of the marker + = help: Using `setMarker*Local` on all except the last call reduces network updates +