sqf: Add lint for vars that are ALL_CAPS and not a macro (#793)

libs/preprocessor/src/processor/directives.rs

@@ -289,6 +289,10 @@ impl Processor {
         };
         #[cfg(feature = "lsp")]
         self.usage.insert(ident.position().clone(), Vec::new());
+        self.macros
+            .entry(ident_string.clone())
+            .or_default()
+            .push(ident.position().clone());
         self.defines.insert(
             &ident_string,
             (

libs/preprocessor/src/processor/mod.rs

@@ -1,4 +1,3 @@
-#[cfg(feature = "lsp")]
 use std::collections::HashMap;
 use std::rc::Rc;
 use std::sync::Arc;
@@ -36,6 +35,8 @@ pub struct Processor {
 
     pub(crate) token_count: usize,
 
+    macros: HashMap<String, Vec<Position>>,
+
     #[cfg(feature = "lsp")]
     /// Map of token usage to definition
     /// (token, definition)
@@ -94,10 +95,9 @@ impl Processor {
 
         Processed::new(
             buffer,
+            processor.macros,
             #[cfg(feature = "lsp")]
             processor.usage,
-            #[cfg(feature = "lsp")]
-            processor.declarations,
             processor.warnings,
             processor.no_rapify,
         )

libs/sqf/src/analyze/lints/s17_var_all_caps.rs

@@ -0,0 +1,189 @@
+use std::{ops::Range, sync::Arc};
+
+use hemtt_common::{config::LintConfig, similar_values};
+use hemtt_workspace::{
+    lint::{AnyLintRunner, Lint, LintRunner},
+    reporting::{Code, Codes, Diagnostic, Label, Processed, Severity, Symbol},
+};
+
+use crate::{analyze::SqfLintData, Expression};
+
+crate::lint!(LintS17VarAllCaps);
+
+impl Lint<SqfLintData> for LintS17VarAllCaps {
+    fn ident(&self) -> &str {
+        "var_all_caps"
+    }
+
+    fn sort(&self) -> u32 {
+        170
+    }
+
+    fn description(&self) -> &str {
+        "Checks for global variables that are ALL_CAPS and may actually be a undefined macro"
+    }
+
+    fn documentation(&self) -> &str {
+        r#"### Configuration
+
+- **ignore**: An array of vars to ignore
+
+```toml
+[lints.sqf.var_all_caps]
+options.ignore = [
+    "XMOD_TEST", "MYMOD_*",
+]
+```
+
+### Example
+
+**Incorrect**
+```sqf
+private _z = _y + DO_NOT_EXIST;
+```
+
+### Explanation
+
+Variables that are all caps are usually reserved for macros. This should should help prevent any accidental typos or uses before definitions when using macros.
+."#
+    }
+    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::Expression;
+
+    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 Expression::Variable(var, span) = target else {
+            return Vec::new();
+        };
+        if var.starts_with('_') || &var.to_ascii_uppercase() != var || var == "SLX_XEH_COMPILE_NEW"
+        {
+            return Vec::new();
+        }
+        if let Some(toml::Value::Array(ignore)) = config.option("ignore") {
+            if ignore.iter().any(|i| {
+                let s = i.as_str().unwrap_or_default();
+                s == var || (s.ends_with('*') && var.starts_with(&s[0..s.len() - 1]))
+            }) {
+                return Vec::new();
+            }
+        }
+        vec![Arc::new(CodeS17VarAllCaps::new(
+            span.clone(),
+            var.clone(),
+            processed,
+            config.severity(),
+        ))]
+    }
+}
+
+#[allow(clippy::module_name_repetitions)]
+pub struct CodeS17VarAllCaps {
+    span: Range<usize>,
+    ident: String,
+    similar: Vec<String>,
+    severity: Severity,
+    diagnostic: Option<Diagnostic>,
+}
+
+impl Code for CodeS17VarAllCaps {
+    fn ident(&self) -> &'static str {
+        "L-S17"
+    }
+
+    fn link(&self) -> Option<&str> {
+        Some("/analysis/sqf.html#var_all_caps")
+    }
+
+    fn severity(&self) -> Severity {
+        self.severity
+    }
+
+    fn message(&self) -> String {
+        format!("Variable should not be all caps: {}", self.ident)
+    }
+
+    fn note(&self) -> Option<String> {
+        Some("All caps variables are usually reserved for macros".to_string())
+    }
+
+    fn label_message(&self) -> String {
+        "All caps variable".to_string()
+    }
+
+    fn help(&self) -> Option<String> {
+        if self.similar.is_empty() {
+            None
+        } else {
+            Some(format!("did you mean `{}`?", self.similar.join("`, `")))
+        }
+    }
+
+    fn diagnostic(&self) -> Option<Diagnostic> {
+        self.diagnostic.clone()
+    }
+}
+
+impl CodeS17VarAllCaps {
+    #[must_use]
+    pub fn new(span: Range<usize>, ident: String, processed: &Processed, severity: Severity) -> Self {
+        Self {
+            similar: similar_values(
+                &ident,
+                &processed.macros().keys().map(std::string::String::as_str).collect::<Vec<_>>(),
+            )
+            .iter()
+            .map(std::string::ToString::to_string)
+            .collect(),
+            span,
+            ident,
+            severity,
+            diagnostic: None,
+        }
+        .generate_processed(processed)
+    }
+
+    fn generate_processed(mut self, processed: &Processed) -> Self {
+        let Some(mut diagnostic) = Diagnostic::new_for_processed(&self, self.span.clone(), processed) else {
+            return self;
+        };
+        self.diagnostic = Some(diagnostic.clone());
+        let mut mappings = processed.mappings(self.span.start);
+        mappings.pop();
+        let symbol = Symbol::Word(self.ident.clone());
+        let Some(mapping) = mappings
+            .iter()
+            .find(|m| {
+                m.token().symbol() == &symbol
+            }) else {
+            return self;
+            };
+        if let Some(l) = diagnostic.labels.get_mut(0) { *l = l.clone().with_message("Used in macro here"); }
+        diagnostic.labels.push(
+            Label::primary(
+                mapping.original().path().clone(),
+                mapping.original().span(),
+            )
+            .with_message("All caps variable"),
+        );
+        self.diagnostic = Some(diagnostic);
+        self
+    }
+}

libs/sqf/tests/lints.rs

@@ -82,4 +82,5 @@ analyze!(s07_select_parse_number);
 analyze!(s08_format_args);
 analyze!(s09_banned_command);
 analyze!(s11_if_not_else);
+analyze!(s17_var_all_caps);
 analyze!(s18_in_vehicle_check);

libs/sqf/tests/lints/project_tests.toml

@@ -8,3 +8,9 @@ options.ignore = [
 
 [lints.sqf]
 if_not_else = true
+
+[lints.sqf.var_all_caps]
+enabled = true
+options.ignore = [
+    "AM_IGNORED",
+]

libs/sqf/tests/lints/s17_var_all_caps/source.sqf

@@ -0,0 +1,9 @@
+#define EXIST 1
+#define TYPO 2
+#define NESTED systemChat UNDEFINED
+
+private _x = 1 + EXIST;
+private _y = _x + AM_IGNORED;
+private _z = _y + DO_NOT_EXIST;
+private _w = _z + TPYO;
+NESTED;

libs/sqf/tests/lints/s17_var_all_caps/stdout.ansi

@@ -0,0 +1,30 @@
+help[L-S17]: Variable should not be all caps: DO_NOT_EXIST
+  ┌─ source.sqf:7:19
+  │
+7 │ private _z = _y + DO_NOT_EXIST;
+  │                   ^^^^^^^^^^^^ All caps variable
+  │
+  = note: All caps variables are usually reserved for macros
+
+
+help[L-S17]: Variable should not be all caps: TPYO
+  ┌─ source.sqf:8:19
+  │
+8 │ private _w = _z + TPYO;
+  │                   ^^^^ All caps variable
+  │
+  = note: All caps variables are usually reserved for macros
+  = help: did you mean `TYPO`?
+
+
+help[L-S17]: Variable should not be all caps: UNDEFINED
+  ┌─ source.sqf:3:27
+  │
+3 │ #define NESTED systemChat UNDEFINED
+  │                           ^^^^^^^^^ All caps variable
+  ·
+9 │ NESTED;
+  │ ^^^^^^ Used in macro here
+  │
+  = note: All caps variables are usually reserved for macros
+

libs/workspace/src/reporting/processed.rs

@@ -22,11 +22,7 @@ pub struct Processed {
     /// string offset(start, stop), source, source position
     mappings: Vec<Mapping>,
 
-    #[allow(dead_code)]
-    #[cfg(feature = "lsp")]
-    /// Map of token usage to definition
-    /// (token, definition)
-    declarations: HashMap<Position, Position>,
+    macros: HashMap<String, Vec<Position>>,
 
     #[allow(dead_code)]
     #[cfg(feature = "lsp")]
@@ -179,14 +175,13 @@ impl Processed {
     /// [`Error::Workspace`] if a workspace path could not be read
     pub fn new(
         output: Vec<Output>,
+        macros: HashMap<String, Vec<Position>>,
         #[cfg(feature = "lsp")] usage: HashMap<Position, Vec<Position>>,
-        #[cfg(feature = "lsp")] declarations: HashMap<Position, Position>,
         warnings: Codes,
         no_rapify: bool,
     ) -> Result<Self, Error> {
         let mut processed = Self {
-            #[cfg(feature = "lsp")]
-            declarations,
+            macros,
             #[cfg(feature = "lsp")]
             usage,
             warnings,
@@ -258,6 +253,12 @@ impl Processed {
         self.mappings(offset).last().copied()
     }
 
+    #[must_use]
+    /// Get the macros defined
+    pub const fn macros(&self) -> &HashMap<String, Vec<Position>> {
+        &self.macros
+    }
+
     #[must_use]
     #[allow(clippy::cast_possible_truncation)]
     /// Get offset as number of raw bytes into the output string