sqf: prevent marker spam (#806)

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<SqfLintData> 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<Box<dyn AnyLintRunner<SqfLintData>>> {
+        vec![Box::new(Runner)]
+    }
+}
+
+struct Runner;
+impl LintRunner<SqfLintData> 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<String, Vec<(String, Range<usize>)>> = 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<String> {
+    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<usize>)>,
+    severity: Severity,
+    diagnostic: Option<Diagnostic>,
+}
+
+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<String> {
+        Some("Global marker commands update the entire state of the marker".to_string())
+    }
+
+    fn help(&self) -> Option<String> {
+        Some("Using `setMarker*Local` on all except the last call reduces network updates".to_string())
+    }
+
+    fn diagnostic(&self) -> Option<Diagnostic> {
+        self.diagnostic.clone()
+    }
+}
+
+impl CodeS24MarkerSpam {
+    #[must_use]
+    pub fn new(calls: Vec<(String, Range<usize>)>, 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<usize>, processed: &Processed) -> Option<(WorkspacePath, Range<usize>)> {
+    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(),
+    ))
+}

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);

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";
+};

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
+