From 7f1ec1df46d88edadd233d01c9c0dfa7d10425a7 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Wed, 25 Sep 2024 20:03:38 -0500 Subject: [PATCH 01/16] sqf: Inspector Initial --- Cargo.lock | 1 + libs/sqf/Cargo.toml | 1 + libs/sqf/src/analyze/inspector/game_value.rs | 237 ++++++ libs/sqf/src/analyze/inspector/mod.rs | 739 ++++++++++++++++++ libs/sqf/src/analyze/lints/s12_inspector.rs | 252 ++++++ libs/sqf/src/analyze/mod.rs | 2 + libs/sqf/src/lib.rs | 1 + libs/sqf/src/parser/mod.rs | 2 + libs/sqf/tests/inspector.rs | 105 +++ libs/sqf/tests/inspector/test_0.sqf | 0 libs/sqf/tests/inspector/test_1.sqf | 48 ++ libs/sqf/tests/lints.rs | 1 + libs/sqf/tests/lints/project_tests.toml | 3 + .../tests/lints/s06_find_in_str/source.sqf | 2 +- .../tests/lints/s06_find_in_str/stdout.ansi | 6 +- libs/sqf/tests/lints/s12_inspector/source.sqf | 4 + .../sqf/tests/lints/s12_inspector/stdout.ansi | 13 + 17 files changed, 1413 insertions(+), 4 deletions(-) create mode 100644 libs/sqf/src/analyze/inspector/game_value.rs create mode 100644 libs/sqf/src/analyze/inspector/mod.rs create mode 100644 libs/sqf/src/analyze/lints/s12_inspector.rs create mode 100644 libs/sqf/tests/inspector.rs create mode 100644 libs/sqf/tests/inspector/test_0.sqf create mode 100644 libs/sqf/tests/inspector/test_1.sqf create mode 100644 libs/sqf/tests/lints/s12_inspector/source.sqf create mode 100644 libs/sqf/tests/lints/s12_inspector/stdout.ansi diff --git a/Cargo.lock b/Cargo.lock index c87fe04b..c21014d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1553,6 +1553,7 @@ dependencies = [ "hemtt-workspace", "linkme", "paste", + "regex", "toml 0.8.19", "tracing", ] diff --git a/libs/sqf/Cargo.toml b/libs/sqf/Cargo.toml index ae9872b7..5ff95a98 100644 --- a/libs/sqf/Cargo.toml +++ b/libs/sqf/Cargo.toml @@ -24,6 +24,7 @@ float-ord = "0.3.2" linkme = { workspace = true } toml = { workspace = true } tracing = { workspace = true } +regex = { workspace = true } [features] default = ["compiler", "parser"] diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs new file mode 100644 index 00000000..d5a10407 --- /dev/null +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -0,0 +1,237 @@ +use std::{collections::HashSet, sync::Arc}; + +use arma3_wiki::model::{Arg, Call, Param, Value}; +use tracing::{trace, warn}; + +use crate::{parser::database::Database, Expression}; + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub enum GameValue { + Anything, + // Assignment, // as in z = call {x=1}??? + Array(Option), + Boolean(Option), + Code(Option), + Config, + Control, + DiaryRecord, + Display, + ForType(Option), + Group, + HashMap, + IfType, + Location, + Namespace, + Number(Option), + Nothing, + Object, + ScriptHandle, + Side, + String(Option), + SwitchType, + Task, + TeamMember, + WhileType, + WithType, +} + +impl GameValue { + #[must_use] + pub fn from_cmd( + expression: &Expression, + lhs_set: Option<&HashSet>, + rhs_set: Option<&HashSet>, + database: &Arc, + ) -> HashSet { + let mut return_types = HashSet::new(); + let cmd_name = expression.command_name().expect("has a name"); + let Some(command) = database.wiki().commands().get(cmd_name) else { + trace!("cmd {cmd_name} not in db?"); //ToDo: this can't find short cmds like &&, || + return HashSet::from([Self::Anything]); + }; + + for syntax in command.syntax() { + match syntax.call() { + Call::Nular => { + if !matches!(expression, Expression::NularCommand(..)) { + continue; + } + } + Call::Unary(rhs_arg) => { + if !matches!(expression, Expression::UnaryCommand(..)) + || !Self::match_set_to_arg( + rhs_set.expect("u args"), + rhs_arg, + syntax.params(), + ) + { + continue; + } + } + Call::Binary(lhs_arg, rhs_arg) => { + if !matches!(expression, Expression::BinaryCommand(..)) + || !Self::match_set_to_arg( + lhs_set.expect("b args"), + lhs_arg, + syntax.params(), + ) + || !Self::match_set_to_arg( + rhs_set.expect("b args"), + rhs_arg, + syntax.params(), + ) + { + continue; + } + } + } + let value = &syntax.ret().0; + return_types.insert(Self::from_wiki_value(value)); + } + // trace!( + // "cmd [{}] = {}:{:?}", + // cmd_name, + // return_types.len(), + // return_types + // ); + return_types + } + + #[must_use] + pub fn match_set_to_arg(set: &HashSet, arg: &Arg, params: &[Param]) -> bool { + match arg { + Arg::Item(name) => { + // trace!("looking for {name} in {params:?}"); + let Some(param) = params.iter().find(|p| p.name() == name) else { + warn!("param not found"); + return true; + }; + Self::match_set_to_value(set, param.typ()) + } + Arg::Array(_vec_arg) => { + // todo: each individual array arg + Self::match_set_to_value(set, &Value::ArrayUnknown) + } + } + } + + #[must_use] + pub fn match_set_to_value(set: &HashSet, right_wiki: &Value) -> bool { + let right = Self::from_wiki_value(right_wiki); + set.iter().any(|gv| Self::match_values(gv, &right)) + } + + #[must_use] + /// matches values are compatible (Anything will always match) + /// todo: think about how nil and any interact? + pub fn match_values(left: &Self, right: &Self) -> bool { + if matches!(left, Self::Anything) { + return true; + } + if matches!(right, Self::Anything) { + return true; + } + std::mem::discriminant(left) == std::mem::discriminant(right) + } + + #[must_use] + /// Maps from Wiki:Value to Inspector:GameValue + pub fn from_wiki_value(value: &Value) -> Self { + match value { + Value::Anything => Self::Anything, + Value::ArrayColor + | Value::ArrayColorRgb + | Value::ArrayColorRgba + | Value::ArrayDate + | Value::ArraySized { .. } + | Value::ArrayUnknown + | Value::ArrayUnsized { .. } + | Value::Position + | Value::Waypoint => Self::Array(None), + Value::Boolean => Self::Boolean(None), + Value::Code => Self::Code(None), + Value::Config => Self::Config, + Value::Control => Self::Control, + Value::DiaryRecord => Self::DiaryRecord, + Value::Display => Self::Display, + Value::ForType => Self::ForType(None), + Value::IfType => Self::IfType, + Value::Group => Self::Group, + Value::Location => Self::Location, + Value::Namespace => Self::Namespace, + Value::Nothing => Self::Nothing, + Value::Number => Self::Number(None), + Value::Object => Self::Object, + Value::ScriptHandle => Self::ScriptHandle, + Value::Side => Self::Side, + Value::String => Self::String(None), + Value::SwitchType => Self::SwitchType, + Value::Task => Self::Task, + Value::TeamMember => Self::TeamMember, + Value::WhileType => Self::WhileType, + Value::WithType => Self::WithType, + Value::Unknown => { + trace!("wiki has syntax with [unknown] type"); + Self::Anything + } + _ => { + warn!("wiki type [{value:?}] not matched"); + Self::Anything + } + } + } + + #[must_use] + /// Get as a string for debugging + pub fn as_debug(&self) -> String { + match self { + // Self::Assignment() => { + // format!("Assignment") + // } + Self::Anything => "Anything".to_string(), + Self::ForType(expression) => { + if let Some(Expression::String(str, _, _)) = expression { + format!("ForType(var {str})") + } else { + "ForType(GENERIC)".to_string() + } + } + Self::Number(expression) => { + if let Some(Expression::Number(num, _)) = expression { + format!("Number({num:?})",) + } else { + "Number(GENERIC)".to_string() + } + } + Self::String(expression) => { + if let Some(Expression::String(str, _, _)) = expression { + format!("String({str})") + } else { + "String(GENERIC)".to_string() + } + } + Self::Boolean(expression) => { + if let Some(Expression::Boolean(bool, _)) = expression { + format!("Boolean({bool})") + } else { + "Boolean(GENERIC)".to_string() + } + } + Self::Array(expression) => { + if let Some(Expression::Array(array, _)) = expression { + format!("ArrayExp(len {})", array.len()) + } else { + "ArrayExp(GENERIC)".to_string() + } + } + Self::Code(expression) => { + if let Some(Expression::Code(statements)) = expression { + format!("Code(len {})", statements.content().len()) + } else { + "Code(GENERIC)".to_string() + } + } + _ => "Other(todo)".to_string(), + } + } +} diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs new file mode 100644 index 00000000..0ea2c936 --- /dev/null +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -0,0 +1,739 @@ +//! Inspects code, checking code args and variable usage +//! +use std::{ + collections::{HashMap, HashSet}, + hash::Hash, + ops::Range, + sync::Arc, + vec, +}; + +use crate::{ + parser::database::Database, BinaryCommand, Expression, Statement, Statements, UnaryCommand, +}; +use game_value::GameValue; +use hemtt_workspace::reporting::Processed; +use regex::Regex; +use tracing::{error, trace}; + +mod game_value; + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub enum Issue { + InvalidArgs(String, Range), + Undefined(String, Range, bool), + Unused(String, Range), + Shadowed(String, Range), + NotPrivate(String, Range), +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub enum VarSource { + Real(Range), + Magic(Range), + Ignore, +} +impl VarSource { + #[must_use] + pub const fn check_unused(&self) -> bool { + matches!(self, Self::Real(..)) + } + #[must_use] + pub const fn check_shadow(&self) -> bool { + matches!(self, Self::Real(..)) + } + #[must_use] + pub fn get_range(&self) -> Option> { + match self { + Self::Real(range) | Self::Magic(range) => Some(range.clone()), + Self::Ignore => None, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VarHolder { + possible: HashSet, + usage: i32, + source: VarSource, +} + +pub type Stack = HashMap; + +pub struct SciptScope { + database: Arc, + errors: HashSet, + global: Stack, + local: Vec, + code_seen: HashSet, + code_used: HashSet, + is_child: bool, + ignored_vars: HashSet, +} + +impl SciptScope { + #[must_use] + pub fn create( + ignored_vars: &HashSet, + database: &Arc, + is_child: bool, + ) -> Self { + // trace!("Creating ScriptScope"); + let mut scope = Self { + database: database.clone(), + errors: HashSet::new(), + global: Stack::new(), + local: Vec::new(), + code_seen: HashSet::new(), + code_used: HashSet::new(), + is_child, + ignored_vars: ignored_vars.clone(), + }; + scope.push(); + for var in ignored_vars { + scope.var_assign( + var, + true, + HashSet::from([GameValue::Anything]), + VarSource::Ignore, + ); + } + scope + } + #[must_use] + pub fn finish(&mut self, check_child_scripts: bool) -> HashSet { + self.pop(); + if check_child_scripts { + let unused = &self.code_seen - &self.code_used; + for expression in unused { + let Expression::Code(statements) = expression else { + error!("non-code in unused"); + continue; + }; + // trace!("-- Checking external scope"); + let mut external_scope = Self::create(&self.ignored_vars, &self.database, true); + external_scope.eval_statements(&statements); + self.errors + .extend(external_scope.finish(check_child_scripts)); + } + } + self.errors.clone() + } + + pub fn push(&mut self) { + // trace!("-- Stack Push {}", self.local.len()); + self.local.push(Stack::new()); + } + pub fn pop(&mut self) { + for (var, holder) in self.local.pop().unwrap_or_default() { + // trace!("-- Stack Pop {}:{} ", var, holder.usage); + if holder.usage == 0 && holder.source.check_unused() { + self.errors.insert(Issue::Unused( + var, + holder.source.get_range().unwrap_or_default(), + )); + } + } + } + + pub fn var_assign( + &mut self, + var: &str, + local: bool, + possible_values: HashSet, + source: VarSource, + ) { + trace!("var_assign: {} @ {}", var, self.local.len()); + let var_lower = var.to_ascii_lowercase(); + if !var_lower.starts_with('_') { + let holder = self.global.entry(var_lower).or_insert(VarHolder { + possible: HashSet::new(), + usage: 0, + source, + }); + holder.possible.extend(possible_values); + return; + } + + let stack_level_search = self + .local + .iter() + .rev() + .position(|s| s.contains_key(&var_lower)); + let mut stack_level = self.local.len() - 1; + if stack_level_search.is_none() { + if !local { + self.errors.insert(Issue::NotPrivate( + var.to_owned(), + source.get_range().unwrap_or_default(), + )); + } + } else if local { + if source.check_shadow() { + self.errors.insert(Issue::Shadowed( + var.to_owned(), + source.get_range().unwrap_or_default(), + )); + } + } else { + stack_level -= stack_level_search.unwrap_or_default(); + } + let holder = self.local[stack_level] + .entry(var_lower) + .or_insert(VarHolder { + possible: HashSet::new(), + usage: 0, + source, + }); + holder.possible.extend(possible_values); + } + + #[must_use] + /// # Panics + pub fn var_retrieve( + &mut self, + var: &str, + source: &Range, + peek: bool, + ) -> HashSet { + let var_lower = var.to_ascii_lowercase(); + let holder_option = if var_lower.starts_with('_') { + let stack_level_search = self + .local + .iter() + .rev() + .position(|s| s.contains_key(&var_lower)); + let mut stack_level = self.local.len() - 1; + if stack_level_search.is_none() { + if !peek { + self.errors.insert(Issue::Undefined( + var.to_owned(), + source.clone(), + self.is_child, + )); + } + } else { + stack_level -= stack_level_search.expect("is_some"); + }; + self.local[stack_level].get_mut(&var_lower) + } else if self.global.contains_key(&var_lower) { + self.global.get_mut(&var_lower) + } else { + return HashSet::from([GameValue::Anything]); + }; + if holder_option.is_none() { + // we've reported the error above, just return Any so it doesn't fail everything after + HashSet::from([GameValue::Anything]) + } else { + let holder = holder_option.expect("is_some"); + holder.usage += 1; + let mut set = holder.possible.clone(); + + if !var_lower.starts_with('_') && self.ignored_vars.contains(&var.to_ascii_lowercase()) + { + // Assume that a ignored global var could be anything + set.insert(GameValue::Anything); + } + set + } + } + #[must_use] + pub fn cmd_u_private(&mut self, rhs: &HashSet) -> HashSet { + fn push_var(s: &mut SciptScope, var: &String, source: &Range) { + if s.ignored_vars.contains(&var.to_ascii_lowercase()) { + s.var_assign( + &var.to_string(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Ignore, + ); + } else { + s.var_assign( + &var.to_string(), + true, + HashSet::from([GameValue::Nothing]), + VarSource::Real(source.clone()), + ); + } + } + for possible in rhs { + if let GameValue::Array(Some(Expression::Array(array, _))) = possible { + for element in array { + let Expression::String(var, source, _) = element else { + continue; + }; + if var.is_empty() { + continue; + } + push_var(self, &var.to_string(), source); + } + } + if let GameValue::String(Some(Expression::String(var, source, _))) = possible { + if var.is_empty() { + continue; + } + push_var(self, &var.to_string(), source); + } + } + HashSet::new() + } + #[must_use] + pub fn cmd_generic_params(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + continue; + }; + + for entry in array { + match entry { + Expression::String(var, source, _) => { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Real(source.clone()), + ); + } + Expression::Array(var_array, _) => { + if !var_array.is_empty() { + if let Expression::String(var, source, _) = &var_array[0] { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Real(source.clone()), + ); + } + } + } + _ => {} + } + } + } + HashSet::from([GameValue::Boolean(None)]) + } + #[must_use] + pub fn cmd_generic_call(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + self.code_used.insert(expression.clone()); + self.eval_statements(statements); + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_b_do( + &mut self, + lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + // look for forType vars with valid strings (ignore old style code) + let mut do_run = true; + for possible in lhs { + if let GameValue::ForType(option) = possible { + match option { + Some(Expression::String(var, source, _)) => { + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Number(None)]), + VarSource::Real(source.clone()), + ); + } + Some(Expression::Array(array, _)) => { + if array.len() != 3 { + error!("for wrong len"); + continue; + } + for for_stage in array { + let Expression::Code(for_statements) = for_stage else { + continue; + }; + self.code_used.insert(for_stage.clone()); + self.eval_statements(for_statements); + } + } + None => { + do_run = false; + } + _ => { + unreachable!(""); + } + } + } + } + self.code_used.insert(expression.clone()); + if do_run { + self.eval_statements(statements); + } + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_generic_call_magic( + &mut self, + code_possibilities: &HashSet, + magic: &Vec, + source: &Range, + ) -> HashSet { + for possible in code_possibilities { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + for var in magic { + self.var_assign( + var, + true, + HashSet::from([GameValue::Anything]), + VarSource::Magic(source.clone()), + ); + } + self.code_used.insert(expression.clone()); + self.eval_statements(statements); + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_for(&mut self, rhs: &HashSet) -> HashSet { + let mut return_value = HashSet::new(); + for possible in rhs { + match possible { + GameValue::Array(option) | GameValue::String(option) => { + return_value.insert(GameValue::ForType(option.clone())); + } + _ => { + error!("shouldn't be reachable?"); + return_value.insert(GameValue::ForType(None)); + } + } + } + return_value + } + #[must_use] + /// for (from, to, step) chained commands + pub fn cmd_b_from_chain( + &self, + lhs: &HashSet, + _rhs: &HashSet, + ) -> HashSet { + lhs.clone() + } + #[must_use] + pub fn cmd_u_is_nil(&mut self, rhs: &HashSet) -> HashSet { + let mut non_string = false; + for possible in rhs { + let GameValue::String(possible) = possible else { + non_string = true; + continue; + }; + let Some(expression) = possible else { + continue; + }; + let Expression::String(var, _, _) = expression else { + continue; + }; + let _ = self.var_retrieve(var, &expression.span(), true); + } + if non_string { + let _ = self.cmd_generic_call(rhs); + } + HashSet::from([GameValue::Boolean(None)]) + } + #[must_use] + pub fn cmd_b_then( + &mut self, + _lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + let mut return_value = HashSet::new(); + for possible in rhs { + if let GameValue::Code(Some(Expression::Code(_statements))) = possible { + return_value.extend(self.cmd_generic_call(rhs)); + } + if let GameValue::Array(Some(Expression::Array(array, _))) = possible { + for expression in array { + return_value.extend(self.cmd_generic_call(&HashSet::from([GameValue::Code( + Some(expression.clone()), + )]))); + } + } + } + return_value + } + #[must_use] + pub fn cmd_b_else( + &self, + lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + let mut return_value = HashSet::new(); // just merge, not really the same but should be fine + for possible in rhs { + return_value.insert(possible.clone()); + } + for possible in lhs { + return_value.insert(possible.clone()); + } + return_value + } + #[must_use] + pub fn cmd_b_get_or_default_call(&mut self, rhs: &HashSet) -> HashSet { + let mut possible_code = HashSet::new(); + for possible in rhs { + let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + continue; + }; + if array.len() < 2 { + continue; + } + possible_code.insert(GameValue::Code(Some(array[1].clone()))); + } + let _ = self.cmd_generic_call(&possible_code); + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_u_to_string(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(_) = expression else { + continue; + }; + // just skip because it will often use a _x + self.code_used.insert(expression.clone()); + } + HashSet::from([GameValue::String(None)]) + } + + #[must_use] + #[allow(clippy::too_many_lines)] + /// Evaluate expression in current scope + pub fn eval_expression(&mut self, expression: &Expression) -> HashSet { + let mut debug_type = String::new(); + let possible_values = match expression { + Expression::Variable(var, source) => self.var_retrieve(var, source, false), + Expression::Number(..) => HashSet::from([GameValue::Number(Some(expression.clone()))]), + Expression::Boolean(..) => { + HashSet::from([GameValue::Boolean(Some(expression.clone()))]) + } + Expression::String(..) => HashSet::from([GameValue::String(Some(expression.clone()))]), + Expression::Array(array, _) => { + for e in array { + let _ = self.eval_expression(e); + } + HashSet::from([GameValue::Array(Some(expression.clone()))]) + } + Expression::NularCommand(cmd, source) => { + debug_type = format!("[N:{}]", cmd.as_str()); + let cmd_set = GameValue::from_cmd(expression, None, None, &self.database); + if cmd_set.is_empty() { + // is this possible? + self.errors + .insert(Issue::InvalidArgs(debug_type.clone(), source.clone())); + } + cmd_set + } + Expression::UnaryCommand(cmd, rhs, source) => { + debug_type = format!("[U:{}]", cmd.as_str()); + let rhs_set = self.eval_expression(rhs); + let cmd_set = GameValue::from_cmd(expression, None, Some(&rhs_set), &self.database); + if cmd_set.is_empty() { + self.errors + .insert(Issue::InvalidArgs(debug_type.clone(), source.clone())); + } + let return_set = match cmd { + UnaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { + "params" => Some(self.cmd_generic_params(&rhs_set)), + "private" => Some(self.cmd_u_private(&rhs_set)), + "call" => Some(self.cmd_generic_call(&rhs_set)), + "isnil" => Some(self.cmd_u_is_nil(&rhs_set)), + "while" | "waituntil" | "default" => { + let _ = self.cmd_generic_call(&rhs_set); + None + } + "for" => Some(self.cmd_for(&rhs_set)), + "tostring" => Some(self.cmd_u_to_string(&rhs_set)), + _ => None, + }, + _ => None, + }; + // Use custom return from cmd or just use wiki set + return_set.unwrap_or(cmd_set) + } + Expression::BinaryCommand(cmd, lhs, rhs, source) => { + debug_type = format!("[B:{}]", cmd.as_str()); + let lhs_set = self.eval_expression(lhs); + let rhs_set = self.eval_expression(rhs); + let cmd_set = + GameValue::from_cmd(expression, Some(&lhs_set), Some(&rhs_set), &self.database); + if cmd_set.is_empty() { + // we must have invalid args + self.errors + .insert(Issue::InvalidArgs(debug_type.clone(), source.clone())); + } + let return_set = match cmd { + BinaryCommand::Associate => { + // the : from case + let _ = self.cmd_generic_call(&rhs_set); + None + } + BinaryCommand::And | BinaryCommand::Or => { + let _ = self.cmd_generic_call(&rhs_set); + None + } + BinaryCommand::Else => Some(self.cmd_b_else(&lhs_set, &rhs_set)), + BinaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { + "params" => Some(self.cmd_generic_params(&rhs_set)), + "call" => Some(self.cmd_generic_call(&rhs_set)), + "exitwith" => { + // todo: handle scope exits + Some(self.cmd_generic_call(&rhs_set)) + } + "do" => { + // from While, With, For, and Switch + Some(self.cmd_b_do(&lhs_set, &rhs_set)) + } + "from" | "to" | "step" => Some(self.cmd_b_from_chain(&lhs_set, &rhs_set)), + "then" => Some(self.cmd_b_then(&lhs_set, &rhs_set)), + "foreach" | "foreachreversed" => Some(self.cmd_generic_call_magic( + &lhs_set, + &vec![ + "_x".to_string(), + "_y".to_string(), + "_forEachIndex".to_string(), + ], + source, + )), + "count" => { + let _ = self.cmd_generic_call_magic( + &lhs_set, + &vec!["_x".to_string()], + source, + ); + None + } + "findif" | "apply" | "select" => { + //todo handle (array select number) or (string select [1,2]); + let _ = self.cmd_generic_call_magic( + &rhs_set, + &vec!["_x".to_string()], + source, + ); + None + } + "getordefaultcall" => Some(self.cmd_b_get_or_default_call(&rhs_set)), + _ => None, + }, + _ => None, + }; + // Use custom return from cmd or just use wiki set + return_set.unwrap_or(cmd_set) + } + Expression::Code(statements) => { + self.code_seen.insert(expression.clone()); + debug_type = format!("CODE:{}", statements.content().len()); + HashSet::from([GameValue::Code(Some(expression.clone()))]) + } + Expression::ConsumeableArray(_, _) => unreachable!(""), + }; + trace!( + "eval expression{}->{:?}", + debug_type, + possible_values + .iter() + .map(GameValue::as_debug) + .collect::>() + ); + possible_values + } + + /// Evaluate statements in the current scope + fn eval_statements(&mut self, statements: &Statements) { + // let mut return_value = HashSet::new(); + for statement in statements.content() { + match statement { + Statement::AssignGlobal(var, expression, source) => { + // x or _x + let possible_values = self.eval_expression(expression); + self.var_assign(var, false, possible_values, VarSource::Real(source.clone())); + // return_value = vec![GameValue::Assignment()]; + } + Statement::AssignLocal(var, expression, source) => { + // private _x + let possible_values = self.eval_expression(expression); + self.var_assign(var, true, possible_values, VarSource::Real(source.clone())); + // return_value = vec![GameValue::Assignment()]; + } + Statement::Expression(expression, _) => { + let _possible_values = self.eval_expression(expression); + // return_value = possible_values; + } + } + } + // return_value + } +} + +#[must_use] +/// Run statements and return issues +pub fn run_processed( + statements: &Statements, + processed: &Processed, + database: &Arc, + check_child_scripts: bool, +) -> Vec { + let mut ignored_vars = Vec::new(); + ignored_vars.push("_this".to_string()); + let Ok(re1) = Regex::new(r"\/\/ ?IGNORE_PRIVATE_WARNING ?\[(.*)\]") else { + return Vec::new(); + }; + let Ok(re2) = Regex::new(r#""(.*?)""#) else { + return Vec::new(); + }; + for (_path, raw_source) in processed.sources() { + for (_, [ignores]) in re1.captures_iter(&raw_source).map(|c| c.extract()) { + for (_, [var]) in re2.captures_iter(ignores).map(|c| c.extract()) { + ignored_vars.push(var.to_ascii_lowercase()); + } + } + } + let mut scope = SciptScope::create(&HashSet::from_iter(ignored_vars), database, false); + scope.eval_statements(statements); + scope.finish(check_child_scripts).into_iter().collect() +} diff --git a/libs/sqf/src/analyze/lints/s12_inspector.rs b/libs/sqf/src/analyze/lints/s12_inspector.rs new file mode 100644 index 00000000..376ca1ae --- /dev/null +++ b/libs/sqf/src/analyze/lints/s12_inspector.rs @@ -0,0 +1,252 @@ +use crate::{ + analyze::{ + inspector::{self, Issue}, + SqfLintData, + }, + 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}; +use tracing::trace; + +crate::lint!(LintS12Inspector); + +impl Lint for LintS12Inspector { + fn ident(&self) -> &str { + "inspector" + } + + fn sort(&self) -> u32 { + 120 + } + + fn description(&self) -> &str { + "Checks for code usage" + } + + fn documentation(&self) -> &str { +r"### Configuration +- **check_invalid_args**: [default: true] check_invalid_args (e.g. `x setFuel true`) +- **check_child_scripts**: [default: false] Checks oprhaned scripts. + Assumes un-called code will run in another scope (can cause false positives) + e.g. `private _var = 5; [{_var}] call CBA_fnc_addPerFrameEventHandler;` +- **check_undefined**: [default: true] Checks local vars that are not defined +- **check_not_private**: [default: true] Checks local vars that are not `private` +- **check_unused**: [default: false] Checks local vars that are never used +- **check_shadow**: [default: false] Checks local vars that are shaddowed + +```toml +[lints.sqf.inspector] +options.check_invalid_args = true +options.check_child_scripts = true +options.check_undefined = true +options.check_not_private = true +options.check_unused = true +options.check_shadow = true +```" + } + + fn default_config(&self) -> LintConfig { + LintConfig::help() + } + + fn runners(&self) -> Vec>> { + vec![Box::new(Runner)] + } +} + +pub struct Runner; +impl LintRunner for Runner { + type Target = Statements; + #[allow(clippy::too_many_lines)] + fn run( + &self, + _project: Option<&hemtt_common::config::ProjectConfig>, + config: &hemtt_common::config::LintConfig, + processed: Option<&hemtt_workspace::reporting::Processed>, + target: &Statements, + data: &SqfLintData, + ) -> hemtt_workspace::reporting::Codes { + let Some(processed) = processed else { + return Vec::new(); + }; + if !target.top_level { + // we only want to handle full files, not all the sub-statements + return Vec::new(); + }; + let (_addon, database) = data; + + let check_invalid_args = + if let Some(toml::Value::Boolean(b)) = config.option("check_invalid_args") { + *b + } else { + true + }; + let check_child_scripts = + if let Some(toml::Value::Boolean(b)) = config.option("check_child_scripts") { + *b + } else { + false // can cause false positives + }; + let check_undefined = + if let Some(toml::Value::Boolean(b)) = config.option("check_undefined") { + *b + } else { + true + }; + let check_not_private = if let Some(toml::Value::Boolean(b)) = + config.option("check_not_private") + { + *b + } else { + true + }; + let check_unused = + if let Some(toml::Value::Boolean(b)) = config.option("check_unused") { + *b + } else { + false + }; + let check_shadow = + if let Some(toml::Value::Boolean(b)) = config.option("check_shadow") { + *b + } else { + false + }; + + let mut errors: Codes = Vec::new(); + let issues = inspector::run_processed(target, processed, database, check_child_scripts); + trace!("issues {}", issues.len()); + + for issue in issues { + match issue { + Issue::InvalidArgs(cmd, range) => { + if check_invalid_args { + errors.push(Arc::new(CodeS12Inspector::new( + range, + format!("Bad Args: {cmd}"), + None, + config.severity(), + processed, + ))); + } + } + Issue::Undefined(var, range, is_child) => { + if check_undefined { + let error_hint = if is_child {Some("From Child Code - may not be a problem".to_owned())} else {None}; + errors.push(Arc::new(CodeS12Inspector::new( + range, + format!("Undefined: {var}"), + error_hint, + config.severity(), + processed, + ))); + } + } + Issue::NotPrivate(var, range) => { + if check_not_private { + errors.push(Arc::new(CodeS12Inspector::new( + range, + format!("NotPrivate: {var}"), + None, + config.severity(), + processed, + ))); + } + } + Issue::Unused(var, range) => { + if check_unused { + errors.push(Arc::new(CodeS12Inspector::new( + range, + format!("Unused: {var}"), + None, + config.severity(), + processed, + ))); + } + } + Issue::Shadowed(var, range) => { + if check_shadow { + errors.push(Arc::new(CodeS12Inspector::new( + range, + format!("Shadowed: {var}"), + None, + config.severity(), + processed, + ))); + } + } + }; + } + + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS12Inspector { + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS12Inspector { + fn ident(&self) -> &'static str { + "L-S12" + } + + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#inspector") + } + /// Top message + fn message(&self) -> String { + format!("Inspector - {}", self.error_type) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + + fn severity(&self) -> Severity { + self.severity + } + + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS12Inspector { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/analyze/mod.rs b/libs/sqf/src/analyze/mod.rs index 7c7ae3b4..4a310a87 100644 --- a/libs/sqf/src/analyze/mod.rs +++ b/libs/sqf/src/analyze/mod.rs @@ -2,6 +2,8 @@ pub mod lints { automod::dir!(pub "src/analyze/lints"); } +pub mod inspector; + use std::sync::Arc; use hemtt_common::config::ProjectConfig; diff --git a/libs/sqf/src/lib.rs b/libs/sqf/src/lib.rs index fb240601..3174a6ba 100644 --- a/libs/sqf/src/lib.rs +++ b/libs/sqf/src/lib.rs @@ -23,6 +23,7 @@ pub struct Statements { /// This isn't required to actually be anything significant, but will be displayed in-game if a script error occurs. source: Arc, span: Range, + top_level: bool, } impl Statements { diff --git a/libs/sqf/src/parser/mod.rs b/libs/sqf/src/parser/mod.rs index 108b4a6e..6634352b 100644 --- a/libs/sqf/src/parser/mod.rs +++ b/libs/sqf/src/parser/mod.rs @@ -43,6 +43,7 @@ pub fn run(database: &Database, processed: &Processed) -> Result( source: processed.extract(span.clone()), span, content, + top_level: false, }) }) } diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs new file mode 100644 index 00000000..7c2e065f --- /dev/null +++ b/libs/sqf/tests/inspector.rs @@ -0,0 +1,105 @@ +use std::sync::Arc; + +use hemtt_sqf::Statements; +use hemtt_workspace::reporting::Processed; + +pub use float_ord::FloatOrd as Scalar; +use hemtt_preprocessor::Processor; +use hemtt_sqf::parser::database::Database; +use hemtt_workspace::LayerType; +const ROOT: &str = "tests/inspector/"; + +fn get_statements(file: &str) -> (Processed, Statements, Arc) { + let folder = std::path::PathBuf::from(ROOT); + let workspace = hemtt_workspace::Workspace::builder() + .physical(&folder, LayerType::Source) + .finish(None, false, &hemtt_common::config::PDriveOption::Disallow) + .unwrap(); + let source = workspace.join(file).unwrap(); + let processed = Processor::run(&source).unwrap(); + let statements = hemtt_sqf::parser::run(&Database::a3(false), &processed).unwrap(); + let database = Arc::new(Database::a3(false)); + (processed, statements, database) +} + +#[cfg(test)] +mod tests { + use crate::get_statements; + use hemtt_sqf::analyze::inspector::{self, Issue}; + + #[test] + pub fn test_0() { + let (pro, sqf, database) = get_statements("test_0.sqf"); + let result = inspector::run_processed(&sqf, &pro, &database, true); + println!("done: {}, {result:?}", result.len()); + } + + #[test] + pub fn test_1() { + let (pro, sqf, database) = get_statements("test_1.sqf"); + let result = inspector::run_processed(&sqf, &pro, &database, true); + println!("done: {result:?}"); + assert_eq!(result.len(), 6); + // Order not guarenteed + assert!(result + .iter() + .find(|i| { + if let Issue::InvalidArgs(cmd, _) = i { + cmd == "[B:setFuel]" + } else { + false + } + }) + .is_some()); + assert!(result + .iter() + .find(|i| { + if let Issue::Undefined(var, _, _) = i { + var == "_guy" + } else { + false + } + }) + .is_some()); + assert!(result + .iter() + .find(|i| { + if let Issue::NotPrivate(var, _) = i { + var == "_z" + } else { + false + } + }) + .is_some()); + assert!(result + .iter() + .find(|i| { + if let Issue::Unused(var, _) = i { + var == "_c" + } else { + false + } + }) + .is_some()); + assert!(result + .iter() + .find(|i| { + if let Issue::Shadowed(var, _) = i { + var == "_var1" + } else { + false + } + }) + .is_some()); + assert!(result + .iter() + .find(|i| { + if let Issue::InvalidArgs(var, _) = i { + var == "[B:addPublicVariableEventHandler]" + } else { + false + } + }) + .is_some()); + } +} diff --git a/libs/sqf/tests/inspector/test_0.sqf b/libs/sqf/tests/inspector/test_0.sqf new file mode 100644 index 00000000..e69de29b diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf new file mode 100644 index 00000000..8c020b5f --- /dev/null +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -0,0 +1,48 @@ +x setFuel true; // args: takes number 0-1 +x setFuel f; +_guy setDamage 1; // _guy undefeind +private _you = player; +_you setDamage 0.5; +_z = 7; // not private +systemChat str _z; +private "_a"; +_a = 8; +private ["_b"]; +_b = _a + 1; +private _c = _b; // unused +params ["_var1"]; +private _var1 = 10; // shadow +diag_log text str _var1; +gx = []; +gx addPublicVariableEventHandler {}; // args: takes lhs string + +for "_i" from 1 to 20 step 0.5 do { + systemChat str _i; +}; +for [{private _k = 0}, {_k < 5}, {_k = _k + 1}] do { + systemChat str _k; +}; + +//IGNORE_PRIVATE_WARNING["_fromUpper"]; +X = _fromUpper; + +[] call { + private "_weird"; + //IGNORE_PRIVATE_WARNING["_weird"] - // No way to know the order is different + for "_i" from 1 to 5 do { + if (_i%2 == 0) then { + truck lock _weird; + }; + if (_i%2 == 1) then { + _weird = 0.5; + }; + }; +}; + +// IGNORE_PRIVATE_WARNING["somePFEH"] - // otherwise will assume it's nil +if (z) then { + somePFEH = nil; +}; +if (y) then { + setObjectViewDistance somePFEH; +}; diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 50331c52..2547454a 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -81,3 +81,4 @@ analyze!(s07_select_parse_number); analyze!(s08_format_args); analyze!(s09_banned_command); analyze!(s11_if_not_else); +analyze!(s12_inspector); diff --git a/libs/sqf/tests/lints/project_tests.toml b/libs/sqf/tests/lints/project_tests.toml index bf02cf4a..e18d39f1 100644 --- a/libs/sqf/tests/lints/project_tests.toml +++ b/libs/sqf/tests/lints/project_tests.toml @@ -6,5 +6,8 @@ options.ignore = [ "addPublicVariableEventHandler", ] +[lints.sqf.inspector] +options.check_shadow = true + [lints.sqf] if_not_else = true diff --git a/libs/sqf/tests/lints/s06_find_in_str/source.sqf b/libs/sqf/tests/lints/s06_find_in_str/source.sqf index d35ed088..97e87e20 100644 --- a/libs/sqf/tests/lints/s06_find_in_str/source.sqf +++ b/libs/sqf/tests/lints/s06_find_in_str/source.sqf @@ -1,2 +1,2 @@ "foobar" find "bar" > -1; -private _hasBar = _things find "bar" > -1; +private _hasBar = things find "bar" > -1; diff --git a/libs/sqf/tests/lints/s06_find_in_str/stdout.ansi b/libs/sqf/tests/lints/s06_find_in_str/stdout.ansi index a9457ac6..bd696b26 100644 --- a/libs/sqf/tests/lints/s06_find_in_str/stdout.ansi +++ b/libs/sqf/tests/lints/s06_find_in_str/stdout.ansi @@ -10,8 +10,8 @@ help[L-S06]: string search using `in` is faster than `find` ┌─ source.sqf:2:19 │ -2 │ private _hasBar = _things find "bar" > -1; - │ ^^^^^^^^^^^^^^^^^^^^^^^ using `find` with -1 +2 │ private _hasBar = things find "bar" > -1; + │ ^^^^^^^^^^^^^^^^^^^^^^ using `find` with -1 │ - = try: "bar" in _things + = try: "bar" in things diff --git a/libs/sqf/tests/lints/s12_inspector/source.sqf b/libs/sqf/tests/lints/s12_inspector/source.sqf new file mode 100644 index 00000000..2c423c03 --- /dev/null +++ b/libs/sqf/tests/lints/s12_inspector/source.sqf @@ -0,0 +1,4 @@ +x setFuel true; // args: takes number 0-1 + +params ["_var1"]; +private _var1 = 7; \ No newline at end of file diff --git a/libs/sqf/tests/lints/s12_inspector/stdout.ansi b/libs/sqf/tests/lints/s12_inspector/stdout.ansi new file mode 100644 index 00000000..5eb6ccd6 --- /dev/null +++ b/libs/sqf/tests/lints/s12_inspector/stdout.ansi @@ -0,0 +1,13 @@ +help[L-S12]: Inspector - Bad Args: [B:setFuel] + ┌─ source.sqf:1:3 + │ +1 │ x setFuel true; // args: takes number 0-1 + │ ^^^^^^^ + + +help[L-S12]: Inspector - Shadowed: _var1 + ┌─ source.sqf:4:1 + │ +4 │ private _var1 = 7; + │ ^^^^^^^^^^^^^^^^^ + From c8ad10eccd8ad5d5e71c5881b6e41f8ec1f059ca Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Thu, 26 Sep 2024 22:49:52 -0500 Subject: [PATCH 02/16] Handle common external funcs, only show shadowing at same level --- libs/sqf/src/analyze/inspector/commands.rs | 312 +++++++++++++++++ .../analyze/inspector/external_functions.rs | 108 ++++++ libs/sqf/src/analyze/inspector/mod.rs | 322 +----------------- libs/sqf/tests/inspector.rs | 119 ++++--- libs/sqf/tests/inspector/test_1.sqf | 30 ++ libs/sqf/tests/lints/s12_inspector/source.sqf | 2 +- .../sqf/tests/lints/s12_inspector/stdout.ansi | 14 +- 7 files changed, 528 insertions(+), 379 deletions(-) create mode 100644 libs/sqf/src/analyze/inspector/commands.rs create mode 100644 libs/sqf/src/analyze/inspector/external_functions.rs diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs new file mode 100644 index 00000000..8be99afe --- /dev/null +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -0,0 +1,312 @@ +use std::{collections::HashSet, ops::Range}; + +use crate::{analyze::inspector::VarSource, Expression}; +use tracing::error; + +use super::{game_value::GameValue, SciptScope}; + +impl SciptScope { + #[must_use] + pub fn cmd_u_private(&mut self, rhs: &HashSet) -> HashSet { + fn push_var(s: &mut SciptScope, var: &String, source: &Range) { + if s.ignored_vars.contains(&var.to_ascii_lowercase()) { + s.var_assign( + &var.to_string(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Ignore, + ); + } else { + s.var_assign( + &var.to_string(), + true, + HashSet::from([GameValue::Nothing]), + VarSource::Real(source.clone()), + ); + } + } + for possible in rhs { + if let GameValue::Array(Some(Expression::Array(array, _))) = possible { + for element in array { + let Expression::String(var, source, _) = element else { + continue; + }; + if var.is_empty() { + continue; + } + push_var(self, &var.to_string(), source); + } + } + if let GameValue::String(Some(Expression::String(var, source, _))) = possible { + if var.is_empty() { + continue; + } + push_var(self, &var.to_string(), source); + } + } + HashSet::new() + } + #[must_use] + pub fn cmd_generic_params(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + continue; + }; + + for entry in array { + match entry { + Expression::String(var, source, _) => { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Real(source.clone()), + ); + } + Expression::Array(var_array, _) => { + if !var_array.is_empty() { + if let Expression::String(var, source, _) = &var_array[0] { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Real(source.clone()), + ); + } + } + } + _ => {} + } + } + } + HashSet::from([GameValue::Boolean(None)]) + } + #[must_use] + pub fn cmd_generic_call(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + self.code_used.insert(expression.clone()); + self.eval_statements(statements); + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_b_do( + &mut self, + lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + // look for forType vars with valid strings (ignore old style code) + let mut do_run = true; + for possible in lhs { + if let GameValue::ForType(option) = possible { + match option { + Some(Expression::String(var, source, _)) => { + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Number(None)]), + VarSource::Real(source.clone()), + ); + } + Some(Expression::Array(array, _)) => { + if array.len() != 3 { + error!("for wrong len"); + continue; + } + for for_stage in array { + let Expression::Code(for_statements) = for_stage else { + continue; + }; + self.code_used.insert(for_stage.clone()); + self.eval_statements(for_statements); + } + } + None => { + do_run = false; + } + _ => { + unreachable!(""); + } + } + } + } + self.code_used.insert(expression.clone()); + if do_run { + self.eval_statements(statements); + } + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_generic_call_magic( + &mut self, + code_possibilities: &HashSet, + magic: &Vec, + source: &Range, + ) -> HashSet { + for possible in code_possibilities { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + for var in magic { + self.var_assign( + var, + true, + HashSet::from([GameValue::Anything]), + VarSource::Magic(source.clone()), + ); + } + self.code_used.insert(expression.clone()); + self.eval_statements(statements); + self.pop(); + } + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_for(&mut self, rhs: &HashSet) -> HashSet { + let mut return_value = HashSet::new(); + for possible in rhs { + match possible { + GameValue::Array(option) | GameValue::String(option) => { + return_value.insert(GameValue::ForType(option.clone())); + } + _ => { + error!("shouldn't be reachable?"); + return_value.insert(GameValue::ForType(None)); + } + } + } + return_value + } + #[must_use] + /// for (from, to, step) chained commands + pub fn cmd_b_from_chain( + &self, + lhs: &HashSet, + _rhs: &HashSet, + ) -> HashSet { + lhs.clone() + } + #[must_use] + pub fn cmd_u_is_nil(&mut self, rhs: &HashSet) -> HashSet { + let mut non_string = false; + for possible in rhs { + let GameValue::String(possible) = possible else { + non_string = true; + continue; + }; + let Some(expression) = possible else { + continue; + }; + let Expression::String(var, _, _) = expression else { + continue; + }; + let _ = self.var_retrieve(var, &expression.span(), true); + } + if non_string { + let _ = self.cmd_generic_call(rhs); + } + HashSet::from([GameValue::Boolean(None)]) + } + #[must_use] + pub fn cmd_b_then( + &mut self, + _lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + let mut return_value = HashSet::new(); + for possible in rhs { + if let GameValue::Code(Some(Expression::Code(_statements))) = possible { + return_value.extend(self.cmd_generic_call(rhs)); + } + if let GameValue::Array(Some(Expression::Array(array, _))) = possible { + for expression in array { + return_value.extend(self.cmd_generic_call(&HashSet::from([GameValue::Code( + Some(expression.clone()), + )]))); + } + } + } + return_value + } + #[must_use] + pub fn cmd_b_else( + &self, + lhs: &HashSet, + rhs: &HashSet, + ) -> HashSet { + let mut return_value = HashSet::new(); // just merge, not really the same but should be fine + for possible in rhs { + return_value.insert(possible.clone()); + } + for possible in lhs { + return_value.insert(possible.clone()); + } + return_value + } + #[must_use] + pub fn cmd_b_get_or_default_call(&mut self, rhs: &HashSet) -> HashSet { + let mut possible_code = HashSet::new(); + for possible in rhs { + let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + continue; + }; + if array.len() < 2 { + continue; + } + possible_code.insert(GameValue::Code(Some(array[1].clone()))); + } + let _ = self.cmd_generic_call(&possible_code); + HashSet::from([GameValue::Anything]) + } + #[must_use] + pub fn cmd_u_to_string(&mut self, rhs: &HashSet) -> HashSet { + for possible in rhs { + let GameValue::Code(Some(expression)) = possible else { + continue; + }; + let Expression::Code(_) = expression else { + continue; + }; + // just skip because it will often use a _x + self.code_used.insert(expression.clone()); + } + HashSet::from([GameValue::String(None)]) + } +} diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs new file mode 100644 index 00000000..311709fa --- /dev/null +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -0,0 +1,108 @@ +use std::collections::HashSet; + +use crate::{analyze::inspector::VarSource, Expression}; + +use super::{game_value::GameValue, SciptScope}; + +impl SciptScope { + pub fn external_function(&mut self, lhs: &HashSet, rhs: &Expression) { + let Expression::Variable(var, _) = rhs else { + return; + }; + for possible in lhs { + let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + continue; + }; + match var.to_ascii_lowercase().as_str() { + "cba_fnc_hasheachpair" | "cba_fnc_hashfilter" => { + if array.len() > 1 { + let code = self.eval_expression(&array[1]); + self.external_current_scope( + &code, + &vec![ + ("_key", GameValue::Anything), + ("_value", GameValue::Anything), + ], + ); + } + } + "cba_fnc_filter" => { + if array.len() > 1 { + let code = self.eval_expression(&array[1]); + self.external_current_scope(&code, &vec![("_x", GameValue::Anything)]); + } + } + "cba_fnc_directcall" => { + if !array.is_empty() { + let code = self.eval_expression(&array[0]); + self.external_current_scope(&code, &vec![]); + } + } + "ace_interact_menu_fnc_createaction" => { + for index in 3..5 { + if array.len() > index { + let code = self.eval_expression(&array[index]); + self.external_new_scope( + &code, + &vec![ + ("_target", GameValue::Object), + ("_player", GameValue::Object), + ], + ); + } + } + } + _ => {} + } + } + } + fn external_new_scope( + &mut self, + possible_arg: &HashSet, + vars: &Vec<(&str, GameValue)>, + ) { + for element in possible_arg { + let GameValue::Code(Some(expression)) = element else { + continue; + }; + let Expression::Code(statements) = expression else { + return; + }; + if self.code_used.contains(expression) { + return; + } + let mut ext_scope = Self::create(&self.ignored_vars, &self.database, true); + + for (var, value) in vars { + ext_scope.var_assign(var, true, HashSet::from([value.clone()]), VarSource::Ignore); + } + self.code_used.insert(expression.clone()); + ext_scope.eval_statements(statements); + self.errors.extend(ext_scope.finish(false)); + } + } + fn external_current_scope( + &mut self, + possible_arg: &HashSet, + vars: &Vec<(&str, GameValue)>, + ) { + for element in possible_arg { + let GameValue::Code(Some(expression)) = element else { + continue; + }; + let Expression::Code(statements) = expression else { + continue; + }; + if self.code_used.contains(expression) { + continue; + } + self.push(); + for (var, value) in vars { + self.var_assign(var, true, HashSet::from([value.clone()]), VarSource::Ignore); + } + self.code_used.insert(expression.clone()); + self.eval_statements(statements); + self.pop(); + } + } +} diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 0ea2c936..e27ca3d5 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -16,6 +16,8 @@ use hemtt_workspace::reporting::Processed; use regex::Regex; use tracing::{error, trace}; +mod commands; +mod external_functions; mod game_value; #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -169,7 +171,8 @@ impl SciptScope { )); } } else if local { - if source.check_shadow() { + // Only check shadowing inside the same scope-level (could make an option) + if source.check_shadow() && stack_level_search.unwrap_or_default() == 0 { self.errors.insert(Issue::Shadowed( var.to_owned(), source.get_range().unwrap_or_default(), @@ -237,309 +240,6 @@ impl SciptScope { set } } - #[must_use] - pub fn cmd_u_private(&mut self, rhs: &HashSet) -> HashSet { - fn push_var(s: &mut SciptScope, var: &String, source: &Range) { - if s.ignored_vars.contains(&var.to_ascii_lowercase()) { - s.var_assign( - &var.to_string(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Ignore, - ); - } else { - s.var_assign( - &var.to_string(), - true, - HashSet::from([GameValue::Nothing]), - VarSource::Real(source.clone()), - ); - } - } - for possible in rhs { - if let GameValue::Array(Some(Expression::Array(array, _))) = possible { - for element in array { - let Expression::String(var, source, _) = element else { - continue; - }; - if var.is_empty() { - continue; - } - push_var(self, &var.to_string(), source); - } - } - if let GameValue::String(Some(Expression::String(var, source, _))) = possible { - if var.is_empty() { - continue; - } - push_var(self, &var.to_string(), source); - } - } - HashSet::new() - } - #[must_use] - pub fn cmd_generic_params(&mut self, rhs: &HashSet) -> HashSet { - for possible in rhs { - let GameValue::Array(Some(Expression::Array(array, _))) = possible else { - continue; - }; - - for entry in array { - match entry { - Expression::String(var, source, _) => { - if var.is_empty() { - continue; - } - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Real(source.clone()), - ); - } - Expression::Array(var_array, _) => { - if !var_array.is_empty() { - if let Expression::String(var, source, _) = &var_array[0] { - if var.is_empty() { - continue; - } - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Real(source.clone()), - ); - } - } - } - _ => {} - } - } - } - HashSet::from([GameValue::Boolean(None)]) - } - #[must_use] - pub fn cmd_generic_call(&mut self, rhs: &HashSet) -> HashSet { - for possible in rhs { - let GameValue::Code(Some(expression)) = possible else { - continue; - }; - let Expression::Code(statements) = expression else { - continue; - }; - if self.code_used.contains(expression) { - continue; - } - self.push(); - self.code_used.insert(expression.clone()); - self.eval_statements(statements); - self.pop(); - } - HashSet::from([GameValue::Anything]) - } - #[must_use] - pub fn cmd_b_do( - &mut self, - lhs: &HashSet, - rhs: &HashSet, - ) -> HashSet { - for possible in rhs { - let GameValue::Code(Some(expression)) = possible else { - continue; - }; - let Expression::Code(statements) = expression else { - continue; - }; - if self.code_used.contains(expression) { - continue; - } - self.push(); - // look for forType vars with valid strings (ignore old style code) - let mut do_run = true; - for possible in lhs { - if let GameValue::ForType(option) = possible { - match option { - Some(Expression::String(var, source, _)) => { - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Number(None)]), - VarSource::Real(source.clone()), - ); - } - Some(Expression::Array(array, _)) => { - if array.len() != 3 { - error!("for wrong len"); - continue; - } - for for_stage in array { - let Expression::Code(for_statements) = for_stage else { - continue; - }; - self.code_used.insert(for_stage.clone()); - self.eval_statements(for_statements); - } - } - None => { - do_run = false; - } - _ => { - unreachable!(""); - } - } - } - } - self.code_used.insert(expression.clone()); - if do_run { - self.eval_statements(statements); - } - self.pop(); - } - HashSet::from([GameValue::Anything]) - } - #[must_use] - pub fn cmd_generic_call_magic( - &mut self, - code_possibilities: &HashSet, - magic: &Vec, - source: &Range, - ) -> HashSet { - for possible in code_possibilities { - let GameValue::Code(Some(expression)) = possible else { - continue; - }; - let Expression::Code(statements) = expression else { - continue; - }; - if self.code_used.contains(expression) { - continue; - } - self.push(); - for var in magic { - self.var_assign( - var, - true, - HashSet::from([GameValue::Anything]), - VarSource::Magic(source.clone()), - ); - } - self.code_used.insert(expression.clone()); - self.eval_statements(statements); - self.pop(); - } - HashSet::from([GameValue::Anything]) - } - #[must_use] - pub fn cmd_for(&mut self, rhs: &HashSet) -> HashSet { - let mut return_value = HashSet::new(); - for possible in rhs { - match possible { - GameValue::Array(option) | GameValue::String(option) => { - return_value.insert(GameValue::ForType(option.clone())); - } - _ => { - error!("shouldn't be reachable?"); - return_value.insert(GameValue::ForType(None)); - } - } - } - return_value - } - #[must_use] - /// for (from, to, step) chained commands - pub fn cmd_b_from_chain( - &self, - lhs: &HashSet, - _rhs: &HashSet, - ) -> HashSet { - lhs.clone() - } - #[must_use] - pub fn cmd_u_is_nil(&mut self, rhs: &HashSet) -> HashSet { - let mut non_string = false; - for possible in rhs { - let GameValue::String(possible) = possible else { - non_string = true; - continue; - }; - let Some(expression) = possible else { - continue; - }; - let Expression::String(var, _, _) = expression else { - continue; - }; - let _ = self.var_retrieve(var, &expression.span(), true); - } - if non_string { - let _ = self.cmd_generic_call(rhs); - } - HashSet::from([GameValue::Boolean(None)]) - } - #[must_use] - pub fn cmd_b_then( - &mut self, - _lhs: &HashSet, - rhs: &HashSet, - ) -> HashSet { - let mut return_value = HashSet::new(); - for possible in rhs { - if let GameValue::Code(Some(Expression::Code(_statements))) = possible { - return_value.extend(self.cmd_generic_call(rhs)); - } - if let GameValue::Array(Some(Expression::Array(array, _))) = possible { - for expression in array { - return_value.extend(self.cmd_generic_call(&HashSet::from([GameValue::Code( - Some(expression.clone()), - )]))); - } - } - } - return_value - } - #[must_use] - pub fn cmd_b_else( - &self, - lhs: &HashSet, - rhs: &HashSet, - ) -> HashSet { - let mut return_value = HashSet::new(); // just merge, not really the same but should be fine - for possible in rhs { - return_value.insert(possible.clone()); - } - for possible in lhs { - return_value.insert(possible.clone()); - } - return_value - } - #[must_use] - pub fn cmd_b_get_or_default_call(&mut self, rhs: &HashSet) -> HashSet { - let mut possible_code = HashSet::new(); - for possible in rhs { - let GameValue::Array(Some(Expression::Array(array, _))) = possible else { - continue; - }; - if array.len() < 2 { - continue; - } - possible_code.insert(GameValue::Code(Some(array[1].clone()))); - } - let _ = self.cmd_generic_call(&possible_code); - HashSet::from([GameValue::Anything]) - } - #[must_use] - pub fn cmd_u_to_string(&mut self, rhs: &HashSet) -> HashSet { - for possible in rhs { - let GameValue::Code(Some(expression)) = possible else { - continue; - }; - let Expression::Code(_) = expression else { - continue; - }; - // just skip because it will often use a _x - self.code_used.insert(expression.clone()); - } - HashSet::from([GameValue::String(None)]) - } #[must_use] #[allow(clippy::too_many_lines)] @@ -620,7 +320,10 @@ impl SciptScope { BinaryCommand::Else => Some(self.cmd_b_else(&lhs_set, &rhs_set)), BinaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { "params" => Some(self.cmd_generic_params(&rhs_set)), - "call" => Some(self.cmd_generic_call(&rhs_set)), + "call" => { + self.external_function(&lhs_set, rhs); + Some(self.cmd_generic_call(&rhs_set)) + } "exitwith" => { // todo: handle scope exits Some(self.cmd_generic_call(&rhs_set)) @@ -718,8 +421,8 @@ pub fn run_processed( database: &Arc, check_child_scripts: bool, ) -> Vec { - let mut ignored_vars = Vec::new(); - ignored_vars.push("_this".to_string()); + let mut ignored_vars = HashSet::new(); + ignored_vars.insert("_this".to_string()); let Ok(re1) = Regex::new(r"\/\/ ?IGNORE_PRIVATE_WARNING ?\[(.*)\]") else { return Vec::new(); }; @@ -729,11 +432,12 @@ pub fn run_processed( for (_path, raw_source) in processed.sources() { for (_, [ignores]) in re1.captures_iter(&raw_source).map(|c| c.extract()) { for (_, [var]) in re2.captures_iter(ignores).map(|c| c.extract()) { - ignored_vars.push(var.to_ascii_lowercase()); + ignored_vars.insert(var.to_ascii_lowercase()); } } } - let mut scope = SciptScope::create(&HashSet::from_iter(ignored_vars), database, false); + + let mut scope = SciptScope::create(&ignored_vars, database, false); scope.eval_statements(statements); scope.finish(check_child_scripts).into_iter().collect() } diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index 7c2e065f..f6b0f74f 100644 --- a/libs/sqf/tests/inspector.rs +++ b/libs/sqf/tests/inspector.rs @@ -38,68 +38,63 @@ mod tests { pub fn test_1() { let (pro, sqf, database) = get_statements("test_1.sqf"); let result = inspector::run_processed(&sqf, &pro, &database, true); - println!("done: {result:?}"); - assert_eq!(result.len(), 6); + assert_eq!(result.len(), 8); // Order not guarenteed - assert!(result - .iter() - .find(|i| { - if let Issue::InvalidArgs(cmd, _) = i { - cmd == "[B:setFuel]" - } else { - false - } - }) - .is_some()); - assert!(result - .iter() - .find(|i| { - if let Issue::Undefined(var, _, _) = i { - var == "_guy" - } else { - false - } - }) - .is_some()); - assert!(result - .iter() - .find(|i| { - if let Issue::NotPrivate(var, _) = i { - var == "_z" - } else { - false - } - }) - .is_some()); - assert!(result - .iter() - .find(|i| { - if let Issue::Unused(var, _) = i { - var == "_c" - } else { - false - } - }) - .is_some()); - assert!(result - .iter() - .find(|i| { - if let Issue::Shadowed(var, _) = i { - var == "_var1" - } else { - false - } - }) - .is_some()); - assert!(result - .iter() - .find(|i| { - if let Issue::InvalidArgs(var, _) = i { - var == "[B:addPublicVariableEventHandler]" - } else { - false - } - }) - .is_some()); + assert!(result.iter().any(|i| { + if let Issue::InvalidArgs(cmd, _) = i { + cmd == "[B:setFuel]" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Undefined(var, _, _) = i { + var == "_guy" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::NotPrivate(var, _) = i { + var == "_z" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Unused(var, _) = i { + var == "_c" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Shadowed(var, _) = i { + var == "_var1" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::InvalidArgs(var, _) = i { + var == "[B:addPublicVariableEventHandler]" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::InvalidArgs(var, _) = i { + var == "[B:ctrlSetText]" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Undefined(var, _, _) = i { + var == "_myLocalVar1" + } else { + false + } + })); } } diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index 8c020b5f..b39b4ff2 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -46,3 +46,33 @@ if (z) then { if (y) then { setObjectViewDistance somePFEH; }; + +private _condition = + { + [_player, _target] call y + }; +[ + "", + localize "STR_A3_Arsenal", + "", + { + x ctrlSetText _player; // bad arg type + [_target, _player] call z; + }, + _condition +] call ace_interact_menu_fnc_createAction; + +private _hash = [] call CBA_fnc_hashCreate; +private _dumpHash = { + diag_log format ["Key: %1, Value: %2", _key, _value]; +}; +[_hash, _dumpHash] call CBA_fnc_hashEachPair; + +private _myLocalVar1 = 555; +_myLocalVar1 = _myLocalVar1 + 1; +[{ + systemChat str _myLocalVar1; // invalid +}, 0, []] call CBA_fnc_addPerFrameHandler; + +private _myLocalVar2 = 55; +[{systemChat str _myLocalVar2}] call CBA_fnc_directCall; // fine diff --git a/libs/sqf/tests/lints/s12_inspector/source.sqf b/libs/sqf/tests/lints/s12_inspector/source.sqf index 2c423c03..e91864ff 100644 --- a/libs/sqf/tests/lints/s12_inspector/source.sqf +++ b/libs/sqf/tests/lints/s12_inspector/source.sqf @@ -1,4 +1,4 @@ x setFuel true; // args: takes number 0-1 params ["_var1"]; -private _var1 = 7; \ No newline at end of file +private _var1 = 7; diff --git a/libs/sqf/tests/lints/s12_inspector/stdout.ansi b/libs/sqf/tests/lints/s12_inspector/stdout.ansi index 5eb6ccd6..736c60c9 100644 --- a/libs/sqf/tests/lints/s12_inspector/stdout.ansi +++ b/libs/sqf/tests/lints/s12_inspector/stdout.ansi @@ -1,13 +1,13 @@ -help[L-S12]: Inspector - Bad Args: [B:setFuel] - ┌─ source.sqf:1:3 - │ -1 │ x setFuel true; // args: takes number 0-1 - │ ^^^^^^^ - - help[L-S12]: Inspector - Shadowed: _var1 ┌─ source.sqf:4:1 │ 4 │ private _var1 = 7; │ ^^^^^^^^^^^^^^^^^ + +help[L-S12]: Inspector - Bad Args: [B:setFuel] + ┌─ source.sqf:1:3 + │ +1 │ x setFuel true; // args: takes number 0-1 + │ ^^^^^^^ + From 03f3ac935b0c6fd2af944e39e3c62a079f0ca35f Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sat, 28 Sep 2024 00:27:35 -0500 Subject: [PATCH 03/16] Check arg arrays --- Cargo.lock | 3 +- Cargo.toml | 3 +- libs/sqf/src/analyze/inspector/commands.rs | 179 ++++++++++-------- .../analyze/inspector/external_functions.rs | 60 +++--- libs/sqf/src/analyze/inspector/game_value.rs | 103 ++++++++-- libs/sqf/src/analyze/inspector/mod.rs | 50 +++-- libs/sqf/src/analyze/lints/s12_inspector.rs | 11 +- libs/sqf/tests/inspector.rs | 52 +++-- libs/sqf/tests/inspector/test_1.sqf | 76 +++++--- libs/sqf/tests/inspector/test_2.sqf | 26 +++ 10 files changed, 373 insertions(+), 190 deletions(-) create mode 100644 libs/sqf/tests/inspector/test_2.sqf diff --git a/Cargo.lock b/Cargo.lock index c21014d2..dc123f08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -190,8 +190,7 @@ dependencies = [ [[package]] name = "arma3-wiki" version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "845a808d6724494856cec8655fd9f64c73d3e52cdaf3f3d2eb3a247af8f29149" +source = "git+https://github.com/acemod/arma3-wiki.git?rev=cd147399fecd21253cba1e5348b7f0e7d7837948#cd147399fecd21253cba1e5348b7f0e7d7837948" dependencies = [ "directories", "fs_extra", diff --git a/Cargo.toml b/Cargo.toml index a10e3dc9..6012579b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,8 @@ future_incompatible = "warn" nonstandard_style = "warn" [workspace.dependencies] -arma3-wiki = "0.3.2" +#arma3-wiki = "0.3.2" +arma3-wiki = { git = "https://github.com/acemod/arma3-wiki.git", rev = "cd147399fecd21253cba1e5348b7f0e7d7837948" } automod = "1.0.14" byteorder = "1.5.0" chumsky = "0.9.3" diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index 8be99afe..da6911be 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -1,7 +1,6 @@ use std::{collections::HashSet, ops::Range}; use crate::{analyze::inspector::VarSource, Expression}; -use tracing::error; use super::{game_value::GameValue, SciptScope}; @@ -21,20 +20,23 @@ impl SciptScope { &var.to_string(), true, HashSet::from([GameValue::Nothing]), - VarSource::Real(source.clone()), + VarSource::Private(source.clone()), ); } } for possible in rhs { - if let GameValue::Array(Some(Expression::Array(array, _))) = possible { - for element in array { - let Expression::String(var, source, _) = element else { - continue; - }; - if var.is_empty() { - continue; + if let GameValue::Array(Some(gv_array)) = possible { + for gv_index in gv_array { + for element in gv_index { + let GameValue::String(Some(Expression::String(var, source, _))) = element + else { + continue; + }; + if var.is_empty() { + continue; + } + push_var(self, &var.to_string(), source); } - push_var(self, &var.to_string(), source); } } if let GameValue::String(Some(Expression::String(var, source, _))) = possible { @@ -49,39 +51,46 @@ impl SciptScope { #[must_use] pub fn cmd_generic_params(&mut self, rhs: &HashSet) -> HashSet { for possible in rhs { - let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + let GameValue::Array(Some(gv_array)) = possible else { continue; }; - for entry in array { - match entry { - Expression::String(var, source, _) => { - if var.is_empty() { - continue; + for gv_index in gv_array { + for element in gv_index { + match element { + GameValue::String(Some(Expression::String(var, source, _))) => { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Params(source.clone()), + ); } - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Real(source.clone()), - ); - } - Expression::Array(var_array, _) => { - if !var_array.is_empty() { - if let Expression::String(var, source, _) = &var_array[0] { - if var.is_empty() { - continue; + GameValue::Array(Some(gv_array)) => { + if gv_array.is_empty() { + continue; + } + for element in &gv_array[0] { + if let GameValue::String(Some(Expression::String(var, source, _))) = + element + { + if var.is_empty() { + continue; + } + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Anything]), + VarSource::Params(source.clone()), + ); } - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Real(source.clone()), - ); } } + _ => {} } - _ => {} } } } @@ -123,39 +132,32 @@ impl SciptScope { continue; } self.push(); - // look for forType vars with valid strings (ignore old style code) - let mut do_run = true; + let mut do_run = false; for possible in lhs { if let GameValue::ForType(option) = possible { - match option { - Some(Expression::String(var, source, _)) => { - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Number(None)]), - VarSource::Real(source.clone()), - ); - } - Some(Expression::Array(array, _)) => { - if array.len() != 3 { - error!("for wrong len"); - continue; + let Some(for_args_array) = option else { + continue; + }; + do_run = true; + for stage in for_args_array { + match stage { + Expression::String(var, source, _) => { + self.var_assign( + var.as_ref(), + true, + HashSet::from([GameValue::Number(None)]), + VarSource::ForLoop(source.clone()), + ); } - for for_stage in array { - let Expression::Code(for_statements) = for_stage else { - continue; - }; - self.code_used.insert(for_stage.clone()); - self.eval_statements(for_statements); + Expression::Code(stage_statement) => { + self.code_used.insert(stage.clone()); + self.eval_statements(stage_statement); } - } - None => { - do_run = false; - } - _ => { - unreachable!(""); + _ => {} } } + } else { + do_run = true; } } self.code_used.insert(expression.clone()); @@ -202,14 +204,35 @@ impl SciptScope { pub fn cmd_for(&mut self, rhs: &HashSet) -> HashSet { let mut return_value = HashSet::new(); for possible in rhs { + let mut possible_array = Vec::new(); match possible { - GameValue::Array(option) | GameValue::String(option) => { - return_value.insert(GameValue::ForType(option.clone())); + GameValue::String(option) => { + let Some(expression) = option else { + return_value.insert(GameValue::ForType(None)); + continue; + }; + possible_array.push(expression.clone()); } - _ => { - error!("shouldn't be reachable?"); - return_value.insert(GameValue::ForType(None)); + GameValue::Array(option) => { + let Some(for_stages) = option else { + return_value.insert(GameValue::ForType(None)); + continue; + }; + for stage in for_stages { + for gv in stage { + let GameValue::Code(Some(expression)) = gv else { + continue; + }; + possible_array.push(expression.clone()); + } + } } + _ => {} + } + if possible_array.is_empty() { + return_value.insert(GameValue::ForType(None)); + } else { + return_value.insert(GameValue::ForType(Some(possible_array))); } } return_value @@ -255,11 +278,15 @@ impl SciptScope { if let GameValue::Code(Some(Expression::Code(_statements))) = possible { return_value.extend(self.cmd_generic_call(rhs)); } - if let GameValue::Array(Some(Expression::Array(array, _))) = possible { - for expression in array { - return_value.extend(self.cmd_generic_call(&HashSet::from([GameValue::Code( - Some(expression.clone()), - )]))); + if let GameValue::Array(Some(gv_array)) = possible { + for gv_index in gv_array { + for element in gv_index { + if let GameValue::Code(Some(expression)) = element { + return_value.extend(self.cmd_generic_call(&HashSet::from([ + GameValue::Code(Some(expression.clone())), + ]))); + } + } } } } @@ -283,14 +310,14 @@ impl SciptScope { #[must_use] pub fn cmd_b_get_or_default_call(&mut self, rhs: &HashSet) -> HashSet { let mut possible_code = HashSet::new(); - for possible in rhs { - let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + for possible_outer in rhs { + let GameValue::Array(Some(gv_array)) = possible_outer else { continue; }; - if array.len() < 2 { + if gv_array.len() < 2 { continue; } - possible_code.insert(GameValue::Code(Some(array[1].clone()))); + possible_code.extend(gv_array[1].clone()); } let _ = self.cmd_generic_call(&possible_code); HashSet::from([GameValue::Anything]) diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs index 311709fa..1f0e9250 100644 --- a/libs/sqf/src/analyze/inspector/external_functions.rs +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -6,19 +6,18 @@ use super::{game_value::GameValue, SciptScope}; impl SciptScope { pub fn external_function(&mut self, lhs: &HashSet, rhs: &Expression) { - let Expression::Variable(var, _) = rhs else { + let Expression::Variable(ext_func, _) = rhs else { return; }; for possible in lhs { - let GameValue::Array(Some(Expression::Array(array, _))) = possible else { + let GameValue::Array(Some(gv_array)) = possible else { continue; }; - match var.to_ascii_lowercase().as_str() { + match ext_func.to_ascii_lowercase().as_str() { "cba_fnc_hasheachpair" | "cba_fnc_hashfilter" => { - if array.len() > 1 { - let code = self.eval_expression(&array[1]); + if gv_array.len() > 1 { self.external_current_scope( - &code, + &gv_array[1], &vec![ ("_key", GameValue::Anything), ("_value", GameValue::Anything), @@ -27,23 +26,28 @@ impl SciptScope { } } "cba_fnc_filter" => { - if array.len() > 1 { - let code = self.eval_expression(&array[1]); - self.external_current_scope(&code, &vec![("_x", GameValue::Anything)]); + if gv_array.len() > 1 { + self.external_current_scope( + &gv_array[1], + &vec![("_x", GameValue::Anything)], + ); } } "cba_fnc_directcall" => { - if !array.is_empty() { - let code = self.eval_expression(&array[0]); - self.external_current_scope(&code, &vec![]); + if !gv_array.is_empty() { + self.external_current_scope(&gv_array[0], &vec![]); + } + } + "ace_common_fnc_cachedcall" => { + if gv_array.len() > 1 { + self.external_current_scope(&gv_array[1], &vec![]); } } "ace_interact_menu_fnc_createaction" => { for index in 3..5 { - if array.len() > index { - let code = self.eval_expression(&array[index]); + if gv_array.len() > index { self.external_new_scope( - &code, + &gv_array[index], &vec![ ("_target", GameValue::Object), ("_player", GameValue::Object), @@ -52,16 +56,22 @@ impl SciptScope { } } } + "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" => { + if !gv_array.is_empty() { + self.external_new_scope(&gv_array[0], &vec![]); + } + } + "cba_fnc_addclasseventhandler" => { + if gv_array.len() > 2 { + self.external_new_scope(&gv_array[2], &vec![]); + } + } _ => {} } } } - fn external_new_scope( - &mut self, - possible_arg: &HashSet, - vars: &Vec<(&str, GameValue)>, - ) { - for element in possible_arg { + fn external_new_scope(&mut self, code_arg: &Vec, vars: &Vec<(&str, GameValue)>) { + for element in code_arg { let GameValue::Code(Some(expression)) = element else { continue; }; @@ -81,12 +91,8 @@ impl SciptScope { self.errors.extend(ext_scope.finish(false)); } } - fn external_current_scope( - &mut self, - possible_arg: &HashSet, - vars: &Vec<(&str, GameValue)>, - ) { - for element in possible_arg { + fn external_current_scope(&mut self, code_arg: &Vec, vars: &Vec<(&str, GameValue)>) { + for element in code_arg { let GameValue::Code(Some(expression)) = element else { continue; }; diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs index d5a10407..b24a49d6 100644 --- a/libs/sqf/src/analyze/inspector/game_value.rs +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -9,14 +9,14 @@ use crate::{parser::database::Database, Expression}; pub enum GameValue { Anything, // Assignment, // as in z = call {x=1}??? - Array(Option), + Array(Option>>), Boolean(Option), Code(Option), Config, Control, DiaryRecord, Display, - ForType(Option), + ForType(Option>), Group, HashMap, IfType, @@ -51,6 +51,7 @@ impl GameValue { }; for syntax in command.syntax() { + // println!("syntax {:?}", syntax.call()); match syntax.call() { Call::Nular => { if !matches!(expression, Expression::NularCommand(..)) { @@ -60,7 +61,8 @@ impl GameValue { Call::Unary(rhs_arg) => { if !matches!(expression, Expression::UnaryCommand(..)) || !Self::match_set_to_arg( - rhs_set.expect("u args"), + cmd_name, + rhs_set.expect("unary rhs"), rhs_arg, syntax.params(), ) @@ -71,12 +73,14 @@ impl GameValue { Call::Binary(lhs_arg, rhs_arg) => { if !matches!(expression, Expression::BinaryCommand(..)) || !Self::match_set_to_arg( - lhs_set.expect("b args"), + cmd_name, + lhs_set.expect("binary lhs"), lhs_arg, syntax.params(), ) || !Self::match_set_to_arg( - rhs_set.expect("b args"), + cmd_name, + rhs_set.expect("binary rhs"), rhs_arg, syntax.params(), ) @@ -86,9 +90,12 @@ impl GameValue { } } let value = &syntax.ret().0; + // println!("match syntax {syntax:?}"); return_types.insert(Self::from_wiki_value(value)); } - // trace!( + // println!("lhs_set {lhs_set:?}"); + // println!("rhs_set {rhs_set:?}"); + // println!( // "cmd [{}] = {}:{:?}", // cmd_name, // return_types.len(), @@ -98,25 +105,83 @@ impl GameValue { } #[must_use] - pub fn match_set_to_arg(set: &HashSet, arg: &Arg, params: &[Param]) -> bool { + pub fn match_set_to_arg( + cmd_name: &str, + set: &HashSet, + arg: &Arg, + params: &[Param], + ) -> bool { match arg { Arg::Item(name) => { - // trace!("looking for {name} in {params:?}"); let Some(param) = params.iter().find(|p| p.name() == name) else { - warn!("param not found"); + /// Varadic cmds which will be missing wiki param matches + const WIKI_CMDS_IGNORE_MISSING_PARAM: &[&str] = &[ + "format", + "formatText", + "param", + "params", + "setGroupId", + "setGroupIdGlobal", + "set3DENMissionAttributes", + "setPiPEffect", + ]; + if !WIKI_CMDS_IGNORE_MISSING_PARAM.contains(&cmd_name) { + warn!("cmd {cmd_name} - param {name} not found"); + } return true; }; + // println!( + // "[arg {name}] typ: {:?}, opt: {:?}", + // param.typ(), + // param.optional() + // ); + if param.optional() { + // todo: maybe still check type if opt and not empty/nil? + return true; + } Self::match_set_to_value(set, param.typ()) } - Arg::Array(_vec_arg) => { - // todo: each individual array arg - Self::match_set_to_value(set, &Value::ArrayUnknown) + Arg::Array(arg_array) => { + const WIKI_CMDS_IGNORE_ARGS: &[&str] = &["createHashMapFromArray"]; + if WIKI_CMDS_IGNORE_ARGS.contains(&cmd_name) { + return true; + } + + let test = set.iter().any(|s| { + match s { + Self::Anything | Self::Array(None) => { + // println!("array (any/generic) pass"); + true + } + Self::Array(Some(gv_array)) => { + // println!("array (gv: {}) expected (arg: {})", gv_array.len(), arg_array.len()); + // if gv_array.len() > arg_array.len() { + // not really an error, some syntaxes take more than others + // } + for (index, arg) in arg_array.iter().enumerate() { + let possible = if index < gv_array.len() { + gv_array[index].iter().cloned().collect() + } else { + HashSet::new() + }; + if !Self::match_set_to_arg(cmd_name, &possible, arg, params) { + return false; + } + } + true + } + _ => false, + } + }); + + test } } } #[must_use] pub fn match_set_to_value(set: &HashSet, right_wiki: &Value) -> bool { + // println!("Checking {:?} against {:?}", set, right_wiki); let right = Self::from_wiki_value(right_wiki); set.iter().any(|gv| Self::match_values(gv, &right)) } @@ -189,9 +254,9 @@ impl GameValue { // format!("Assignment") // } Self::Anything => "Anything".to_string(), - Self::ForType(expression) => { - if let Some(Expression::String(str, _, _)) = expression { - format!("ForType(var {str})") + Self::ForType(for_args_array) => { + if for_args_array.is_some() { + format!("ForType(var {for_args_array:?})") } else { "ForType(GENERIC)".to_string() } @@ -217,9 +282,11 @@ impl GameValue { "Boolean(GENERIC)".to_string() } } - Self::Array(expression) => { - if let Some(Expression::Array(array, _)) = expression { - format!("ArrayExp(len {})", array.len()) + Self::Array(gv_array_option) => + { + #[allow(clippy::option_if_let_else)] + if let Some(gv_array) = gv_array_option { + format!("ArrayExp(len {})", gv_array.len()) } else { "ArrayExp(GENERIC)".to_string() } diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index e27ca3d5..68aa79d7 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -24,30 +24,33 @@ mod game_value; pub enum Issue { InvalidArgs(String, Range), Undefined(String, Range, bool), - Unused(String, Range), + Unused(String, VarSource), Shadowed(String, Range), NotPrivate(String, Range), } #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum VarSource { - Real(Range), + Assignment(Range), + ForLoop(Range), + Params(Range), + Private(Range), Magic(Range), Ignore, } impl VarSource { #[must_use] - pub const fn check_unused(&self) -> bool { - matches!(self, Self::Real(..)) - } - #[must_use] - pub const fn check_shadow(&self) -> bool { - matches!(self, Self::Real(..)) + pub const fn skip_errors(&self) -> bool { + matches!(self, Self::Magic(..)) || matches!(self, Self::Ignore) } #[must_use] pub fn get_range(&self) -> Option> { match self { - Self::Real(range) | Self::Magic(range) => Some(range.clone()), + Self::Assignment(range) + | Self::ForLoop(range) + | Self::Params(range) + | Self::Private(range) + | Self::Magic(range) => Some(range.clone()), Self::Ignore => None, } } @@ -129,10 +132,10 @@ impl SciptScope { pub fn pop(&mut self) { for (var, holder) in self.local.pop().unwrap_or_default() { // trace!("-- Stack Pop {}:{} ", var, holder.usage); - if holder.usage == 0 && holder.source.check_unused() { + if holder.usage == 0 && !holder.source.skip_errors() { self.errors.insert(Issue::Unused( var, - holder.source.get_range().unwrap_or_default(), + holder.source, )); } } @@ -172,7 +175,7 @@ impl SciptScope { } } else if local { // Only check shadowing inside the same scope-level (could make an option) - if source.check_shadow() && stack_level_search.unwrap_or_default() == 0 { + if stack_level_search.unwrap_or_default() == 0 && !source.skip_errors() { self.errors.insert(Issue::Shadowed( var.to_owned(), source.get_range().unwrap_or_default(), @@ -254,10 +257,11 @@ impl SciptScope { } Expression::String(..) => HashSet::from([GameValue::String(Some(expression.clone()))]), Expression::Array(array, _) => { - for e in array { - let _ = self.eval_expression(e); - } - HashSet::from([GameValue::Array(Some(expression.clone()))]) + let gv_array: Vec> = array + .iter() + .map(|e| self.eval_expression(e).into_iter().collect()) + .collect(); + HashSet::from([GameValue::Array(Some(gv_array))]) } Expression::NularCommand(cmd, source) => { debug_type = format!("[N:{}]", cmd.as_str()); @@ -394,13 +398,23 @@ impl SciptScope { Statement::AssignGlobal(var, expression, source) => { // x or _x let possible_values = self.eval_expression(expression); - self.var_assign(var, false, possible_values, VarSource::Real(source.clone())); + self.var_assign( + var, + false, + possible_values, + VarSource::Assignment(source.clone()), + ); // return_value = vec![GameValue::Assignment()]; } Statement::AssignLocal(var, expression, source) => { // private _x let possible_values = self.eval_expression(expression); - self.var_assign(var, true, possible_values, VarSource::Real(source.clone())); + self.var_assign( + var, + true, + possible_values, + VarSource::Assignment(source.clone()), + ); // return_value = vec![GameValue::Assignment()]; } Statement::Expression(expression, _) => { diff --git a/libs/sqf/src/analyze/lints/s12_inspector.rs b/libs/sqf/src/analyze/lints/s12_inspector.rs index 376ca1ae..dde7772a 100644 --- a/libs/sqf/src/analyze/lints/s12_inspector.rs +++ b/libs/sqf/src/analyze/lints/s12_inspector.rs @@ -1,6 +1,6 @@ use crate::{ analyze::{ - inspector::{self, Issue}, + inspector::{self, Issue, VarSource}, SqfLintData, }, Statements, @@ -90,7 +90,7 @@ impl LintRunner for Runner { if let Some(toml::Value::Boolean(b)) = config.option("check_child_scripts") { *b } else { - false // can cause false positives + true // can cause false positives }; let check_undefined = if let Some(toml::Value::Boolean(b)) = config.option("check_undefined") { @@ -109,13 +109,13 @@ impl LintRunner for Runner { if let Some(toml::Value::Boolean(b)) = config.option("check_unused") { *b } else { - false + true }; let check_shadow = if let Some(toml::Value::Boolean(b)) = config.option("check_shadow") { *b } else { - false + true }; let mut errors: Codes = Vec::new(); @@ -158,7 +158,8 @@ impl LintRunner for Runner { ))); } } - Issue::Unused(var, range) => { + Issue::Unused(var, source) => { + let VarSource::Assignment(range) = source else { continue }; if check_unused { errors.push(Arc::new(CodeS12Inspector::new( range, diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index f6b0f74f..5771aa63 100644 --- a/libs/sqf/tests/inspector.rs +++ b/libs/sqf/tests/inspector.rs @@ -14,10 +14,10 @@ fn get_statements(file: &str) -> (Processed, Statements, Arc) { let workspace = hemtt_workspace::Workspace::builder() .physical(&folder, LayerType::Source) .finish(None, false, &hemtt_common::config::PDriveOption::Disallow) - .unwrap(); - let source = workspace.join(file).unwrap(); - let processed = Processor::run(&source).unwrap(); - let statements = hemtt_sqf::parser::run(&Database::a3(false), &processed).unwrap(); + .expect("for test"); + let source = workspace.join(file).expect("for test"); + let processed = Processor::run(&source).expect("for test"); + let statements = hemtt_sqf::parser::run(&Database::a3(false), &processed).expect("for test"); let database = Arc::new(Database::a3(false)); (processed, statements, database) } @@ -25,7 +25,7 @@ fn get_statements(file: &str) -> (Processed, Statements, Arc) { #[cfg(test)] mod tests { use crate::get_statements; - use hemtt_sqf::analyze::inspector::{self, Issue}; + use hemtt_sqf::analyze::inspector::{self, Issue, VarSource}; #[test] pub fn test_0() { @@ -38,7 +38,7 @@ mod tests { pub fn test_1() { let (pro, sqf, database) = get_statements("test_1.sqf"); let result = inspector::run_processed(&sqf, &pro, &database, true); - assert_eq!(result.len(), 8); + assert_eq!(result.len(), 11); // Order not guarenteed assert!(result.iter().any(|i| { if let Issue::InvalidArgs(cmd, _) = i { @@ -49,28 +49,28 @@ mod tests { })); assert!(result.iter().any(|i| { if let Issue::Undefined(var, _, _) = i { - var == "_guy" + var == "_test2" } else { false } })); assert!(result.iter().any(|i| { if let Issue::NotPrivate(var, _) = i { - var == "_z" + var == "_test3" } else { false } })); assert!(result.iter().any(|i| { - if let Issue::Unused(var, _) = i { - var == "_c" + if let Issue::Unused(var, source) = i { + var == "_test4" && matches!(source, VarSource::Assignment(_)) } else { false } })); assert!(result.iter().any(|i| { if let Issue::Shadowed(var, _) = i { - var == "_var1" + var == "_test5" } else { false } @@ -91,10 +91,38 @@ mod tests { })); assert!(result.iter().any(|i| { if let Issue::Undefined(var, _, _) = i { - var == "_myLocalVar1" + var == "_test8" } else { false } })); + assert!(result.iter().any(|i| { + if let Issue::Undefined(var, _, _) = i { + var == "_test9" + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Unused(var, source) = i { + var == "_test10" && matches!(source, VarSource::ForLoop(_)) + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Unused(var, source) = i { + var == "_test11" && matches!(source, VarSource::Params(_)) + } else { + false + } + })); + } + + #[test] + pub fn test_2() { + let (pro, sqf, database) = get_statements("test_2.sqf"); + let result = inspector::run_processed(&sqf, &pro, &database, true); + assert_eq!(result.len(), 0); } } diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index b39b4ff2..2d68148c 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -1,26 +1,28 @@ -x setFuel true; // args: takes number 0-1 -x setFuel f; -_guy setDamage 1; // _guy undefeind -private _you = player; -_you setDamage 0.5; -_z = 7; // not private -systemChat str _z; -private "_a"; -_a = 8; -private ["_b"]; -_b = _a + 1; -private _c = _b; // unused -params ["_var1"]; -private _var1 = 10; // shadow -diag_log text str _var1; +a setFuel b; +a setFuel 0; +a setFuel true; // invalidArgs: takes number 0-1 +_test2 setDamage 1; // undefiend +private _var1 = player; +_var1 setDamage 0.5; + +_test3 = 7; // not private +systemChat str _test3; +private "_var2"; +_var2 = 8; +private ["_var3"]; +_var3 = _var2 + 1; +private _test4 = _var3; // unused +params ["_test5"]; +private _test5 = 10; // shadow (same level) +diag_log text str _test5; gx = []; gx addPublicVariableEventHandler {}; // args: takes lhs string -for "_i" from 1 to 20 step 0.5 do { - systemChat str _i; +for "_var5" from 1 to 20 step 0.5 do { + systemChat str _var5; }; -for [{private _k = 0}, {_k < 5}, {_k = _k + 1}] do { - systemChat str _k; +for [{private _var6 = 0}, {_var6 < 5}, {_var6 = _var6 + 1}] do { + systemChat str _var6; }; //IGNORE_PRIVATE_WARNING["_fromUpper"]; @@ -29,11 +31,11 @@ X = _fromUpper; [] call { private "_weird"; //IGNORE_PRIVATE_WARNING["_weird"] - // No way to know the order is different - for "_i" from 1 to 5 do { - if (_i%2 == 0) then { + for "_var7" from 1 to 5 do { + if (_var7%2 == 0) then { truck lock _weird; }; - if (_i%2 == 1) then { + if (_var7%2 == 1) then { _weird = 0.5; }; }; @@ -47,10 +49,12 @@ if (y) then { setObjectViewDistance somePFEH; }; -private _condition = - { - [_player, _target] call y - }; +somehash getOrDefaultCall ["key", {_test8}, true]; //undefined +private _var8 = objNull; +somehash getOrDefaultCall ["key", {_var8}, true]; + +// Will have _player and _target +private _condition = { [_player, _target] call y }; [ "", localize "STR_A3_Arsenal", @@ -64,15 +68,25 @@ private _condition = private _hash = [] call CBA_fnc_hashCreate; private _dumpHash = { + // Will have _key and _value diag_log format ["Key: %1, Value: %2", _key, _value]; }; [_hash, _dumpHash] call CBA_fnc_hashEachPair; -private _myLocalVar1 = 555; -_myLocalVar1 = _myLocalVar1 + 1; +private _test9 = 555; +_test9= _test9 + 1; [{ - systemChat str _myLocalVar1; // invalid + systemChat str _test9; // invalid }, 0, []] call CBA_fnc_addPerFrameHandler; -private _myLocalVar2 = 55; -[{systemChat str _myLocalVar2}] call CBA_fnc_directCall; // fine +private _var9 = 55; +[{systemChat str _var9}] call CBA_fnc_directCall; + + // Will have _x +filter = [orig, {_x + 1}] call CBA_fnc_filter; + +private _var10 = 123; +[player, {x = _var10}] call ace_common_fnc_cachedcall; + +for "_test10" from 1 to 1 step 0.1 do {}; +[5] params ["_test11"]; diff --git a/libs/sqf/tests/inspector/test_2.sqf b/libs/sqf/tests/inspector/test_2.sqf new file mode 100644 index 00000000..de085507 --- /dev/null +++ b/libs/sqf/tests/inspector/test_2.sqf @@ -0,0 +1,26 @@ +// Mainly checking wiki syntax for correct optionals + +// check inner nil +obj addWeaponItem ["weapon", ["item", nil, "muzzle"], true]; +obj addWeaponItem ["weapon", ["item"], true]; + + // check too many/few on variadic +format ["%1 %2 %3 %4 %5", 1, 2, 3, 4, 5]; +format [""]; +[] params []; + +// False positives on wiki +configProperties [configFile >> "ACE_Curator"]; +x selectionPosition [y, "Memory"]; +ropeCreate [obj1, pos1, objNull, [0, 0, 0], dist1]; +lineIntersectsSurfaces [[], [], objNull, objNull, true, 2]; +uuid insert [8, ["-"]]; +createTrigger["EmptyDetector", [1,2,3]]; +showHUD [true,false,false,false,false,false,false,true]; +createVehicle ["", [0,0,0]]; +x drawRectangle [getPos player, 20, 20, getDir player, [0,0,1,1],""]; + +createHashMapFromArray [["empty", {0}]]; +lineIntersectsObjs [eyePos player, ATLToASL screenToWorld [0.5, 0.5]]; +formatText ["%1%2%3", "line1", "
", "line2"]; +[] select [2]; From 8047f7cfa141db7c2daa8c72c92da7aa346b1c94 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 6 Oct 2024 03:01:31 -0500 Subject: [PATCH 04/16] Split-up lints --- libs/sqf/src/analyze/inspector/commands.rs | 75 ++++-- .../analyze/inspector/external_functions.rs | 159 +++++++---- libs/sqf/src/analyze/inspector/game_value.rs | 23 +- libs/sqf/src/analyze/inspector/mod.rs | 108 ++++---- libs/sqf/src/analyze/lints/s12_inspector.rs | 253 ------------------ .../sqf/src/analyze/lints/s12_invalid_args.rs | 135 ++++++++++ libs/sqf/src/analyze/lints/s13_undefined.rs | 149 +++++++++++ libs/sqf/src/analyze/lints/s14_unused.rs | 155 +++++++++++ libs/sqf/src/analyze/lints/s15_shadowed.rs | 136 ++++++++++ libs/sqf/src/analyze/lints/s16_not_private.rs | 135 ++++++++++ libs/sqf/src/lib.rs | 10 +- libs/sqf/src/parser/mod.rs | 5 +- libs/sqf/tests/inspector.rs | 30 ++- libs/sqf/tests/inspector/test_1.sqf | 70 +++-- libs/sqf/tests/lints.rs | 33 ++- libs/sqf/tests/lints/project_tests.toml | 13 +- libs/sqf/tests/lints/s12_inspector/source.sqf | 4 - .../sqf/tests/lints/s12_inspector/stdout.ansi | 13 - .../tests/lints/s12_invalid_args/source.sqf | 3 + .../tests/lints/s12_invalid_args/stdout.ansi | 6 + libs/sqf/tests/lints/s13_undefined/source.sqf | 1 + .../sqf/tests/lints/s13_undefined/stdout.ansi | 8 + libs/sqf/tests/lints/s14_unused/source.sqf | 2 + libs/sqf/tests/lints/s14_unused/stdout.ansi | 6 + libs/sqf/tests/lints/s15_shadowed/source.sqf | 4 + libs/sqf/tests/lints/s15_shadowed/stdout.ansi | 6 + .../tests/lints/s16_not_private/source.sqf | 3 + .../tests/lints/s16_not_private/stdout.ansi | 6 + 28 files changed, 1094 insertions(+), 457 deletions(-) delete mode 100644 libs/sqf/src/analyze/lints/s12_inspector.rs create mode 100644 libs/sqf/src/analyze/lints/s12_invalid_args.rs create mode 100644 libs/sqf/src/analyze/lints/s13_undefined.rs create mode 100644 libs/sqf/src/analyze/lints/s14_unused.rs create mode 100644 libs/sqf/src/analyze/lints/s15_shadowed.rs create mode 100644 libs/sqf/src/analyze/lints/s16_not_private.rs delete mode 100644 libs/sqf/tests/lints/s12_inspector/source.sqf delete mode 100644 libs/sqf/tests/lints/s12_inspector/stdout.ansi create mode 100644 libs/sqf/tests/lints/s12_invalid_args/source.sqf create mode 100644 libs/sqf/tests/lints/s12_invalid_args/stdout.ansi create mode 100644 libs/sqf/tests/lints/s13_undefined/source.sqf create mode 100644 libs/sqf/tests/lints/s13_undefined/stdout.ansi create mode 100644 libs/sqf/tests/lints/s14_unused/source.sqf create mode 100644 libs/sqf/tests/lints/s14_unused/stdout.ansi create mode 100644 libs/sqf/tests/lints/s15_shadowed/source.sqf create mode 100644 libs/sqf/tests/lints/s15_shadowed/stdout.ansi create mode 100644 libs/sqf/tests/lints/s16_not_private/source.sqf create mode 100644 libs/sqf/tests/lints/s16_not_private/stdout.ansi diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index da6911be..88a443bd 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -1,6 +1,6 @@ use std::{collections::HashSet, ops::Range}; -use crate::{analyze::inspector::VarSource, Expression}; +use crate::{analyze::inspector::VarSource, parser::database::Database, Expression}; use super::{game_value::GameValue, SciptScope}; @@ -97,7 +97,11 @@ impl SciptScope { HashSet::from([GameValue::Boolean(None)]) } #[must_use] - pub fn cmd_generic_call(&mut self, rhs: &HashSet) -> HashSet { + pub fn cmd_generic_call( + &mut self, + rhs: &HashSet, + database: &Database, + ) -> HashSet { for possible in rhs { let GameValue::Code(Some(expression)) = possible else { continue; @@ -110,7 +114,7 @@ impl SciptScope { } self.push(); self.code_used.insert(expression.clone()); - self.eval_statements(statements); + self.eval_statements(statements, database); self.pop(); } HashSet::from([GameValue::Anything]) @@ -120,6 +124,7 @@ impl SciptScope { &mut self, lhs: &HashSet, rhs: &HashSet, + database: &Database, ) -> HashSet { for possible in rhs { let GameValue::Code(Some(expression)) = possible else { @@ -151,7 +156,7 @@ impl SciptScope { } Expression::Code(stage_statement) => { self.code_used.insert(stage.clone()); - self.eval_statements(stage_statement); + self.eval_statements(stage_statement, database); } _ => {} } @@ -162,7 +167,7 @@ impl SciptScope { } self.code_used.insert(expression.clone()); if do_run { - self.eval_statements(statements); + self.eval_statements(statements, database); } self.pop(); } @@ -172,8 +177,9 @@ impl SciptScope { pub fn cmd_generic_call_magic( &mut self, code_possibilities: &HashSet, - magic: &Vec, + magic: &Vec<&str>, source: &Range, + database: &Database, ) -> HashSet { for possible in code_possibilities { let GameValue::Code(Some(expression)) = possible else { @@ -195,7 +201,7 @@ impl SciptScope { ); } self.code_used.insert(expression.clone()); - self.eval_statements(statements); + self.eval_statements(statements, database); self.pop(); } HashSet::from([GameValue::Anything]) @@ -247,7 +253,11 @@ impl SciptScope { lhs.clone() } #[must_use] - pub fn cmd_u_is_nil(&mut self, rhs: &HashSet) -> HashSet { + pub fn cmd_u_is_nil( + &mut self, + rhs: &HashSet, + database: &Database, + ) -> HashSet { let mut non_string = false; for possible in rhs { let GameValue::String(possible) = possible else { @@ -263,7 +273,7 @@ impl SciptScope { let _ = self.var_retrieve(var, &expression.span(), true); } if non_string { - let _ = self.cmd_generic_call(rhs); + let _ = self.cmd_generic_call(rhs, database); } HashSet::from([GameValue::Boolean(None)]) } @@ -272,19 +282,21 @@ impl SciptScope { &mut self, _lhs: &HashSet, rhs: &HashSet, + database: &Database, ) -> HashSet { let mut return_value = HashSet::new(); for possible in rhs { if let GameValue::Code(Some(Expression::Code(_statements))) = possible { - return_value.extend(self.cmd_generic_call(rhs)); + return_value.extend(self.cmd_generic_call(rhs, database)); } if let GameValue::Array(Some(gv_array)) = possible { for gv_index in gv_array { for element in gv_index { if let GameValue::Code(Some(expression)) = element { - return_value.extend(self.cmd_generic_call(&HashSet::from([ - GameValue::Code(Some(expression.clone())), - ]))); + return_value.extend(self.cmd_generic_call( + &HashSet::from([GameValue::Code(Some(expression.clone()))]), + database, + )); } } } @@ -308,7 +320,11 @@ impl SciptScope { return_value } #[must_use] - pub fn cmd_b_get_or_default_call(&mut self, rhs: &HashSet) -> HashSet { + pub fn cmd_b_get_or_default_call( + &mut self, + rhs: &HashSet, + database: &Database, + ) -> HashSet { let mut possible_code = HashSet::new(); for possible_outer in rhs { let GameValue::Array(Some(gv_array)) = possible_outer else { @@ -319,7 +335,7 @@ impl SciptScope { } possible_code.extend(gv_array[1].clone()); } - let _ = self.cmd_generic_call(&possible_code); + let _ = self.cmd_generic_call(&possible_code, database); HashSet::from([GameValue::Anything]) } #[must_use] @@ -336,4 +352,33 @@ impl SciptScope { } HashSet::from([GameValue::String(None)]) } + #[must_use] + pub fn cmd_b_select( + &mut self, + lhs: &HashSet, + rhs: &HashSet, + cmd_set: &HashSet, + source: &Range, + database: &Database, + ) -> HashSet { + let mut return_value = cmd_set.clone(); + // Check: `array select expression` + let _ = self.cmd_generic_call_magic(rhs, &vec!["_x"], source, database); + // if lhs is array, and rhs is bool/number then put array into return + if lhs.len() == 1 + && rhs + .iter() + .any(|r| matches!(r, GameValue::Boolean(..)) || matches!(r, GameValue::Number(..))) + { + if let Some(GameValue::Array(Some(gv_array))) = lhs.iter().next() { + // return_value.clear(); // todo: could clear if we handle pushBack + for gv_index in gv_array { + for element in gv_index { + return_value.insert(element.clone()); + } + } + } + } + return_value + } } diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs index 1f0e9250..e7d6f1cb 100644 --- a/libs/sqf/src/analyze/inspector/external_functions.rs +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -1,76 +1,128 @@ use std::collections::HashSet; -use crate::{analyze::inspector::VarSource, Expression}; +use crate::{analyze::inspector::VarSource, parser::database::Database, Expression}; use super::{game_value::GameValue, SciptScope}; impl SciptScope { - pub fn external_function(&mut self, lhs: &HashSet, rhs: &Expression) { + #[allow(clippy::too_many_lines)] + pub fn external_function( + &mut self, + lhs: &HashSet, + rhs: &Expression, + database: &Database, + ) { let Expression::Variable(ext_func, _) = rhs else { return; }; for possible in lhs { - let GameValue::Array(Some(gv_array)) = possible else { - continue; - }; - match ext_func.to_ascii_lowercase().as_str() { - "cba_fnc_hasheachpair" | "cba_fnc_hashfilter" => { - if gv_array.len() > 1 { - self.external_current_scope( - &gv_array[1], - &vec![ - ("_key", GameValue::Anything), - ("_value", GameValue::Anything), - ], - ); - } - } - "cba_fnc_filter" => { - if gv_array.len() > 1 { + match possible { + GameValue::Code(Some(statements)) => { + // handle `{} call cba_fnc_directcall` + if ext_func.to_ascii_lowercase().as_str() == "cba_fnc_directcall" { self.external_current_scope( - &gv_array[1], - &vec![("_x", GameValue::Anything)], + &vec![GameValue::Code(Some(statements.clone()))], + &vec![], + database, ); } } - "cba_fnc_directcall" => { - if !gv_array.is_empty() { - self.external_current_scope(&gv_array[0], &vec![]); + GameValue::Array(Some(gv_array)) => match ext_func.to_ascii_lowercase().as_str() { + // Functions that will run in existing scope + "cba_fnc_hasheachpair" | "cba_fnc_hashfilter" => { + if gv_array.len() > 1 { + self.external_current_scope( + &gv_array[1], + &vec![ + ("_key", GameValue::Anything), + ("_value", GameValue::Anything), + ], + database, + ); + } } - } - "ace_common_fnc_cachedcall" => { - if gv_array.len() > 1 { - self.external_current_scope(&gv_array[1], &vec![]); + "cba_fnc_filter" => { + if gv_array.len() > 1 { + self.external_current_scope( + &gv_array[1], + &vec![("_x", GameValue::Anything)], + database, + ); + } } - } - "ace_interact_menu_fnc_createaction" => { - for index in 3..5 { - if gv_array.len() > index { - self.external_new_scope( - &gv_array[index], + "cba_fnc_inject" => { + if gv_array.len() > 2 { + self.external_current_scope( + &gv_array[2], &vec![ - ("_target", GameValue::Object), - ("_player", GameValue::Object), + ("_x", GameValue::Anything), + ("_accumulator", GameValue::Anything), ], + database, ); } } - } - "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" => { - if !gv_array.is_empty() { - self.external_new_scope(&gv_array[0], &vec![]); + "cba_fnc_directcall" => { + if !gv_array.is_empty() { + self.external_current_scope(&gv_array[0], &vec![], database); + } } - } - "cba_fnc_addclasseventhandler" => { - if gv_array.len() > 2 { - self.external_new_scope(&gv_array[2], &vec![]); + "ace_common_fnc_cachedcall" => { + if gv_array.len() > 1 { + self.external_current_scope(&gv_array[1], &vec![], database); + } } - } + // Functions that will start in a new scope + "ace_interact_menu_fnc_createaction" => { + for index in 3..=5 { + if gv_array.len() > index { + self.external_new_scope( + &gv_array[index], + &vec![ + ("_target", GameValue::Object), + ("_player", GameValue::Object), + ], + database, + ); + } + } + } + "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" => { + if !gv_array.is_empty() { + self.external_new_scope(&gv_array[0], &vec![], database); + } + } + "cba_fnc_addclasseventhandler" => { + if gv_array.len() > 2 { + self.external_new_scope(&gv_array[2], &vec![], database); + } + } + "cba_fnc_addbiseventhandler" => { + if gv_array.len() > 2 { + self.external_new_scope( + &gv_array[2], + &vec![ + ("_thisType", GameValue::String(None)), + ("_thisId", GameValue::Number(None)), + ("_thisFnc", GameValue::Code(None)), + ("_thisArgs", GameValue::Anything), + ], + database, + ); + } + } + _ => {} + }, _ => {} } } } - fn external_new_scope(&mut self, code_arg: &Vec, vars: &Vec<(&str, GameValue)>) { + fn external_new_scope( + &mut self, + code_arg: &Vec, + vars: &Vec<(&str, GameValue)>, + database: &Database, + ) { for element in code_arg { let GameValue::Code(Some(expression)) = element else { continue; @@ -81,17 +133,22 @@ impl SciptScope { if self.code_used.contains(expression) { return; } - let mut ext_scope = Self::create(&self.ignored_vars, &self.database, true); + let mut ext_scope = Self::create(&self.ignored_vars, true); for (var, value) in vars { ext_scope.var_assign(var, true, HashSet::from([value.clone()]), VarSource::Ignore); } self.code_used.insert(expression.clone()); - ext_scope.eval_statements(statements); - self.errors.extend(ext_scope.finish(false)); + ext_scope.eval_statements(statements, database); + self.errors.extend(ext_scope.finish(false, database)); } } - fn external_current_scope(&mut self, code_arg: &Vec, vars: &Vec<(&str, GameValue)>) { + fn external_current_scope( + &mut self, + code_arg: &Vec, + vars: &Vec<(&str, GameValue)>, + database: &Database, + ) { for element in code_arg { let GameValue::Code(Some(expression)) = element else { continue; @@ -107,7 +164,7 @@ impl SciptScope { self.var_assign(var, true, HashSet::from([value.clone()]), VarSource::Ignore); } self.code_used.insert(expression.clone()); - self.eval_statements(statements); + self.eval_statements(statements, database); self.pop(); } } diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs index b24a49d6..080aae78 100644 --- a/libs/sqf/src/analyze/inspector/game_value.rs +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -1,4 +1,4 @@ -use std::{collections::HashSet, sync::Arc}; +use std::collections::HashSet; use arma3_wiki::model::{Arg, Call, Param, Value}; use tracing::{trace, warn}; @@ -37,16 +37,17 @@ pub enum GameValue { impl GameValue { #[must_use] + /// Gets cmd return types based on input types pub fn from_cmd( expression: &Expression, lhs_set: Option<&HashSet>, rhs_set: Option<&HashSet>, - database: &Arc, + database: &Database, ) -> HashSet { let mut return_types = HashSet::new(); let cmd_name = expression.command_name().expect("has a name"); let Some(command) = database.wiki().commands().get(cmd_name) else { - trace!("cmd {cmd_name} not in db?"); //ToDo: this can't find short cmds like &&, || + println!("cmd {cmd_name} not in db?"); return HashSet::from([Self::Anything]); }; @@ -124,6 +125,7 @@ impl GameValue { "setGroupIdGlobal", "set3DENMissionAttributes", "setPiPEffect", + "ppEffectCreate", ]; if !WIKI_CMDS_IGNORE_MISSING_PARAM.contains(&cmd_name) { warn!("cmd {cmd_name} - param {name} not found"); @@ -135,11 +137,7 @@ impl GameValue { // param.typ(), // param.optional() // ); - if param.optional() { - // todo: maybe still check type if opt and not empty/nil? - return true; - } - Self::match_set_to_value(set, param.typ()) + Self::match_set_to_value(set, param.typ(), param.optional()) } Arg::Array(arg_array) => { const WIKI_CMDS_IGNORE_ARGS: &[&str] = &["createHashMapFromArray"]; @@ -180,8 +178,11 @@ impl GameValue { } #[must_use] - pub fn match_set_to_value(set: &HashSet, right_wiki: &Value) -> bool { - // println!("Checking {:?} against {:?}", set, right_wiki); + pub fn match_set_to_value(set: &HashSet, right_wiki: &Value, optional: bool) -> bool { + // println!("Checking {:?} against {:?} [O:{optional}]", set, right_wiki); + if optional && (set.is_empty() || set.contains(&Self::Nothing)) { + return true; + } let right = Self::from_wiki_value(right_wiki); set.iter().any(|gv| Self::match_values(gv, &right)) } @@ -203,7 +204,7 @@ impl GameValue { /// Maps from Wiki:Value to Inspector:GameValue pub fn from_wiki_value(value: &Value) -> Self { match value { - Value::Anything => Self::Anything, + Value::Anything | Value::EdenEntity => Self::Anything, Value::ArrayColor | Value::ArrayColorRgb | Value::ArrayColorRgba diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 68aa79d7..1387e682 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -4,7 +4,6 @@ use std::{ collections::{HashMap, HashSet}, hash::Hash, ops::Range, - sync::Arc, vec, }; @@ -66,32 +65,26 @@ pub struct VarHolder { pub type Stack = HashMap; pub struct SciptScope { - database: Arc, errors: HashSet, global: Stack, local: Vec, code_seen: HashSet, code_used: HashSet, - is_child: bool, + is_orphan_scope: bool, ignored_vars: HashSet, } impl SciptScope { #[must_use] - pub fn create( - ignored_vars: &HashSet, - database: &Arc, - is_child: bool, - ) -> Self { + pub fn create(ignored_vars: &HashSet, is_orphan_scope: bool) -> Self { // trace!("Creating ScriptScope"); let mut scope = Self { - database: database.clone(), errors: HashSet::new(), global: Stack::new(), local: Vec::new(), code_seen: HashSet::new(), code_used: HashSet::new(), - is_child, + is_orphan_scope, ignored_vars: ignored_vars.clone(), }; scope.push(); @@ -106,7 +99,7 @@ impl SciptScope { scope } #[must_use] - pub fn finish(&mut self, check_child_scripts: bool) -> HashSet { + pub fn finish(&mut self, check_child_scripts: bool, database: &Database) -> HashSet { self.pop(); if check_child_scripts { let unused = &self.code_seen - &self.code_used; @@ -116,10 +109,10 @@ impl SciptScope { continue; }; // trace!("-- Checking external scope"); - let mut external_scope = Self::create(&self.ignored_vars, &self.database, true); - external_scope.eval_statements(&statements); + let mut external_scope = Self::create(&self.ignored_vars, true); + external_scope.eval_statements(&statements, database); self.errors - .extend(external_scope.finish(check_child_scripts)); + .extend(external_scope.finish(check_child_scripts, database)); } } self.errors.clone() @@ -133,10 +126,7 @@ impl SciptScope { for (var, holder) in self.local.pop().unwrap_or_default() { // trace!("-- Stack Pop {}:{} ", var, holder.usage); if holder.usage == 0 && !holder.source.skip_errors() { - self.errors.insert(Issue::Unused( - var, - holder.source, - )); + self.errors.insert(Issue::Unused(var, holder.source)); } } } @@ -215,7 +205,7 @@ impl SciptScope { self.errors.insert(Issue::Undefined( var.to_owned(), source.clone(), - self.is_child, + self.is_orphan_scope, )); } } else { @@ -247,7 +237,11 @@ impl SciptScope { #[must_use] #[allow(clippy::too_many_lines)] /// Evaluate expression in current scope - pub fn eval_expression(&mut self, expression: &Expression) -> HashSet { + pub fn eval_expression( + &mut self, + expression: &Expression, + database: &Database, + ) -> HashSet { let mut debug_type = String::new(); let possible_values = match expression { Expression::Variable(var, source) => self.var_retrieve(var, source, false), @@ -259,13 +253,13 @@ impl SciptScope { Expression::Array(array, _) => { let gv_array: Vec> = array .iter() - .map(|e| self.eval_expression(e).into_iter().collect()) + .map(|e| self.eval_expression(e, database).into_iter().collect()) .collect(); HashSet::from([GameValue::Array(Some(gv_array))]) } Expression::NularCommand(cmd, source) => { debug_type = format!("[N:{}]", cmd.as_str()); - let cmd_set = GameValue::from_cmd(expression, None, None, &self.database); + let cmd_set = GameValue::from_cmd(expression, None, None, database); if cmd_set.is_empty() { // is this possible? self.errors @@ -275,8 +269,8 @@ impl SciptScope { } Expression::UnaryCommand(cmd, rhs, source) => { debug_type = format!("[U:{}]", cmd.as_str()); - let rhs_set = self.eval_expression(rhs); - let cmd_set = GameValue::from_cmd(expression, None, Some(&rhs_set), &self.database); + let rhs_set = self.eval_expression(rhs, database); + let cmd_set = GameValue::from_cmd(expression, None, Some(&rhs_set), database); if cmd_set.is_empty() { self.errors .insert(Issue::InvalidArgs(debug_type.clone(), source.clone())); @@ -285,10 +279,10 @@ impl SciptScope { UnaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { "params" => Some(self.cmd_generic_params(&rhs_set)), "private" => Some(self.cmd_u_private(&rhs_set)), - "call" => Some(self.cmd_generic_call(&rhs_set)), - "isnil" => Some(self.cmd_u_is_nil(&rhs_set)), + "call" => Some(self.cmd_generic_call(&rhs_set, database)), + "isnil" => Some(self.cmd_u_is_nil(&rhs_set, database)), "while" | "waituntil" | "default" => { - let _ = self.cmd_generic_call(&rhs_set); + let _ = self.cmd_generic_call(&rhs_set, database); None } "for" => Some(self.cmd_for(&rhs_set)), @@ -302,10 +296,10 @@ impl SciptScope { } Expression::BinaryCommand(cmd, lhs, rhs, source) => { debug_type = format!("[B:{}]", cmd.as_str()); - let lhs_set = self.eval_expression(lhs); - let rhs_set = self.eval_expression(rhs); + let lhs_set = self.eval_expression(lhs, database); + let rhs_set = self.eval_expression(rhs, database); let cmd_set = - GameValue::from_cmd(expression, Some(&lhs_set), Some(&rhs_set), &self.database); + GameValue::from_cmd(expression, Some(&lhs_set), Some(&rhs_set), database); if cmd_set.is_empty() { // we must have invalid args self.errors @@ -314,57 +308,60 @@ impl SciptScope { let return_set = match cmd { BinaryCommand::Associate => { // the : from case - let _ = self.cmd_generic_call(&rhs_set); + let _ = self.cmd_generic_call(&rhs_set, database); None } BinaryCommand::And | BinaryCommand::Or => { - let _ = self.cmd_generic_call(&rhs_set); + let _ = self.cmd_generic_call(&rhs_set, database); None } BinaryCommand::Else => Some(self.cmd_b_else(&lhs_set, &rhs_set)), BinaryCommand::Named(named) => match named.to_ascii_lowercase().as_str() { "params" => Some(self.cmd_generic_params(&rhs_set)), "call" => { - self.external_function(&lhs_set, rhs); - Some(self.cmd_generic_call(&rhs_set)) + self.external_function(&lhs_set, rhs, database); + Some(self.cmd_generic_call(&rhs_set, database)) } "exitwith" => { // todo: handle scope exits - Some(self.cmd_generic_call(&rhs_set)) + Some(self.cmd_generic_call(&rhs_set, database)) } "do" => { // from While, With, For, and Switch - Some(self.cmd_b_do(&lhs_set, &rhs_set)) + Some(self.cmd_b_do(&lhs_set, &rhs_set, database)) } "from" | "to" | "step" => Some(self.cmd_b_from_chain(&lhs_set, &rhs_set)), - "then" => Some(self.cmd_b_then(&lhs_set, &rhs_set)), + "then" => Some(self.cmd_b_then(&lhs_set, &rhs_set, database)), "foreach" | "foreachreversed" => Some(self.cmd_generic_call_magic( &lhs_set, - &vec![ - "_x".to_string(), - "_y".to_string(), - "_forEachIndex".to_string(), - ], + &vec!["_x", "_y", "_forEachIndex"], source, + database, )), "count" => { let _ = self.cmd_generic_call_magic( &lhs_set, - &vec!["_x".to_string()], + &vec!["_x"], source, + database, ); None } - "findif" | "apply" | "select" => { - //todo handle (array select number) or (string select [1,2]); + "findif" | "apply" => { let _ = self.cmd_generic_call_magic( &rhs_set, - &vec!["_x".to_string()], + &vec!["_x"], source, + database, ); None } - "getordefaultcall" => Some(self.cmd_b_get_or_default_call(&rhs_set)), + "getordefaultcall" => { + Some(self.cmd_b_get_or_default_call(&rhs_set, database)) + } + "select" => { + Some(self.cmd_b_select(&lhs_set, &rhs_set, &cmd_set, source, database)) + } _ => None, }, _ => None, @@ -391,13 +388,13 @@ impl SciptScope { } /// Evaluate statements in the current scope - fn eval_statements(&mut self, statements: &Statements) { + fn eval_statements(&mut self, statements: &Statements, database: &Database) { // let mut return_value = HashSet::new(); for statement in statements.content() { match statement { Statement::AssignGlobal(var, expression, source) => { // x or _x - let possible_values = self.eval_expression(expression); + let possible_values = self.eval_expression(expression, database); self.var_assign( var, false, @@ -408,7 +405,7 @@ impl SciptScope { } Statement::AssignLocal(var, expression, source) => { // private _x - let possible_values = self.eval_expression(expression); + let possible_values = self.eval_expression(expression, database); self.var_assign( var, true, @@ -418,7 +415,7 @@ impl SciptScope { // return_value = vec![GameValue::Assignment()]; } Statement::Expression(expression, _) => { - let _possible_values = self.eval_expression(expression); + let _possible_values = self.eval_expression(expression, database); // return_value = possible_values; } } @@ -432,8 +429,7 @@ impl SciptScope { pub fn run_processed( statements: &Statements, processed: &Processed, - database: &Arc, - check_child_scripts: bool, + database: &Database, ) -> Vec { let mut ignored_vars = HashSet::new(); ignored_vars.insert("_this".to_string()); @@ -451,7 +447,7 @@ pub fn run_processed( } } - let mut scope = SciptScope::create(&ignored_vars, database, false); - scope.eval_statements(statements); - scope.finish(check_child_scripts).into_iter().collect() + let mut scope = SciptScope::create(&ignored_vars, false); + scope.eval_statements(statements, database); + scope.finish(true, database).into_iter().collect() } diff --git a/libs/sqf/src/analyze/lints/s12_inspector.rs b/libs/sqf/src/analyze/lints/s12_inspector.rs deleted file mode 100644 index dde7772a..00000000 --- a/libs/sqf/src/analyze/lints/s12_inspector.rs +++ /dev/null @@ -1,253 +0,0 @@ -use crate::{ - analyze::{ - inspector::{self, Issue, VarSource}, - SqfLintData, - }, - 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}; -use tracing::trace; - -crate::lint!(LintS12Inspector); - -impl Lint for LintS12Inspector { - fn ident(&self) -> &str { - "inspector" - } - - fn sort(&self) -> u32 { - 120 - } - - fn description(&self) -> &str { - "Checks for code usage" - } - - fn documentation(&self) -> &str { -r"### Configuration -- **check_invalid_args**: [default: true] check_invalid_args (e.g. `x setFuel true`) -- **check_child_scripts**: [default: false] Checks oprhaned scripts. - Assumes un-called code will run in another scope (can cause false positives) - e.g. `private _var = 5; [{_var}] call CBA_fnc_addPerFrameEventHandler;` -- **check_undefined**: [default: true] Checks local vars that are not defined -- **check_not_private**: [default: true] Checks local vars that are not `private` -- **check_unused**: [default: false] Checks local vars that are never used -- **check_shadow**: [default: false] Checks local vars that are shaddowed - -```toml -[lints.sqf.inspector] -options.check_invalid_args = true -options.check_child_scripts = true -options.check_undefined = true -options.check_not_private = true -options.check_unused = true -options.check_shadow = true -```" - } - - fn default_config(&self) -> LintConfig { - LintConfig::help() - } - - fn runners(&self) -> Vec>> { - vec![Box::new(Runner)] - } -} - -pub struct Runner; -impl LintRunner for Runner { - type Target = Statements; - #[allow(clippy::too_many_lines)] - fn run( - &self, - _project: Option<&hemtt_common::config::ProjectConfig>, - config: &hemtt_common::config::LintConfig, - processed: Option<&hemtt_workspace::reporting::Processed>, - target: &Statements, - data: &SqfLintData, - ) -> hemtt_workspace::reporting::Codes { - let Some(processed) = processed else { - return Vec::new(); - }; - if !target.top_level { - // we only want to handle full files, not all the sub-statements - return Vec::new(); - }; - let (_addon, database) = data; - - let check_invalid_args = - if let Some(toml::Value::Boolean(b)) = config.option("check_invalid_args") { - *b - } else { - true - }; - let check_child_scripts = - if let Some(toml::Value::Boolean(b)) = config.option("check_child_scripts") { - *b - } else { - true // can cause false positives - }; - let check_undefined = - if let Some(toml::Value::Boolean(b)) = config.option("check_undefined") { - *b - } else { - true - }; - let check_not_private = if let Some(toml::Value::Boolean(b)) = - config.option("check_not_private") - { - *b - } else { - true - }; - let check_unused = - if let Some(toml::Value::Boolean(b)) = config.option("check_unused") { - *b - } else { - true - }; - let check_shadow = - if let Some(toml::Value::Boolean(b)) = config.option("check_shadow") { - *b - } else { - true - }; - - let mut errors: Codes = Vec::new(); - let issues = inspector::run_processed(target, processed, database, check_child_scripts); - trace!("issues {}", issues.len()); - - for issue in issues { - match issue { - Issue::InvalidArgs(cmd, range) => { - if check_invalid_args { - errors.push(Arc::new(CodeS12Inspector::new( - range, - format!("Bad Args: {cmd}"), - None, - config.severity(), - processed, - ))); - } - } - Issue::Undefined(var, range, is_child) => { - if check_undefined { - let error_hint = if is_child {Some("From Child Code - may not be a problem".to_owned())} else {None}; - errors.push(Arc::new(CodeS12Inspector::new( - range, - format!("Undefined: {var}"), - error_hint, - config.severity(), - processed, - ))); - } - } - Issue::NotPrivate(var, range) => { - if check_not_private { - errors.push(Arc::new(CodeS12Inspector::new( - range, - format!("NotPrivate: {var}"), - None, - config.severity(), - processed, - ))); - } - } - Issue::Unused(var, source) => { - let VarSource::Assignment(range) = source else { continue }; - if check_unused { - errors.push(Arc::new(CodeS12Inspector::new( - range, - format!("Unused: {var}"), - None, - config.severity(), - processed, - ))); - } - } - Issue::Shadowed(var, range) => { - if check_shadow { - errors.push(Arc::new(CodeS12Inspector::new( - range, - format!("Shadowed: {var}"), - None, - config.severity(), - processed, - ))); - } - } - }; - } - - errors - } -} - -#[allow(clippy::module_name_repetitions)] -pub struct CodeS12Inspector { - span: Range, - error_type: String, - error_hint: Option, - severity: Severity, - diagnostic: Option, -} - -impl Code for CodeS12Inspector { - fn ident(&self) -> &'static str { - "L-S12" - } - - fn link(&self) -> Option<&str> { - Some("/analysis/sqf.html#inspector") - } - /// Top message - fn message(&self) -> String { - format!("Inspector - {}", self.error_type) - } - /// Under ^^^span hint - fn label_message(&self) -> String { - String::new() - } - /// bottom note - fn note(&self) -> Option { - self.error_hint.clone() - } - - fn severity(&self) -> Severity { - self.severity - } - - fn diagnostic(&self) -> Option { - self.diagnostic.clone() - } -} - -impl CodeS12Inspector { - #[must_use] - pub fn new( - span: Range, - error_type: String, - error_hint: Option, - severity: Severity, - processed: &Processed, - ) -> Self { - Self { - span, - error_type, - error_hint, - severity, - diagnostic: None, - } - .generate_processed(processed) - } - - fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); - self - } -} diff --git a/libs/sqf/src/analyze/lints/s12_invalid_args.rs b/libs/sqf/src/analyze/lints/s12_invalid_args.rs new file mode 100644 index 00000000..a535a18a --- /dev/null +++ b/libs/sqf/src/analyze/lints/s12_invalid_args.rs @@ -0,0 +1,135 @@ +use crate::{ + analyze::{inspector::Issue, SqfLintData}, + 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::lint!(LintS12InvalidArgs); + +impl Lint for LintS12InvalidArgs { + fn ident(&self) -> &str { + "invalid_args" + } + fn sort(&self) -> u32 { + 120 + } + fn description(&self) -> &str { + "Invalid Args" + } + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +(vehicle player) setFuel true; // bad args: takes number 0-1 +``` + +### Explanation + +Checks correct syntax usage." + } + fn default_config(&self) -> LintConfig { + LintConfig::help() + } + 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: &SqfLintData, + ) -> 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::InvalidArgs(cmd, range) = issue { + errors.push(Arc::new(CodeS12InvalidArgs::new( + range.to_owned(), + cmd.to_owned(), + None, + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS12InvalidArgs { + span: Range, + token_name: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS12InvalidArgs { + fn ident(&self) -> &'static str { + "L-S12" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#invalid_args") + } + /// Top message + fn message(&self) -> String { + format!("Invalid Args - {}", self.token_name) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS12InvalidArgs { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + token_name: error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/analyze/lints/s13_undefined.rs b/libs/sqf/src/analyze/lints/s13_undefined.rs new file mode 100644 index 00000000..01ac0a12 --- /dev/null +++ b/libs/sqf/src/analyze/lints/s13_undefined.rs @@ -0,0 +1,149 @@ +use crate::{ + analyze::{inspector::Issue, SqfLintData}, + 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::lint!(LintS13Undefined); + +impl Lint for LintS13Undefined { + fn ident(&self) -> &str { + "undefined" + } + fn sort(&self) -> u32 { + 130 + } + fn description(&self) -> &str { + "Undefined Variable" + } + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +systemChat _neverDefined; +``` + +### Explanation + +Checks correct syntax usage." + } + fn default_config(&self) -> LintConfig { + LintConfig::help() + } + 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: &SqfLintData, + ) -> hemtt_workspace::reporting::Codes { + if target.issues().is_empty() { + return Vec::new(); + }; + let Some(processed) = processed else { + return Vec::new(); + }; + let check_oprhan_code = + if let Some(toml::Value::Boolean(b)) = config.option("check_oprhan_code") { + *b + } else { + false + }; + let mut errors: Codes = Vec::new(); + for issue in target.issues() { + if let Issue::Undefined(var, range, is_orphan_scope) = issue { + let error_hint = if *is_orphan_scope { + if !check_oprhan_code { + continue; + } + Some("From Orphan Code - may not be a problem".to_owned()) + } else { + None + }; + errors.push(Arc::new(CodeS13Undefined::new( + range.to_owned(), + var.to_owned(), + error_hint, + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS13Undefined { + span: Range, + token_name: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS13Undefined { + fn ident(&self) -> &'static str { + "L-S13" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#undefined") + } + /// Top message + fn message(&self) -> String { + format!("Undefined Var - {}", self.token_name) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS13Undefined { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + token_name: error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/analyze/lints/s14_unused.rs b/libs/sqf/src/analyze/lints/s14_unused.rs new file mode 100644 index 00000000..a35e3ae7 --- /dev/null +++ b/libs/sqf/src/analyze/lints/s14_unused.rs @@ -0,0 +1,155 @@ +use crate::{ + analyze::{ + inspector::{Issue, VarSource}, + SqfLintData, + }, + 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::lint!(LintS14Unused); + +impl Lint for LintS14Unused { + fn ident(&self) -> &str { + "unused" + } + fn sort(&self) -> u32 { + 120 + } + fn description(&self) -> &str { + "Unused Var" + } + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +private _z = 5; // and never used +``` + +### Explanation + +Checks for vars that are never used." + } + fn default_config(&self) -> LintConfig { + LintConfig::help().with_enabled(false) + } + 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: &SqfLintData, + ) -> hemtt_workspace::reporting::Codes { + if target.issues().is_empty() { + return Vec::new(); + }; + let Some(processed) = processed else { + return Vec::new(); + }; + + let check_params = if let Some(toml::Value::Boolean(b)) = config.option("check_params") { + *b + } else { + false + }; + let mut errors: Codes = Vec::new(); + for issue in target.issues() { + if let Issue::Unused(var, source) = issue { + match source { + VarSource::Assignment(_) => {} + VarSource::Params(_) => { + if !check_params { + continue; + }; + } + _ => { + continue; + } + } + errors.push(Arc::new(CodeS14Unused::new( + source.get_range().unwrap_or_default(), + var.to_owned(), + None, + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS14Unused { + span: Range, + token_name: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS14Unused { + fn ident(&self) -> &'static str { + "L-S14" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#unused") + } + /// Top message + fn message(&self) -> String { + format!("Unused Var - {}", self.token_name) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS14Unused { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + token_name: error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/analyze/lints/s15_shadowed.rs b/libs/sqf/src/analyze/lints/s15_shadowed.rs new file mode 100644 index 00000000..38264954 --- /dev/null +++ b/libs/sqf/src/analyze/lints/s15_shadowed.rs @@ -0,0 +1,136 @@ +use crate::{ + analyze::{inspector::Issue, SqfLintData}, + 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::lint!(LintS15Shadowed); + +impl Lint for LintS15Shadowed { + fn ident(&self) -> &str { + "shadowed" + } + fn sort(&self) -> u32 { + 150 + } + fn description(&self) -> &str { + "Shadowed Var" + } + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +private _z = 5; +private _z = 5; +``` + +### Explanation + +Checks for variables being shadowed." + } + fn default_config(&self) -> LintConfig { + LintConfig::help().with_enabled(false) + } + 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: &SqfLintData, + ) -> 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::Shadowed(var, range) = issue { + errors.push(Arc::new(CodeS15Shadowed::new( + range.to_owned(), + var.to_owned(), + None, + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS15Shadowed { + span: Range, + token_name: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS15Shadowed { + fn ident(&self) -> &'static str { + "L-S15" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#shadowed") + } + /// Top message + fn message(&self) -> String { + format!("Shadowed Var - {}", self.token_name) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS15Shadowed { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + token_name: error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/analyze/lints/s16_not_private.rs b/libs/sqf/src/analyze/lints/s16_not_private.rs new file mode 100644 index 00000000..a938b28f --- /dev/null +++ b/libs/sqf/src/analyze/lints/s16_not_private.rs @@ -0,0 +1,135 @@ +use crate::{ + analyze::{inspector::Issue, SqfLintData}, + 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::lint!(LintS16NotPrivate); + +impl Lint for LintS16NotPrivate { + fn ident(&self) -> &str { + "not_private" + } + fn sort(&self) -> u32 { + 160 + } + fn description(&self) -> &str { + "Not Private Var" + } + fn documentation(&self) -> &str { + r"### Example + +**Incorrect** +```sqf +_z = 6; +``` + +### Explanation + +Checks local variables that are not private." + } + fn default_config(&self) -> LintConfig { + LintConfig::help().with_enabled(false) + } + 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: &SqfLintData, + ) -> 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::NotPrivate(var, range) = issue { + errors.push(Arc::new(CodeS16NotPrivate::new( + range.to_owned(), + var.to_owned(), + None, + config.severity(), + processed, + ))); + } + } + errors + } +} + +#[allow(clippy::module_name_repetitions)] +pub struct CodeS16NotPrivate { + span: Range, + token_name: String, + error_hint: Option, + severity: Severity, + diagnostic: Option, +} + +impl Code for CodeS16NotPrivate { + fn ident(&self) -> &'static str { + "L-S16" + } + fn link(&self) -> Option<&str> { + Some("/analysis/sqf.html#not_private") + } + /// Top message + fn message(&self) -> String { + format!("Not Private - {}", self.token_name) + } + /// Under ^^^span hint + fn label_message(&self) -> String { + String::new() + } + /// bottom note + fn note(&self) -> Option { + self.error_hint.clone() + } + fn severity(&self) -> Severity { + self.severity + } + fn diagnostic(&self) -> Option { + self.diagnostic.clone() + } +} + +impl CodeS16NotPrivate { + #[must_use] + pub fn new( + span: Range, + error_type: String, + error_hint: Option, + severity: Severity, + processed: &Processed, + ) -> Self { + Self { + span, + token_name: error_type, + error_hint, + severity, + diagnostic: None, + } + .generate_processed(processed) + } + fn generate_processed(mut self, processed: &Processed) -> Self { + self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self + } +} diff --git a/libs/sqf/src/lib.rs b/libs/sqf/src/lib.rs index 3174a6ba..0d0356b2 100644 --- a/libs/sqf/src/lib.rs +++ b/libs/sqf/src/lib.rs @@ -11,6 +11,7 @@ use std::{ops::Range, sync::Arc}; pub use self::error::Error; +use analyze::inspector::Issue; use arma3_wiki::model::Version; #[doc(no_inline)] pub use float_ord::FloatOrd as Scalar; @@ -23,7 +24,7 @@ pub struct Statements { /// This isn't required to actually be anything significant, but will be displayed in-game if a script error occurs. source: Arc, span: Range, - top_level: bool, + issues: Vec, } impl Statements { @@ -42,6 +43,10 @@ impl Statements { self.span.clone() } + #[must_use] + pub const fn issues(&self) -> &Vec { + &self.issues + } #[must_use] /// Gets the highest version required by any command in this code chunk. pub fn required_version(&self, database: &Database) -> (String, Version, Range) { @@ -105,6 +110,9 @@ impl Statements { } (command, version, span) } + pub fn testing_clear_issues(&mut self) { + self.issues.clear(); + } } #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/libs/sqf/src/parser/mod.rs b/libs/sqf/src/parser/mod.rs index 6634352b..d69d3cd6 100644 --- a/libs/sqf/src/parser/mod.rs +++ b/libs/sqf/src/parser/mod.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use self::database::{is_special_command, Database}; use self::lexer::{Control, Operator, Token}; +use crate::analyze::inspector; use crate::{BinaryCommand, Expression, NularCommand, Statement, Statements, UnaryCommand}; use chumsky::prelude::*; @@ -43,7 +44,7 @@ pub fn run(database: &Database, processed: &Processed) -> Result( source: processed.extract(span.clone()), span, content, - top_level: false, + issues: vec![], }) }) } diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index 5771aa63..2441b17c 100644 --- a/libs/sqf/tests/inspector.rs +++ b/libs/sqf/tests/inspector.rs @@ -1,5 +1,3 @@ -use std::sync::Arc; - use hemtt_sqf::Statements; use hemtt_workspace::reporting::Processed; @@ -9,7 +7,7 @@ use hemtt_sqf::parser::database::Database; use hemtt_workspace::LayerType; const ROOT: &str = "tests/inspector/"; -fn get_statements(file: &str) -> (Processed, Statements, Arc) { +fn get_statements(file: &str) -> (Processed, Statements, Database) { let folder = std::path::PathBuf::from(ROOT); let workspace = hemtt_workspace::Workspace::builder() .physical(&folder, LayerType::Source) @@ -18,27 +16,28 @@ fn get_statements(file: &str) -> (Processed, Statements, Arc) { let source = workspace.join(file).expect("for test"); let processed = Processor::run(&source).expect("for test"); let statements = hemtt_sqf::parser::run(&Database::a3(false), &processed).expect("for test"); - let database = Arc::new(Database::a3(false)); + let database = Database::a3(false); (processed, statements, database) } #[cfg(test)] mod tests { use crate::get_statements; - use hemtt_sqf::analyze::inspector::{self, Issue, VarSource}; + use hemtt_sqf::analyze::inspector::{Issue, VarSource}; #[test] pub fn test_0() { - let (pro, sqf, database) = get_statements("test_0.sqf"); - let result = inspector::run_processed(&sqf, &pro, &database, true); + let (_pro, sqf, _database) = get_statements("test_0.sqf"); + // let result = inspector::run_processed(&sqf, &pro, &database); + let result = sqf.issues(); println!("done: {}, {result:?}", result.len()); } #[test] pub fn test_1() { - let (pro, sqf, database) = get_statements("test_1.sqf"); - let result = inspector::run_processed(&sqf, &pro, &database, true); - assert_eq!(result.len(), 11); + let (_pro, sqf, _database) = get_statements("test_1.sqf"); + let result = sqf.issues(); + assert_eq!(result.len(), 12); // Order not guarenteed assert!(result.iter().any(|i| { if let Issue::InvalidArgs(cmd, _) = i { @@ -117,12 +116,19 @@ mod tests { false } })); + assert!(result.iter().any(|i| { + if let Issue::InvalidArgs(cmd, _) = i { + cmd == "[B:drawIcon]" + } else { + false + } + })); } #[test] pub fn test_2() { - let (pro, sqf, database) = get_statements("test_2.sqf"); - let result = inspector::run_processed(&sqf, &pro, &database, true); + let (_pro, sqf, _database) = get_statements("test_2.sqf"); + let result = sqf.issues(); assert_eq!(result.len(), 0); } } diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index 2d68148c..9a5ad934 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -1,28 +1,31 @@ +// _var[LETTER] are safe +// _test[NUMBER] are errors + a setFuel b; a setFuel 0; a setFuel true; // invalidArgs: takes number 0-1 _test2 setDamage 1; // undefiend -private _var1 = player; -_var1 setDamage 0.5; +private _varA = player; +_varA setDamage 0.5; _test3 = 7; // not private systemChat str _test3; -private "_var2"; -_var2 = 8; -private ["_var3"]; -_var3 = _var2 + 1; -private _test4 = _var3; // unused +private "_varB"; +_varB = 8; +private ["_varC"]; +_varC = _varB + 1; +private _test4 = _varC; // unused params ["_test5"]; private _test5 = 10; // shadow (same level) diag_log text str _test5; gx = []; gx addPublicVariableEventHandler {}; // args: takes lhs string -for "_var5" from 1 to 20 step 0.5 do { - systemChat str _var5; +for "_varE" from 1 to 20 step 0.5 do { + systemChat str _varE; }; -for [{private _var6 = 0}, {_var6 < 5}, {_var6 = _var6 + 1}] do { - systemChat str _var6; +for [{private _varF = 0}, {_varF < 5}, {_varF = _varF + 1}] do { + systemChat str _varF; }; //IGNORE_PRIVATE_WARNING["_fromUpper"]; @@ -31,27 +34,27 @@ X = _fromUpper; [] call { private "_weird"; //IGNORE_PRIVATE_WARNING["_weird"] - // No way to know the order is different - for "_var7" from 1 to 5 do { - if (_var7%2 == 0) then { + for "_varG" from 1 to 5 do { + if (_varG%2 == 0) then { truck lock _weird; }; - if (_var7%2 == 1) then { + if (_varG%2 == 1) then { _weird = 0.5; }; }; }; -// IGNORE_PRIVATE_WARNING["somePFEH"] - // otherwise will assume it's nil +// IGNORE_PRIVATE_WARNING["somePFEH"] if (z) then { - somePFEH = nil; + somePFEH = nil; // otherwise will assume it's always nil }; if (y) then { setObjectViewDistance somePFEH; }; somehash getOrDefaultCall ["key", {_test8}, true]; //undefined -private _var8 = objNull; -somehash getOrDefaultCall ["key", {_var8}, true]; +private _varH = objNull; +somehash getOrDefaultCall ["key", {_varH}, true]; // Will have _player and _target private _condition = { [_player, _target] call y }; @@ -63,7 +66,8 @@ private _condition = { [_player, _target] call y }; x ctrlSetText _player; // bad arg type [_target, _player] call z; }, - _condition + _condition, + {systemChat str _player; []} ] call ace_interact_menu_fnc_createAction; private _hash = [] call CBA_fnc_hashCreate; @@ -79,14 +83,34 @@ _test9= _test9 + 1; systemChat str _test9; // invalid }, 0, []] call CBA_fnc_addPerFrameHandler; -private _var9 = 55; -[{systemChat str _var9}] call CBA_fnc_directCall; +private _varI = 55; +[{systemChat str _varI}] call CBA_fnc_directCall; // Will have _x filter = [orig, {_x + 1}] call CBA_fnc_filter; -private _var10 = 123; -[player, {x = _var10}] call ace_common_fnc_cachedcall; +private _varJ = 123; +[player, {x = _varJ}] call ace_common_fnc_cachedcall; for "_test10" from 1 to 1 step 0.1 do {}; [5] params ["_test11"]; + +params [["_varK", objNull, [objNull]]]; +{ + private _varName = vehicleVarName _varK; + _varK setVehicleVarName (_varName + "ok"); +} call CBA_fnc_directCall; + +_this select 0 drawIcon [ // invalidArgs + "#(rgb,1,1,1)color(1,1,1,1)", + [0,1,0,1], + player, + 0, + 0, + 0, + 5555 // text - optional +]; + +private _varL = nil; +call ([{_varL = 0;}, {_varL = 1;}] select (x == 1)); +["A", "B"] select _varL; diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 2547454a..9fe92871 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -10,17 +10,17 @@ use hemtt_workspace::{addons::Addon, reporting::WorkspaceFiles, LayerType}; const ROOT: &str = "tests/lints/"; macro_rules! analyze { - ($dir:ident) => { + ($dir:ident, $ignore:expr) => { paste::paste! { #[test] fn []() { - test_analyze(stringify!($dir)); + test_analyze(stringify!($dir), $ignore); } } }; } -fn test_analyze(dir: &str) { +fn test_analyze(dir: &str, ignore_inspector: bool) { let folder = std::path::PathBuf::from(ROOT).join(dir); let workspace = hemtt_workspace::Workspace::builder() .physical(&folder, LayerType::Source) @@ -35,7 +35,10 @@ fn test_analyze(dir: &str) { let config = ProjectConfig::from_file(&config_path_full).unwrap(); match hemtt_sqf::parser::run(&database, &processed) { - Ok(sqf) => { + Ok(mut sqf) => { + if ignore_inspector { + sqf.testing_clear_issues(); + } let codes = analyze( &sqf, Some(&config), @@ -73,12 +76,16 @@ fn test_analyze(dir: &str) { }; } -analyze!(s03_static_typename); -analyze!(s04_command_case); -analyze!(s05_if_assign); -analyze!(s06_find_in_str); -analyze!(s07_select_parse_number); -analyze!(s08_format_args); -analyze!(s09_banned_command); -analyze!(s11_if_not_else); -analyze!(s12_inspector); +analyze!(s03_static_typename, true); +analyze!(s04_command_case, true); +analyze!(s05_if_assign, true); +analyze!(s06_find_in_str, true); +analyze!(s07_select_parse_number, true); +analyze!(s08_format_args, true); +analyze!(s09_banned_command, true); +analyze!(s11_if_not_else, true); +analyze!(s12_invalid_args, false); +analyze!(s13_undefined, false); +analyze!(s14_unused, false); +analyze!(s15_shadowed, false); +analyze!(s16_not_private, false); diff --git a/libs/sqf/tests/lints/project_tests.toml b/libs/sqf/tests/lints/project_tests.toml index e18d39f1..22bc5f9e 100644 --- a/libs/sqf/tests/lints/project_tests.toml +++ b/libs/sqf/tests/lints/project_tests.toml @@ -6,8 +6,15 @@ options.ignore = [ "addPublicVariableEventHandler", ] -[lints.sqf.inspector] -options.check_shadow = true - [lints.sqf] if_not_else = true +shadowed = true +not_private = true + +[lints.sqf.unused] +enabled = true +options.check_params = false + +[lints.sqf.undefined] +enabled = true +options.check_oprhan_code = true \ No newline at end of file diff --git a/libs/sqf/tests/lints/s12_inspector/source.sqf b/libs/sqf/tests/lints/s12_inspector/source.sqf deleted file mode 100644 index e91864ff..00000000 --- a/libs/sqf/tests/lints/s12_inspector/source.sqf +++ /dev/null @@ -1,4 +0,0 @@ -x setFuel true; // args: takes number 0-1 - -params ["_var1"]; -private _var1 = 7; diff --git a/libs/sqf/tests/lints/s12_inspector/stdout.ansi b/libs/sqf/tests/lints/s12_inspector/stdout.ansi deleted file mode 100644 index 736c60c9..00000000 --- a/libs/sqf/tests/lints/s12_inspector/stdout.ansi +++ /dev/null @@ -1,13 +0,0 @@ -help[L-S12]: Inspector - Shadowed: _var1 - ┌─ source.sqf:4:1 - │ -4 │ private _var1 = 7; - │ ^^^^^^^^^^^^^^^^^ - - -help[L-S12]: Inspector - Bad Args: [B:setFuel] - ┌─ source.sqf:1:3 - │ -1 │ x setFuel true; // args: takes number 0-1 - │ ^^^^^^^ - diff --git a/libs/sqf/tests/lints/s12_invalid_args/source.sqf b/libs/sqf/tests/lints/s12_invalid_args/source.sqf new file mode 100644 index 00000000..c36004ae --- /dev/null +++ b/libs/sqf/tests/lints/s12_invalid_args/source.sqf @@ -0,0 +1,3 @@ +player setPos [0,1,0]; +alive player; +(vehicle player) setFuel true; // bad args: takes number 0-1 diff --git a/libs/sqf/tests/lints/s12_invalid_args/stdout.ansi b/libs/sqf/tests/lints/s12_invalid_args/stdout.ansi new file mode 100644 index 00000000..860b6bd5 --- /dev/null +++ b/libs/sqf/tests/lints/s12_invalid_args/stdout.ansi @@ -0,0 +1,6 @@ +help[L-S12]: Invalid Args - [B:setFuel] + ┌─ source.sqf:3:18 + │ +3 │ (vehicle player) setFuel true; // bad args: takes number 0-1 + │ ^^^^^^^ + diff --git a/libs/sqf/tests/lints/s13_undefined/source.sqf b/libs/sqf/tests/lints/s13_undefined/source.sqf new file mode 100644 index 00000000..17a4d852 --- /dev/null +++ b/libs/sqf/tests/lints/s13_undefined/source.sqf @@ -0,0 +1 @@ +x = {systemChat _neverDefined;}; diff --git a/libs/sqf/tests/lints/s13_undefined/stdout.ansi b/libs/sqf/tests/lints/s13_undefined/stdout.ansi new file mode 100644 index 00000000..de3c9990 --- /dev/null +++ b/libs/sqf/tests/lints/s13_undefined/stdout.ansi @@ -0,0 +1,8 @@ +help[L-S13]: Undefined Var - _neverDefined + ┌─ source.sqf:1:17 + │ +1 │ x = {systemChat _neverDefined;}; + │ ^^^^^^^^^^^^^ + │ + = note: From Orphan Code - may not be a problem + diff --git a/libs/sqf/tests/lints/s14_unused/source.sqf b/libs/sqf/tests/lints/s14_unused/source.sqf new file mode 100644 index 00000000..5c8f0821 --- /dev/null +++ b/libs/sqf/tests/lints/s14_unused/source.sqf @@ -0,0 +1,2 @@ +private _z = 5; // and never used +params ["_something"]; \ No newline at end of file diff --git a/libs/sqf/tests/lints/s14_unused/stdout.ansi b/libs/sqf/tests/lints/s14_unused/stdout.ansi new file mode 100644 index 00000000..8891cc01 --- /dev/null +++ b/libs/sqf/tests/lints/s14_unused/stdout.ansi @@ -0,0 +1,6 @@ +help[L-S14]: Unused Var - _z + ┌─ source.sqf:1:1 + │ +1 │ private _z = 5; // and never used + │ ^^^^^^^^^^^^^^ + diff --git a/libs/sqf/tests/lints/s15_shadowed/source.sqf b/libs/sqf/tests/lints/s15_shadowed/source.sqf new file mode 100644 index 00000000..c6af8b2b --- /dev/null +++ b/libs/sqf/tests/lints/s15_shadowed/source.sqf @@ -0,0 +1,4 @@ +private _z = 5; +private _z = 5; + +systemChat str _z; // dummy use diff --git a/libs/sqf/tests/lints/s15_shadowed/stdout.ansi b/libs/sqf/tests/lints/s15_shadowed/stdout.ansi new file mode 100644 index 00000000..8734505f --- /dev/null +++ b/libs/sqf/tests/lints/s15_shadowed/stdout.ansi @@ -0,0 +1,6 @@ +help[L-S15]: Shadowed Var - _z + ┌─ source.sqf:2:1 + │ +2 │ private _z = 5; + │ ^^^^^^^^^^^^^^ + diff --git a/libs/sqf/tests/lints/s16_not_private/source.sqf b/libs/sqf/tests/lints/s16_not_private/source.sqf new file mode 100644 index 00000000..8b7c5246 --- /dev/null +++ b/libs/sqf/tests/lints/s16_not_private/source.sqf @@ -0,0 +1,3 @@ +_z = 6; + +systemChat str _z; // dummy use diff --git a/libs/sqf/tests/lints/s16_not_private/stdout.ansi b/libs/sqf/tests/lints/s16_not_private/stdout.ansi new file mode 100644 index 00000000..6d705302 --- /dev/null +++ b/libs/sqf/tests/lints/s16_not_private/stdout.ansi @@ -0,0 +1,6 @@ +help[L-S16]: Not Private - _z + ┌─ source.sqf:1:1 + │ +1 │ _z = 6; + │ ^^^^^^ + From 86db38326bb4de2e650dc6d0a8f4d65d27e9c019 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Wed, 9 Oct 2024 00:53:04 -0500 Subject: [PATCH 05/16] _forEachIndex --- libs/sqf/src/analyze/inspector/commands.rs | 8 ++++---- libs/sqf/src/analyze/inspector/mod.rs | 6 +++--- libs/sqf/tests/inspector/test_0.sqf | 3 +++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index 88a443bd..a348c1e2 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -177,7 +177,7 @@ impl SciptScope { pub fn cmd_generic_call_magic( &mut self, code_possibilities: &HashSet, - magic: &Vec<&str>, + magic: Vec<(&str, GameValue)>, source: &Range, database: &Database, ) -> HashSet { @@ -192,11 +192,11 @@ impl SciptScope { continue; } self.push(); - for var in magic { + for (var, value) in magic.iter() { self.var_assign( var, true, - HashSet::from([GameValue::Anything]), + HashSet::from([value.clone()]), VarSource::Magic(source.clone()), ); } @@ -363,7 +363,7 @@ impl SciptScope { ) -> HashSet { let mut return_value = cmd_set.clone(); // Check: `array select expression` - let _ = self.cmd_generic_call_magic(rhs, &vec!["_x"], source, database); + let _ = self.cmd_generic_call_magic(rhs, vec![("_x", GameValue::Anything)], source, database); // if lhs is array, and rhs is bool/number then put array into return if lhs.len() == 1 && rhs diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 1387e682..3df52d81 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -334,14 +334,14 @@ impl SciptScope { "then" => Some(self.cmd_b_then(&lhs_set, &rhs_set, database)), "foreach" | "foreachreversed" => Some(self.cmd_generic_call_magic( &lhs_set, - &vec!["_x", "_y", "_forEachIndex"], + vec![("_x", GameValue::Anything), ("_y", GameValue::Anything), ("_forEachIndex", GameValue::Number(None))], source, database, )), "count" => { let _ = self.cmd_generic_call_magic( &lhs_set, - &vec!["_x"], + vec![("_x", GameValue::Anything)], source, database, ); @@ -350,7 +350,7 @@ impl SciptScope { "findif" | "apply" => { let _ = self.cmd_generic_call_magic( &rhs_set, - &vec!["_x"], + vec![("_x", GameValue::Anything)], source, database, ); diff --git a/libs/sqf/tests/inspector/test_0.sqf b/libs/sqf/tests/inspector/test_0.sqf index e69de29b..6aa0f015 100644 --- a/libs/sqf/tests/inspector/test_0.sqf +++ b/libs/sqf/tests/inspector/test_0.sqf @@ -0,0 +1,3 @@ +{ + _forEachIndex + "A"; +} forEach z; \ No newline at end of file From 86fd860240d7a33b07c5f748ce497bf58e598f0e Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Mon, 14 Oct 2024 20:58:56 -0500 Subject: [PATCH 06/16] use `params` for possible types --- libs/sqf/src/analyze/inspector/commands.rs | 52 +++++++++++++++------- libs/sqf/src/analyze/inspector/mod.rs | 10 +++-- libs/sqf/tests/inspector.rs | 9 +++- libs/sqf/tests/inspector/test_0.sqf | 3 -- libs/sqf/tests/inspector/test_1.sqf | 11 +++++ libs/sqf/tests/lints.rs | 2 +- 6 files changed, 62 insertions(+), 25 deletions(-) diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index a348c1e2..00208f1e 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -69,25 +69,42 @@ impl SciptScope { VarSource::Params(source.clone()), ); } - GameValue::Array(Some(gv_array)) => { - if gv_array.is_empty() { + GameValue::Array(Some(arg_array)) => { + if arg_array.is_empty() || arg_array[0].is_empty() { continue; } - for element in &gv_array[0] { - if let GameValue::String(Some(Expression::String(var, source, _))) = - element - { - if var.is_empty() { - continue; + let GameValue::String(Some(Expression::String(var_name, source, _))) = + &arg_array[0][0] + else { + continue; + }; + if var_name.is_empty() { + continue; + } + let mut var_types = HashSet::new(); + if arg_array.len() > 2 { + for type_p in &arg_array[2] { + if let GameValue::Array(Some(type_array)) = type_p { + for type_i in type_array { + var_types.extend(type_i.iter().cloned()); + } } - self.var_assign( - var.as_ref(), - true, - HashSet::from([GameValue::Anything]), - VarSource::Params(source.clone()), - ); } } + if var_types.is_empty() { + var_types.insert(GameValue::Anything); + } + // Add the default value to types + // It should be possible to move this above the is_empty check but not always safe + if arg_array.len() > 1 && !arg_array[1].is_empty() { + var_types.insert(arg_array[1][0].clone()); + } + self.var_assign( + var_name.as_ref(), + true, + var_types, + VarSource::Params(source.clone()), + ); } _ => {} } @@ -177,7 +194,7 @@ impl SciptScope { pub fn cmd_generic_call_magic( &mut self, code_possibilities: &HashSet, - magic: Vec<(&str, GameValue)>, + magic: &Vec<(&str, GameValue)>, source: &Range, database: &Database, ) -> HashSet { @@ -192,7 +209,7 @@ impl SciptScope { continue; } self.push(); - for (var, value) in magic.iter() { + for (var, value) in magic { self.var_assign( var, true, @@ -363,7 +380,8 @@ impl SciptScope { ) -> HashSet { let mut return_value = cmd_set.clone(); // Check: `array select expression` - let _ = self.cmd_generic_call_magic(rhs, vec![("_x", GameValue::Anything)], source, database); + let _ = + self.cmd_generic_call_magic(rhs, &vec![("_x", GameValue::Anything)], source, database); // if lhs is array, and rhs is bool/number then put array into return if lhs.len() == 1 && rhs diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 3df52d81..bd113503 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -334,14 +334,18 @@ impl SciptScope { "then" => Some(self.cmd_b_then(&lhs_set, &rhs_set, database)), "foreach" | "foreachreversed" => Some(self.cmd_generic_call_magic( &lhs_set, - vec![("_x", GameValue::Anything), ("_y", GameValue::Anything), ("_forEachIndex", GameValue::Number(None))], + &vec![ + ("_x", GameValue::Anything), + ("_y", GameValue::Anything), + ("_forEachIndex", GameValue::Number(None)), + ], source, database, )), "count" => { let _ = self.cmd_generic_call_magic( &lhs_set, - vec![("_x", GameValue::Anything)], + &vec![("_x", GameValue::Anything)], source, database, ); @@ -350,7 +354,7 @@ impl SciptScope { "findif" | "apply" => { let _ = self.cmd_generic_call_magic( &rhs_set, - vec![("_x", GameValue::Anything)], + &vec![("_x", GameValue::Anything)], source, database, ); diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index 2441b17c..bc92976b 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(), 12); + assert_eq!(result.len(), 13); // Order not guarenteed assert!(result.iter().any(|i| { if let Issue::InvalidArgs(cmd, _) = i { @@ -123,6 +123,13 @@ mod tests { false } })); + assert!(result.iter().any(|i| { + if let Issue::InvalidArgs(cmd, _) = i { + cmd == "[B:setGusts]" + } else { + false + } + })); } #[test] diff --git a/libs/sqf/tests/inspector/test_0.sqf b/libs/sqf/tests/inspector/test_0.sqf index 6aa0f015..e69de29b 100644 --- a/libs/sqf/tests/inspector/test_0.sqf +++ b/libs/sqf/tests/inspector/test_0.sqf @@ -1,3 +0,0 @@ -{ - _forEachIndex + "A"; -} forEach z; \ No newline at end of file diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index 9a5ad934..4f7ad86a 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -114,3 +114,14 @@ _this select 0 drawIcon [ // invalidArgs private _varL = nil; call ([{_varL = 0;}, {_varL = 1;}] select (x == 1)); ["A", "B"] select _varL; + +params xParams; +params []; +params [[""]]; +params [["_varM", "", [""]], ["_varN", 1, [0]], ["_varO", { systemChat _varM }]]; +_varM = _varM + ""; +_varN = _varN + 2; +call _varO; + +params [["_someString", "abc", [""]], ["_someCode", { 60 setGusts _someString }]]; +call _someCode; // InvalidArgs for setGusts diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 257ce440..992e4821 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -90,4 +90,4 @@ analyze!(s14_unused, false); analyze!(s15_shadowed, false); analyze!(s16_not_private, false); -analyze!(s18_in_vehicle_check, false); \ No newline at end of file +analyze!(s18_in_vehicle_check, true); From 1878680546d3c36b614c8b6ad68a148fccd036b1 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 20 Oct 2024 18:41:42 -0500 Subject: [PATCH 07/16] update snaps? --- libs/sqf/tests/optimizer.rs | 6 +++--- .../tests/snapshots/lints__simple_s06_find_in_str.snap | 4 ++-- .../tests/snapshots/optimizer__simple_consume_array.snap | 3 +++ libs/sqf/tests/snapshots/optimizer__simple_scalar.snap | 1 + .../tests/snapshots/optimizer__simple_static_math.snap | 1 + .../tests/snapshots/optimizer__simple_string_case.snap | 1 + .../tests/snapshots/simple__simple_eventhandler-2.snap | 8 ++++++++ libs/sqf/tests/snapshots/simple__simple_foreach-2.snap | 3 +++ .../tests/snapshots/simple__simple_get_visibility-2.snap | 2 ++ libs/sqf/tests/snapshots/simple__simple_include-2.snap | 1 + libs/sqf/tests/snapshots/simple__simple_semicolons-2.snap | 1 + 11 files changed, 26 insertions(+), 5 deletions(-) diff --git a/libs/sqf/tests/optimizer.rs b/libs/sqf/tests/optimizer.rs index 953a5c82..8222e88c 100644 --- a/libs/sqf/tests/optimizer.rs +++ b/libs/sqf/tests/optimizer.rs @@ -31,7 +31,7 @@ fn optimize(file: &str) -> Statements { .unwrap(); let source = workspace.join(format!("{file}.sqf")).unwrap(); let processed = Processor::run(&source).unwrap(); - hemtt_sqf::parser::run(&Database::a3(false), &processed) - .unwrap() - .optimize() + let mut sqf = hemtt_sqf::parser::run(&Database::a3(false), &processed).unwrap(); + sqf.testing_clear_issues(); + sqf.optimize() } diff --git a/libs/sqf/tests/snapshots/lints__simple_s06_find_in_str.snap b/libs/sqf/tests/snapshots/lints__simple_s06_find_in_str.snap index 69e71fcf..9d73e975 100644 --- a/libs/sqf/tests/snapshots/lints__simple_s06_find_in_str.snap +++ b/libs/sqf/tests/snapshots/lints__simple_s06_find_in_str.snap @@ -1,6 +1,6 @@ --- source: libs/sqf/tests/lints.rs -expression: lint(stringify! (s06_find_in_str)) +expression: "lint(stringify! (s06_find_in_str), true)" --- help[L-S06]: string search using `in` is faster than `find` ┌─ s06_find_in_str.sqf:1:1 @@ -17,4 +17,4 @@ expression: lint(stringify! (s06_find_in_str)) 2 │ private _hasBar = things find "bar" > -1; │ ^^^^^^^^^^^^^^^^^^^^^^ using `find` with -1 │ - = try: "bar" in _things + = try: "bar" in things diff --git a/libs/sqf/tests/snapshots/optimizer__simple_consume_array.snap b/libs/sqf/tests/snapshots/optimizer__simple_consume_array.snap index 3ea53afb..41eb91d6 100644 --- a/libs/sqf/tests/snapshots/optimizer__simple_consume_array.snap +++ b/libs/sqf/tests/snapshots/optimizer__simple_consume_array.snap @@ -300,6 +300,7 @@ Statements { ], source: "1;2;3;4;", span: 247..255, + issues: [], }, ), Code( @@ -326,6 +327,7 @@ Statements { ], source: "-1;-2;", span: 265..271, + issues: [], }, ), ], @@ -380,4 +382,5 @@ Statements { ], source: "params [\"_a\", \"_b\"];\n\nparams [\"_a\", \"_b\", [\"_c\", []]];\n\nmissionNamespace getVariable [\"a\", -1];\n\nz setVariable [\"b\", [], true];\n\n[1,0] vectorAdd p;\n\npositionCameraToWorld [10000, 0, 10000];\n\n\nrandom [0, _x, 1];\n\nprivate _z = if (time > 10) then { 1;2;3;4; } else { -1;-2; };\n\nparam [\"_d\"];\n\n[] param [\"_e\"];\n", span: 0..307, + issues: [], } diff --git a/libs/sqf/tests/snapshots/optimizer__simple_scalar.snap b/libs/sqf/tests/snapshots/optimizer__simple_scalar.snap index a1551d85..01cb2780 100644 --- a/libs/sqf/tests/snapshots/optimizer__simple_scalar.snap +++ b/libs/sqf/tests/snapshots/optimizer__simple_scalar.snap @@ -16,4 +16,5 @@ Statements { ], source: "-5;\n", span: 0..3, + issues: [], } diff --git a/libs/sqf/tests/snapshots/optimizer__simple_static_math.snap b/libs/sqf/tests/snapshots/optimizer__simple_static_math.snap index 0110c807..bd12765d 100644 --- a/libs/sqf/tests/snapshots/optimizer__simple_static_math.snap +++ b/libs/sqf/tests/snapshots/optimizer__simple_static_math.snap @@ -46,4 +46,5 @@ Statements { ], source: "1 + (2 * 2) + (36 % 31) + (36 / 6) + (sqrt 100) - 3;\n\nsqrt -100;\n\n\nz + z;\n", span: 0..73, + issues: [], } diff --git a/libs/sqf/tests/snapshots/optimizer__simple_string_case.snap b/libs/sqf/tests/snapshots/optimizer__simple_string_case.snap index e81e8564..3e6cabd8 100644 --- a/libs/sqf/tests/snapshots/optimizer__simple_string_case.snap +++ b/libs/sqf/tests/snapshots/optimizer__simple_string_case.snap @@ -15,4 +15,5 @@ Statements { ], source: "toLower \"A\" + toUpper \"b\" + toUpperAnsi \"C\" + toLowerAnsi \"d\";\n", span: 0..62, + issues: [], } diff --git a/libs/sqf/tests/snapshots/simple__simple_eventhandler-2.snap b/libs/sqf/tests/snapshots/simple__simple_eventhandler-2.snap index e09ece6f..09537998 100644 --- a/libs/sqf/tests/snapshots/simple__simple_eventhandler-2.snap +++ b/libs/sqf/tests/snapshots/simple__simple_eventhandler-2.snap @@ -27,6 +27,7 @@ expression: ast ], source: "deleteVehicle _x", span: 3..19, + issues: [], }, ), NularCommand( @@ -110,6 +111,7 @@ expression: ast ], source: "alive _x", span: 115..123, + issues: [], }, ), 106..112, @@ -140,6 +142,7 @@ expression: ast ], source: "deleteVehicle _x;", span: 149..166, + issues: [], }, ), NularCommand( @@ -155,6 +158,7 @@ expression: ast ], source: "allPlayers findIf { alive _x };\n {\n deleteVehicle _x;\n } forEach allPlayers;", span: 95..196, + issues: [], }, ), 80..84, @@ -164,6 +168,7 @@ expression: ast ], source: "if (alive player) then {\n allPlayers findIf { alive _x };\n {\n deleteVehicle _x;\n } forEach allPlayers;\n };", span: 62..203, + issues: [], }, ), ], @@ -242,6 +247,7 @@ expression: ast ], source: "deleteVehicle _x", span: 294..310, + issues: [], }, ), NularCommand( @@ -257,6 +263,7 @@ expression: ast ], source: "{ deleteVehicle _x } count allPlayers;", span: 292..330, + issues: [], }, ), 277..281, @@ -266,6 +273,7 @@ expression: ast ], source: "if (alive player) then {\n { deleteVehicle _x } count allPlayers;\n };", span: 259..337, + issues: [], }, ), ], diff --git a/libs/sqf/tests/snapshots/simple__simple_foreach-2.snap b/libs/sqf/tests/snapshots/simple__simple_foreach-2.snap index 24bc3e96..96cab024 100644 --- a/libs/sqf/tests/snapshots/simple__simple_foreach-2.snap +++ b/libs/sqf/tests/snapshots/simple__simple_foreach-2.snap @@ -27,6 +27,7 @@ expression: ast ], source: "deleteVehicle _x;", span: 7..24, + issues: [], }, ), NularCommand( @@ -106,6 +107,7 @@ expression: ast ], source: "_x setDamage 1;", span: 97..112, + issues: [], }, ), UnaryCommand( @@ -125,6 +127,7 @@ expression: ast ], source: "systemChat format [\"%1\", _x];\n {\n _x setDamage 1;\n } forEach crew _x;", span: 53..135, + issues: [], }, ), NularCommand( diff --git a/libs/sqf/tests/snapshots/simple__simple_get_visibility-2.snap b/libs/sqf/tests/snapshots/simple__simple_get_visibility-2.snap index 0e168ddf..3c935a5e 100644 --- a/libs/sqf/tests/snapshots/simple__simple_get_visibility-2.snap +++ b/libs/sqf/tests/snapshots/simple__simple_get_visibility-2.snap @@ -86,6 +86,7 @@ expression: ast ], source: "_arg1 = [eyePos _arg1, _arg1]", span: 66..95, + issues: [], }, ), 59..63, @@ -151,6 +152,7 @@ expression: ast ], source: "_arg2 = [eyePos _arg2, _arg2]", span: 138..167, + issues: [], }, ), 131..135, diff --git a/libs/sqf/tests/snapshots/simple__simple_include-2.snap b/libs/sqf/tests/snapshots/simple__simple_include-2.snap index 726ce909..1adfd6f8 100644 --- a/libs/sqf/tests/snapshots/simple__simple_include-2.snap +++ b/libs/sqf/tests/snapshots/simple__simple_include-2.snap @@ -130,6 +130,7 @@ expression: ast ], source: "private thinghi = _x + \"test\";", span: 94..124, + issues: [], }, ), Array( diff --git a/libs/sqf/tests/snapshots/simple__simple_semicolons-2.snap b/libs/sqf/tests/snapshots/simple__simple_semicolons-2.snap index ede82283..a982af3d 100644 --- a/libs/sqf/tests/snapshots/simple__simple_semicolons-2.snap +++ b/libs/sqf/tests/snapshots/simple__simple_semicolons-2.snap @@ -78,6 +78,7 @@ expression: ast ], source: "systemChat \"this is a test\";", span: 153..181, + issues: [], }, ), 136..140, From 8161ba6b25edd8304b955e77b949bdf8c3d0db20 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 20 Oct 2024 21:31:56 -0500 Subject: [PATCH 08/16] update tests --- libs/sqf/tests/lints.rs | 10 +++++----- libs/sqf/tests/lints/project_tests.toml | 2 +- .../source.sqf => s12_invalid_args.sqf} | 0 .../{s13_undefined/source.sqf => s13_undefined.sqf} | 0 .../lints/{s14_unused/source.sqf => s14_unused.sqf} | 0 .../{s15_shadowed/source.sqf => s15_shadowed.sqf} | 0 .../source.sqf => s16_not_private.sqf} | 0 .../lints__simple_s12_invalid_args.snap} | 7 +++++-- .../lints__simple_s13_undefined.snap} | 7 +++++-- .../lints__simple_s14_unused.snap} | 7 +++++-- .../lints__simple_s15_shadowed.snap} | 7 +++++-- .../lints__simple_s16_not_private.snap} | 7 +++++-- 12 files changed, 31 insertions(+), 16 deletions(-) rename libs/sqf/tests/lints/{s12_invalid_args/source.sqf => s12_invalid_args.sqf} (100%) rename libs/sqf/tests/lints/{s13_undefined/source.sqf => s13_undefined.sqf} (100%) rename libs/sqf/tests/lints/{s14_unused/source.sqf => s14_unused.sqf} (100%) rename libs/sqf/tests/lints/{s15_shadowed/source.sqf => s15_shadowed.sqf} (100%) rename libs/sqf/tests/lints/{s16_not_private/source.sqf => s16_not_private.sqf} (100%) rename libs/sqf/tests/{lints/s12_invalid_args/stdout.ansi => snapshots/lints__simple_s12_invalid_args.snap} (63%) rename libs/sqf/tests/{lints/s13_undefined/stdout.ansi => snapshots/lints__simple_s13_undefined.snap} (69%) rename libs/sqf/tests/{lints/s14_unused/stdout.ansi => snapshots/lints__simple_s14_unused.snap} (60%) rename libs/sqf/tests/{lints/s15_shadowed/stdout.ansi => snapshots/lints__simple_s15_shadowed.snap} (58%) rename libs/sqf/tests/{lints/s16_not_private/stdout.ansi => snapshots/lints__simple_s16_not_private.snap} (54%) diff --git a/libs/sqf/tests/lints.rs b/libs/sqf/tests/lints.rs index 4abb13e2..e2b7653c 100644 --- a/libs/sqf/tests/lints.rs +++ b/libs/sqf/tests/lints.rs @@ -29,11 +29,11 @@ lint!(s07_select_parse_number, true); lint!(s08_format_args, true); lint!(s09_banned_command, true); lint!(s11_if_not_else, true); -// analyze!(s12_invalid_args, false); -// analyze!(s13_undefined, false); -// analyze!(s14_unused, false); -// analyze!(s15_shadowed, false); -// analyze!(s16_not_private, false); +lint!(s12_invalid_args, false); +lint!(s13_undefined, false); +lint!(s14_unused, false); +lint!(s15_shadowed, false); +lint!(s16_not_private, false); lint!(s17_var_all_caps, true); lint!(s18_in_vehicle_check, true); lint!(s20_bool_static_comparison, true); diff --git a/libs/sqf/tests/lints/project_tests.toml b/libs/sqf/tests/lints/project_tests.toml index 6090ebd3..5cde1a13 100644 --- a/libs/sqf/tests/lints/project_tests.toml +++ b/libs/sqf/tests/lints/project_tests.toml @@ -14,7 +14,7 @@ if_not_else = true enabled = true [lints.sqf.not_private] -not_private = true +enabled = true [lints.sqf.unused] enabled = true diff --git a/libs/sqf/tests/lints/s12_invalid_args/source.sqf b/libs/sqf/tests/lints/s12_invalid_args.sqf similarity index 100% rename from libs/sqf/tests/lints/s12_invalid_args/source.sqf rename to libs/sqf/tests/lints/s12_invalid_args.sqf diff --git a/libs/sqf/tests/lints/s13_undefined/source.sqf b/libs/sqf/tests/lints/s13_undefined.sqf similarity index 100% rename from libs/sqf/tests/lints/s13_undefined/source.sqf rename to libs/sqf/tests/lints/s13_undefined.sqf diff --git a/libs/sqf/tests/lints/s14_unused/source.sqf b/libs/sqf/tests/lints/s14_unused.sqf similarity index 100% rename from libs/sqf/tests/lints/s14_unused/source.sqf rename to libs/sqf/tests/lints/s14_unused.sqf diff --git a/libs/sqf/tests/lints/s15_shadowed/source.sqf b/libs/sqf/tests/lints/s15_shadowed.sqf similarity index 100% rename from libs/sqf/tests/lints/s15_shadowed/source.sqf rename to libs/sqf/tests/lints/s15_shadowed.sqf diff --git a/libs/sqf/tests/lints/s16_not_private/source.sqf b/libs/sqf/tests/lints/s16_not_private.sqf similarity index 100% rename from libs/sqf/tests/lints/s16_not_private/source.sqf rename to libs/sqf/tests/lints/s16_not_private.sqf diff --git a/libs/sqf/tests/lints/s12_invalid_args/stdout.ansi b/libs/sqf/tests/snapshots/lints__simple_s12_invalid_args.snap similarity index 63% rename from libs/sqf/tests/lints/s12_invalid_args/stdout.ansi rename to libs/sqf/tests/snapshots/lints__simple_s12_invalid_args.snap index 860b6bd5..6babe1b4 100644 --- a/libs/sqf/tests/lints/s12_invalid_args/stdout.ansi +++ b/libs/sqf/tests/snapshots/lints__simple_s12_invalid_args.snap @@ -1,6 +1,9 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s12_invalid_args), false)" +--- help[L-S12]: Invalid Args - [B:setFuel] - ┌─ source.sqf:3:18 + ┌─ s12_invalid_args.sqf:3:18 │ 3 │ (vehicle player) setFuel true; // bad args: takes number 0-1 │ ^^^^^^^ - diff --git a/libs/sqf/tests/lints/s13_undefined/stdout.ansi b/libs/sqf/tests/snapshots/lints__simple_s13_undefined.snap similarity index 69% rename from libs/sqf/tests/lints/s13_undefined/stdout.ansi rename to libs/sqf/tests/snapshots/lints__simple_s13_undefined.snap index de3c9990..71fe54bc 100644 --- a/libs/sqf/tests/lints/s13_undefined/stdout.ansi +++ b/libs/sqf/tests/snapshots/lints__simple_s13_undefined.snap @@ -1,8 +1,11 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s13_undefined), false)" +--- help[L-S13]: Undefined Var - _neverDefined - ┌─ source.sqf:1:17 + ┌─ s13_undefined.sqf:1:17 │ 1 │ x = {systemChat _neverDefined;}; │ ^^^^^^^^^^^^^ │ = note: From Orphan Code - may not be a problem - diff --git a/libs/sqf/tests/lints/s14_unused/stdout.ansi b/libs/sqf/tests/snapshots/lints__simple_s14_unused.snap similarity index 60% rename from libs/sqf/tests/lints/s14_unused/stdout.ansi rename to libs/sqf/tests/snapshots/lints__simple_s14_unused.snap index 8891cc01..c68fafc5 100644 --- a/libs/sqf/tests/lints/s14_unused/stdout.ansi +++ b/libs/sqf/tests/snapshots/lints__simple_s14_unused.snap @@ -1,6 +1,9 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s14_unused), false)" +--- help[L-S14]: Unused Var - _z - ┌─ source.sqf:1:1 + ┌─ s14_unused.sqf:1:1 │ 1 │ private _z = 5; // and never used │ ^^^^^^^^^^^^^^ - diff --git a/libs/sqf/tests/lints/s15_shadowed/stdout.ansi b/libs/sqf/tests/snapshots/lints__simple_s15_shadowed.snap similarity index 58% rename from libs/sqf/tests/lints/s15_shadowed/stdout.ansi rename to libs/sqf/tests/snapshots/lints__simple_s15_shadowed.snap index 8734505f..ae970ed3 100644 --- a/libs/sqf/tests/lints/s15_shadowed/stdout.ansi +++ b/libs/sqf/tests/snapshots/lints__simple_s15_shadowed.snap @@ -1,6 +1,9 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s15_shadowed), false)" +--- help[L-S15]: Shadowed Var - _z - ┌─ source.sqf:2:1 + ┌─ s15_shadowed.sqf:2:1 │ 2 │ private _z = 5; │ ^^^^^^^^^^^^^^ - diff --git a/libs/sqf/tests/lints/s16_not_private/stdout.ansi b/libs/sqf/tests/snapshots/lints__simple_s16_not_private.snap similarity index 54% rename from libs/sqf/tests/lints/s16_not_private/stdout.ansi rename to libs/sqf/tests/snapshots/lints__simple_s16_not_private.snap index 6d705302..47dd8cd6 100644 --- a/libs/sqf/tests/lints/s16_not_private/stdout.ansi +++ b/libs/sqf/tests/snapshots/lints__simple_s16_not_private.snap @@ -1,6 +1,9 @@ +--- +source: libs/sqf/tests/lints.rs +expression: "lint(stringify! (s16_not_private), false)" +--- help[L-S16]: Not Private - _z - ┌─ source.sqf:1:1 + ┌─ s16_not_private.sqf:1:1 │ 1 │ _z = 6; │ ^^^^^^ - From 20090060e4cfaf776f02702e7a197e2f4c21eed2 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Mon, 21 Oct 2024 23:50:19 -0500 Subject: [PATCH 09/16] fix config spelling --- libs/sqf/src/analyze/lints/s13_undefined.rs | 6 +++--- libs/sqf/tests/lints/project_tests.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/sqf/src/analyze/lints/s13_undefined.rs b/libs/sqf/src/analyze/lints/s13_undefined.rs index 01ac0a12..6afc46eb 100644 --- a/libs/sqf/src/analyze/lints/s13_undefined.rs +++ b/libs/sqf/src/analyze/lints/s13_undefined.rs @@ -58,8 +58,8 @@ impl LintRunner for Runner { let Some(processed) = processed else { return Vec::new(); }; - let check_oprhan_code = - if let Some(toml::Value::Boolean(b)) = config.option("check_oprhan_code") { + let check_orphan_code = + if let Some(toml::Value::Boolean(b)) = config.option("check_orphan_code") { *b } else { false @@ -68,7 +68,7 @@ impl LintRunner for Runner { for issue in target.issues() { if let Issue::Undefined(var, range, is_orphan_scope) = issue { let error_hint = if *is_orphan_scope { - if !check_oprhan_code { + if !check_orphan_code { continue; } Some("From Orphan Code - may not be a problem".to_owned()) diff --git a/libs/sqf/tests/lints/project_tests.toml b/libs/sqf/tests/lints/project_tests.toml index 5cde1a13..2fd943a6 100644 --- a/libs/sqf/tests/lints/project_tests.toml +++ b/libs/sqf/tests/lints/project_tests.toml @@ -22,7 +22,7 @@ options.check_params = false [lints.sqf.undefined] enabled = true -options.check_oprhan_code = true +options.check_orphan_code = true [lints.sqf.var_all_caps] enabled = true From b3bb7e66a0030e542f7edd9e9eb4ab809dc6b390 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Mon, 21 Oct 2024 23:53:59 -0500 Subject: [PATCH 10/16] update lints --- libs/sqf/src/analyze/lints/s12_invalid_args.rs | 4 ++-- libs/sqf/src/analyze/lints/s13_undefined.rs | 4 ++-- libs/sqf/src/analyze/lints/s14_unused.rs | 4 ++-- libs/sqf/src/analyze/lints/s15_shadowed.rs | 4 ++-- libs/sqf/src/analyze/lints/s16_not_private.rs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libs/sqf/src/analyze/lints/s12_invalid_args.rs b/libs/sqf/src/analyze/lints/s12_invalid_args.rs index a535a18a..af7dbb8d 100644 --- a/libs/sqf/src/analyze/lints/s12_invalid_args.rs +++ b/libs/sqf/src/analyze/lints/s12_invalid_args.rs @@ -9,7 +9,7 @@ use hemtt_workspace::{ }; use std::{ops::Range, sync::Arc}; -crate::lint!(LintS12InvalidArgs); +crate::analyze::lint!(LintS12InvalidArgs); impl Lint for LintS12InvalidArgs { fn ident(&self) -> &str { @@ -129,7 +129,7 @@ impl CodeS12InvalidArgs { .generate_processed(processed) } fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); self } } diff --git a/libs/sqf/src/analyze/lints/s13_undefined.rs b/libs/sqf/src/analyze/lints/s13_undefined.rs index 6afc46eb..65799803 100644 --- a/libs/sqf/src/analyze/lints/s13_undefined.rs +++ b/libs/sqf/src/analyze/lints/s13_undefined.rs @@ -9,7 +9,7 @@ use hemtt_workspace::{ }; use std::{ops::Range, sync::Arc}; -crate::lint!(LintS13Undefined); +crate::analyze::lint!(LintS13Undefined); impl Lint for LintS13Undefined { fn ident(&self) -> &str { @@ -143,7 +143,7 @@ impl CodeS13Undefined { .generate_processed(processed) } fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); self } } diff --git a/libs/sqf/src/analyze/lints/s14_unused.rs b/libs/sqf/src/analyze/lints/s14_unused.rs index a35e3ae7..852f83e1 100644 --- a/libs/sqf/src/analyze/lints/s14_unused.rs +++ b/libs/sqf/src/analyze/lints/s14_unused.rs @@ -12,7 +12,7 @@ use hemtt_workspace::{ }; use std::{ops::Range, sync::Arc}; -crate::lint!(LintS14Unused); +crate::analyze::lint!(LintS14Unused); impl Lint for LintS14Unused { fn ident(&self) -> &str { @@ -149,7 +149,7 @@ impl CodeS14Unused { .generate_processed(processed) } fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); self } } diff --git a/libs/sqf/src/analyze/lints/s15_shadowed.rs b/libs/sqf/src/analyze/lints/s15_shadowed.rs index 38264954..9f46615e 100644 --- a/libs/sqf/src/analyze/lints/s15_shadowed.rs +++ b/libs/sqf/src/analyze/lints/s15_shadowed.rs @@ -9,7 +9,7 @@ use hemtt_workspace::{ }; use std::{ops::Range, sync::Arc}; -crate::lint!(LintS15Shadowed); +crate::analyze::lint!(LintS15Shadowed); impl Lint for LintS15Shadowed { fn ident(&self) -> &str { @@ -130,7 +130,7 @@ impl CodeS15Shadowed { .generate_processed(processed) } fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); self } } diff --git a/libs/sqf/src/analyze/lints/s16_not_private.rs b/libs/sqf/src/analyze/lints/s16_not_private.rs index a938b28f..fe305e9e 100644 --- a/libs/sqf/src/analyze/lints/s16_not_private.rs +++ b/libs/sqf/src/analyze/lints/s16_not_private.rs @@ -9,7 +9,7 @@ use hemtt_workspace::{ }; use std::{ops::Range, sync::Arc}; -crate::lint!(LintS16NotPrivate); +crate::analyze::lint!(LintS16NotPrivate); impl Lint for LintS16NotPrivate { fn ident(&self) -> &str { @@ -129,7 +129,7 @@ impl CodeS16NotPrivate { .generate_processed(processed) } fn generate_processed(mut self, processed: &Processed) -> Self { - self.diagnostic = Diagnostic::new_for_processed(&self, self.span.clone(), processed); + self.diagnostic = Diagnostic::from_code_processed(&self, self.span.clone(), processed); self } } From a7d9792e52426b67f727ab6ce628d58512f5f865 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 3 Nov 2024 11:11:40 -0600 Subject: [PATCH 11/16] Update game_value.rs --- libs/sqf/src/analyze/inspector/game_value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs index 080aae78..5a3e29e0 100644 --- a/libs/sqf/src/analyze/inspector/game_value.rs +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -126,6 +126,7 @@ impl GameValue { "set3DENMissionAttributes", "setPiPEffect", "ppEffectCreate", + "inAreaArray", ]; if !WIKI_CMDS_IGNORE_MISSING_PARAM.contains(&cmd_name) { warn!("cmd {cmd_name} - param {name} not found"); From 8838a34e07d22deb59439510235edf9306bd78a8 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 10 Nov 2024 15:18:34 -0600 Subject: [PATCH 12/16] use generic version of value for param types --- libs/sqf/src/analyze/inspector/commands.rs | 7 +++-- .../analyze/inspector/external_functions.rs | 2 ++ libs/sqf/src/analyze/inspector/game_value.rs | 29 ++++++++++++------- libs/sqf/src/analyze/inspector/mod.rs | 1 + libs/sqf/tests/inspector/test_1.sqf | 5 ++++ 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index 00208f1e..d955339f 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -1,3 +1,5 @@ +//! Emulates engine commands + use std::{collections::HashSet, ops::Range}; use crate::{analyze::inspector::VarSource, parser::database::Database, Expression}; @@ -86,7 +88,7 @@ impl SciptScope { for type_p in &arg_array[2] { if let GameValue::Array(Some(type_array)) = type_p { for type_i in type_array { - var_types.extend(type_i.iter().cloned()); + var_types.extend(type_i.iter().map(GameValue::make_generic)); } } } @@ -95,7 +97,8 @@ impl SciptScope { var_types.insert(GameValue::Anything); } // Add the default value to types - // It should be possible to move this above the is_empty check but not always safe + // It would be nice to move this above the is_empty check but not always safe + // ie: assume `params ["_z", ""]` is type string, but this is not guarentted if arg_array.len() > 1 && !arg_array[1].is_empty() { var_types.insert(arg_array[1][0].clone()); } diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs index e7d6f1cb..a70b7e5a 100644 --- a/libs/sqf/src/analyze/inspector/external_functions.rs +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -1,3 +1,5 @@ +//! Emulate how common external functions will handle code + use std::collections::HashSet; use crate::{analyze::inspector::VarSource, parser::database::Database, Expression}; diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs index 5a3e29e0..aa3fa805 100644 --- a/libs/sqf/src/analyze/inspector/game_value.rs +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -1,3 +1,5 @@ +//! Game Values and mapping them from commands + use std::collections::HashSet; use arma3_wiki::model::{Arg, Call, Param, Value}; @@ -129,7 +131,7 @@ impl GameValue { "inAreaArray", ]; if !WIKI_CMDS_IGNORE_MISSING_PARAM.contains(&cmd_name) { - warn!("cmd {cmd_name} - param {name} not found"); + // warn!("cmd {cmd_name} - param {name} not found"); } return true; }; @@ -146,7 +148,7 @@ impl GameValue { return true; } - let test = set.iter().any(|s| { + set.iter().any(|s| { match s { Self::Anything | Self::Array(None) => { // println!("array (any/generic) pass"); @@ -154,9 +156,7 @@ impl GameValue { } Self::Array(Some(gv_array)) => { // println!("array (gv: {}) expected (arg: {})", gv_array.len(), arg_array.len()); - // if gv_array.len() > arg_array.len() { - // not really an error, some syntaxes take more than others - // } + // note: some syntaxes take more than others for (index, arg) in arg_array.iter().enumerate() { let possible = if index < gv_array.len() { gv_array[index].iter().cloned().collect() @@ -171,9 +171,7 @@ impl GameValue { } _ => false, } - }); - - test + }) } } } @@ -190,7 +188,6 @@ impl GameValue { #[must_use] /// matches values are compatible (Anything will always match) - /// todo: think about how nil and any interact? pub fn match_values(left: &Self, right: &Self) -> bool { if matches!(left, Self::Anything) { return true; @@ -247,7 +244,19 @@ impl GameValue { } } } - + #[must_use] + /// Gets a generic version of a type + pub fn make_generic(&self) -> Self { + match self { + Self::Array(_) => Self::Array(None), + Self::Boolean(_) => Self::Boolean(None), + Self::Code(_) => Self::Code(None), + Self::ForType(_) => Self::ForType(None), + Self::Number(_) => Self::Number(None), + Self::String(_) => Self::String(None), + _ => self.clone() + } + } #[must_use] /// Get as a string for debugging pub fn as_debug(&self) -> String { diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index bd113503..8add67d6 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -328,6 +328,7 @@ impl SciptScope { } "do" => { // from While, With, For, and Switch + // todo: handle switch return value Some(self.cmd_b_do(&lhs_set, &rhs_set, database)) } "from" | "to" | "step" => Some(self.cmd_b_from_chain(&lhs_set, &rhs_set)), diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index 4f7ad86a..d83115a9 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -125,3 +125,8 @@ call _varO; params [["_someString", "abc", [""]], ["_someCode", { 60 setGusts _someString }]]; call _someCode; // InvalidArgs for setGusts + +// ensure we use a generic version of the array param types or format would have an error +params [["_varP", "", ["", []]]]; +format _varP; + From b686b2a2f4e9bc601b413e775cbd8ae46807a32d Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sun, 24 Nov 2024 15:13:36 -0600 Subject: [PATCH 13/16] Improve checking code in external scopes --- .../analyze/inspector/external_functions.rs | 18 ++++++++++-- libs/sqf/src/analyze/inspector/mod.rs | 1 + libs/sqf/tests/inspector.rs | 28 ++++++++++++++----- libs/sqf/tests/inspector/test_1.sqf | 8 ++++++ 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs index a70b7e5a..f96d8b8e 100644 --- a/libs/sqf/src/analyze/inspector/external_functions.rs +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -89,7 +89,7 @@ impl SciptScope { } } } - "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" => { + "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" | "cba_fnc_execnextframe" => { if !gv_array.is_empty() { self.external_new_scope(&gv_array[0], &vec![], database); } @@ -113,6 +113,20 @@ impl SciptScope { ); } } + "cba_fnc_addeventhandlerargs" => { + if gv_array.len() > 1 { + self.external_new_scope( + &gv_array[1], + &vec![ + ("_thisType", GameValue::String(None)), + ("_thisId", GameValue::Number(None)), + ("_thisFnc", GameValue::Code(None)), + ("_thisArgs", GameValue::Anything), + ], + database, + ); + } + } _ => {} }, _ => {} @@ -135,7 +149,7 @@ impl SciptScope { if self.code_used.contains(expression) { return; } - let mut ext_scope = Self::create(&self.ignored_vars, true); + let mut ext_scope = Self::create(&self.ignored_vars, false); for (var, value) in vars { ext_scope.var_assign(var, true, HashSet::from([value.clone()]), VarSource::Ignore); diff --git a/libs/sqf/src/analyze/inspector/mod.rs b/libs/sqf/src/analyze/inspector/mod.rs index 8add67d6..0313a736 100644 --- a/libs/sqf/src/analyze/inspector/mod.rs +++ b/libs/sqf/src/analyze/inspector/mod.rs @@ -70,6 +70,7 @@ pub struct SciptScope { local: Vec, code_seen: HashSet, code_used: HashSet, + /// Orphan scopes are code blocks that are created but don't appear to be called in a known way is_orphan_scope: bool, ignored_vars: HashSet, } diff --git a/libs/sqf/tests/inspector.rs b/libs/sqf/tests/inspector.rs index bc92976b..2e5c630a 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(), 13); + assert_eq!(result.len(), 15); // Order not guarenteed assert!(result.iter().any(|i| { if let Issue::InvalidArgs(cmd, _) = i { @@ -47,8 +47,8 @@ mod tests { } })); assert!(result.iter().any(|i| { - if let Issue::Undefined(var, _, _) = i { - var == "_test2" + if let Issue::Undefined(var, _, orphan) = i { + var == "_test2" && !orphan } else { false } @@ -89,15 +89,15 @@ mod tests { } })); assert!(result.iter().any(|i| { - if let Issue::Undefined(var, _, _) = i { - var == "_test8" + if let Issue::Undefined(var, _, orphan) = i { + var == "_test8" && !orphan } else { false } })); assert!(result.iter().any(|i| { - if let Issue::Undefined(var, _, _) = i { - var == "_test9" + if let Issue::Undefined(var, _, orphan) = i { + var == "_test9" && !orphan } else { false } @@ -130,6 +130,20 @@ mod tests { false } })); + assert!(result.iter().any(|i| { + if let Issue::Undefined(var, _, orphan) = i { + var == "_test12" && !orphan + } else { + false + } + })); + assert!(result.iter().any(|i| { + if let Issue::Undefined(var, _, orphan) = i { + var == "_test13" && *orphan + } else { + false + } + })); } #[test] diff --git a/libs/sqf/tests/inspector/test_1.sqf b/libs/sqf/tests/inspector/test_1.sqf index d83115a9..6505d61c 100644 --- a/libs/sqf/tests/inspector/test_1.sqf +++ b/libs/sqf/tests/inspector/test_1.sqf @@ -130,3 +130,11 @@ call _someCode; // InvalidArgs for setGusts params [["_varP", "", ["", []]]]; format _varP; + +[{ + [_test12] call some_func; // undef, not orphan because CBA_fnc_execNextFrame is a known clean scope +}, player] call CBA_fnc_execNextFrame; +[{ + [_test13] call some_func; // undef, is orphan +}, player] call unknown_fnc_Usage; + From 2fa6d8b631ecd09ecfbf18038b3602f3fbde989d Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Sat, 14 Dec 2024 00:02:21 -0600 Subject: [PATCH 14/16] func sig change --- libs/sqf/src/analyze/lints/s12_invalid_args.rs | 6 +++--- libs/sqf/src/analyze/lints/s13_undefined.rs | 6 +++--- libs/sqf/src/analyze/lints/s14_unused.rs | 6 +++--- libs/sqf/src/analyze/lints/s15_shadowed.rs | 6 +++--- libs/sqf/src/analyze/lints/s16_not_private.rs | 6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/libs/sqf/src/analyze/lints/s12_invalid_args.rs b/libs/sqf/src/analyze/lints/s12_invalid_args.rs index af7dbb8d..acff86ac 100644 --- a/libs/sqf/src/analyze/lints/s12_invalid_args.rs +++ b/libs/sqf/src/analyze/lints/s12_invalid_args.rs @@ -12,16 +12,16 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS12InvalidArgs); impl Lint for LintS12InvalidArgs { - fn ident(&self) -> &str { + fn ident(&self) -> &'static str { "invalid_args" } fn sort(&self) -> u32 { 120 } - fn description(&self) -> &str { + fn description(&self) -> &'static str { "Invalid Args" } - fn documentation(&self) -> &str { + fn documentation(&self) -> &'static str { r"### Example **Incorrect** diff --git a/libs/sqf/src/analyze/lints/s13_undefined.rs b/libs/sqf/src/analyze/lints/s13_undefined.rs index 65799803..9ee4691e 100644 --- a/libs/sqf/src/analyze/lints/s13_undefined.rs +++ b/libs/sqf/src/analyze/lints/s13_undefined.rs @@ -12,16 +12,16 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS13Undefined); impl Lint for LintS13Undefined { - fn ident(&self) -> &str { + fn ident(&self) -> &'static str { "undefined" } fn sort(&self) -> u32 { 130 } - fn description(&self) -> &str { + fn description(&self) -> &'static str { "Undefined Variable" } - fn documentation(&self) -> &str { + fn documentation(&self) -> &'static str { r"### Example **Incorrect** diff --git a/libs/sqf/src/analyze/lints/s14_unused.rs b/libs/sqf/src/analyze/lints/s14_unused.rs index 852f83e1..269d67af 100644 --- a/libs/sqf/src/analyze/lints/s14_unused.rs +++ b/libs/sqf/src/analyze/lints/s14_unused.rs @@ -15,16 +15,16 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS14Unused); impl Lint for LintS14Unused { - fn ident(&self) -> &str { + fn ident(&self) -> &'static str { "unused" } fn sort(&self) -> u32 { 120 } - fn description(&self) -> &str { + fn description(&self) -> &'static str { "Unused Var" } - fn documentation(&self) -> &str { + fn documentation(&self) -> &'static str { r"### Example **Incorrect** diff --git a/libs/sqf/src/analyze/lints/s15_shadowed.rs b/libs/sqf/src/analyze/lints/s15_shadowed.rs index 9f46615e..eb0b0388 100644 --- a/libs/sqf/src/analyze/lints/s15_shadowed.rs +++ b/libs/sqf/src/analyze/lints/s15_shadowed.rs @@ -12,16 +12,16 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS15Shadowed); impl Lint for LintS15Shadowed { - fn ident(&self) -> &str { + fn ident(&self) -> &'static str { "shadowed" } fn sort(&self) -> u32 { 150 } - fn description(&self) -> &str { + fn description(&self) -> &'static str { "Shadowed Var" } - fn documentation(&self) -> &str { + fn documentation(&self) -> &'static str { r"### Example **Incorrect** diff --git a/libs/sqf/src/analyze/lints/s16_not_private.rs b/libs/sqf/src/analyze/lints/s16_not_private.rs index fe305e9e..58fab197 100644 --- a/libs/sqf/src/analyze/lints/s16_not_private.rs +++ b/libs/sqf/src/analyze/lints/s16_not_private.rs @@ -12,16 +12,16 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS16NotPrivate); impl Lint for LintS16NotPrivate { - fn ident(&self) -> &str { + fn ident(&self) -> &'static str { "not_private" } fn sort(&self) -> u32 { 160 } - fn description(&self) -> &str { + fn description(&self) -> &'static str { "Not Private Var" } - fn documentation(&self) -> &str { + fn documentation(&self) -> &'static str { r"### Example **Incorrect** From 756fc91b48e7dd976759cd2fd5f363b7df90b4f0 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Wed, 18 Dec 2024 01:33:10 -0600 Subject: [PATCH 15/16] fmt --- libs/sqf/src/analyze/inspector/commands.rs | 3 ++- libs/sqf/src/analyze/inspector/external_functions.rs | 4 +++- libs/sqf/src/analyze/inspector/game_value.rs | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/sqf/src/analyze/inspector/commands.rs b/libs/sqf/src/analyze/inspector/commands.rs index d955339f..86aefd08 100644 --- a/libs/sqf/src/analyze/inspector/commands.rs +++ b/libs/sqf/src/analyze/inspector/commands.rs @@ -88,7 +88,8 @@ impl SciptScope { for type_p in &arg_array[2] { if let GameValue::Array(Some(type_array)) = type_p { for type_i in type_array { - var_types.extend(type_i.iter().map(GameValue::make_generic)); + var_types + .extend(type_i.iter().map(GameValue::make_generic)); } } } diff --git a/libs/sqf/src/analyze/inspector/external_functions.rs b/libs/sqf/src/analyze/inspector/external_functions.rs index f96d8b8e..37f56f59 100644 --- a/libs/sqf/src/analyze/inspector/external_functions.rs +++ b/libs/sqf/src/analyze/inspector/external_functions.rs @@ -89,7 +89,9 @@ impl SciptScope { } } } - "cba_fnc_addperframehandler" | "cba_fnc_waitandexecute" | "cba_fnc_execnextframe" => { + "cba_fnc_addperframehandler" + | "cba_fnc_waitandexecute" + | "cba_fnc_execnextframe" => { if !gv_array.is_empty() { self.external_new_scope(&gv_array[0], &vec![], database); } diff --git a/libs/sqf/src/analyze/inspector/game_value.rs b/libs/sqf/src/analyze/inspector/game_value.rs index aa3fa805..99b2d254 100644 --- a/libs/sqf/src/analyze/inspector/game_value.rs +++ b/libs/sqf/src/analyze/inspector/game_value.rs @@ -254,8 +254,8 @@ impl GameValue { Self::ForType(_) => Self::ForType(None), Self::Number(_) => Self::Number(None), Self::String(_) => Self::String(None), - _ => self.clone() - } + _ => self.clone(), + } } #[must_use] /// Get as a string for debugging From 894ac8a075db2e93cc5590f7351614abc1afa445 Mon Sep 17 00:00:00 2001 From: PabstMirror Date: Wed, 18 Dec 2024 22:25:48 -0600 Subject: [PATCH 16/16] cleanup merge --- libs/sqf/src/analyze/lints/s12_invalid_args.rs | 10 +++++----- libs/sqf/src/analyze/lints/s13_undefined.rs | 10 +++++----- libs/sqf/src/analyze/lints/s14_unused.rs | 10 +++++----- libs/sqf/src/analyze/lints/s15_shadowed.rs | 10 +++++----- libs/sqf/src/analyze/lints/s16_not_private.rs | 10 +++++----- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/libs/sqf/src/analyze/lints/s12_invalid_args.rs b/libs/sqf/src/analyze/lints/s12_invalid_args.rs index acff86ac..7041b592 100644 --- a/libs/sqf/src/analyze/lints/s12_invalid_args.rs +++ b/libs/sqf/src/analyze/lints/s12_invalid_args.rs @@ -1,5 +1,5 @@ use crate::{ - analyze::{inspector::Issue, SqfLintData}, + analyze::{inspector::Issue, LintData}, Statements, }; use hemtt_common::config::LintConfig; @@ -11,7 +11,7 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS12InvalidArgs); -impl Lint for LintS12InvalidArgs { +impl Lint for LintS12InvalidArgs { fn ident(&self) -> &'static str { "invalid_args" } @@ -36,13 +36,13 @@ Checks correct syntax usage." fn default_config(&self) -> LintConfig { LintConfig::help() } - fn runners(&self) -> Vec>> { + fn runners(&self) -> Vec>> { vec![Box::new(Runner)] } } pub struct Runner; -impl LintRunner for Runner { +impl LintRunner for Runner { type Target = Statements; fn run( &self, @@ -50,7 +50,7 @@ impl LintRunner for Runner { config: &hemtt_common::config::LintConfig, processed: Option<&hemtt_workspace::reporting::Processed>, target: &Statements, - _data: &SqfLintData, + _data: &LintData, ) -> hemtt_workspace::reporting::Codes { if target.issues().is_empty() { return Vec::new(); diff --git a/libs/sqf/src/analyze/lints/s13_undefined.rs b/libs/sqf/src/analyze/lints/s13_undefined.rs index 9ee4691e..ca531b9d 100644 --- a/libs/sqf/src/analyze/lints/s13_undefined.rs +++ b/libs/sqf/src/analyze/lints/s13_undefined.rs @@ -1,5 +1,5 @@ use crate::{ - analyze::{inspector::Issue, SqfLintData}, + analyze::{inspector::Issue, LintData}, Statements, }; use hemtt_common::config::LintConfig; @@ -11,7 +11,7 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS13Undefined); -impl Lint for LintS13Undefined { +impl Lint for LintS13Undefined { fn ident(&self) -> &'static str { "undefined" } @@ -36,13 +36,13 @@ Checks correct syntax usage." fn default_config(&self) -> LintConfig { LintConfig::help() } - fn runners(&self) -> Vec>> { + fn runners(&self) -> Vec>> { vec![Box::new(Runner)] } } pub struct Runner; -impl LintRunner for Runner { +impl LintRunner for Runner { type Target = Statements; fn run( &self, @@ -50,7 +50,7 @@ impl LintRunner for Runner { config: &hemtt_common::config::LintConfig, processed: Option<&hemtt_workspace::reporting::Processed>, target: &Statements, - _data: &SqfLintData, + _data: &LintData, ) -> hemtt_workspace::reporting::Codes { if target.issues().is_empty() { return Vec::new(); diff --git a/libs/sqf/src/analyze/lints/s14_unused.rs b/libs/sqf/src/analyze/lints/s14_unused.rs index 269d67af..9a17be1e 100644 --- a/libs/sqf/src/analyze/lints/s14_unused.rs +++ b/libs/sqf/src/analyze/lints/s14_unused.rs @@ -1,7 +1,7 @@ use crate::{ analyze::{ inspector::{Issue, VarSource}, - SqfLintData, + LintData, }, Statements, }; @@ -14,7 +14,7 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS14Unused); -impl Lint for LintS14Unused { +impl Lint for LintS14Unused { fn ident(&self) -> &'static str { "unused" } @@ -39,13 +39,13 @@ Checks for vars that are never used." fn default_config(&self) -> LintConfig { LintConfig::help().with_enabled(false) } - fn runners(&self) -> Vec>> { + fn runners(&self) -> Vec>> { vec![Box::new(Runner)] } } pub struct Runner; -impl LintRunner for Runner { +impl LintRunner for Runner { type Target = Statements; fn run( &self, @@ -53,7 +53,7 @@ impl LintRunner for Runner { config: &hemtt_common::config::LintConfig, processed: Option<&hemtt_workspace::reporting::Processed>, target: &Statements, - _data: &SqfLintData, + _data: &LintData, ) -> hemtt_workspace::reporting::Codes { if target.issues().is_empty() { return Vec::new(); diff --git a/libs/sqf/src/analyze/lints/s15_shadowed.rs b/libs/sqf/src/analyze/lints/s15_shadowed.rs index eb0b0388..d5370b0a 100644 --- a/libs/sqf/src/analyze/lints/s15_shadowed.rs +++ b/libs/sqf/src/analyze/lints/s15_shadowed.rs @@ -1,5 +1,5 @@ use crate::{ - analyze::{inspector::Issue, SqfLintData}, + analyze::{inspector::Issue, LintData}, Statements, }; use hemtt_common::config::LintConfig; @@ -11,7 +11,7 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS15Shadowed); -impl Lint for LintS15Shadowed { +impl Lint for LintS15Shadowed { fn ident(&self) -> &'static str { "shadowed" } @@ -37,13 +37,13 @@ Checks for variables being shadowed." fn default_config(&self) -> LintConfig { LintConfig::help().with_enabled(false) } - fn runners(&self) -> Vec>> { + fn runners(&self) -> Vec>> { vec![Box::new(Runner)] } } pub struct Runner; -impl LintRunner for Runner { +impl LintRunner for Runner { type Target = Statements; fn run( &self, @@ -51,7 +51,7 @@ impl LintRunner for Runner { config: &hemtt_common::config::LintConfig, processed: Option<&hemtt_workspace::reporting::Processed>, target: &Statements, - _data: &SqfLintData, + _data: &LintData, ) -> hemtt_workspace::reporting::Codes { if target.issues().is_empty() { return Vec::new(); diff --git a/libs/sqf/src/analyze/lints/s16_not_private.rs b/libs/sqf/src/analyze/lints/s16_not_private.rs index 58fab197..2e83481e 100644 --- a/libs/sqf/src/analyze/lints/s16_not_private.rs +++ b/libs/sqf/src/analyze/lints/s16_not_private.rs @@ -1,5 +1,5 @@ use crate::{ - analyze::{inspector::Issue, SqfLintData}, + analyze::{inspector::Issue, LintData}, Statements, }; use hemtt_common::config::LintConfig; @@ -11,7 +11,7 @@ use std::{ops::Range, sync::Arc}; crate::analyze::lint!(LintS16NotPrivate); -impl Lint for LintS16NotPrivate { +impl Lint for LintS16NotPrivate { fn ident(&self) -> &'static str { "not_private" } @@ -36,13 +36,13 @@ Checks local variables that are not private." fn default_config(&self) -> LintConfig { LintConfig::help().with_enabled(false) } - fn runners(&self) -> Vec>> { + fn runners(&self) -> Vec>> { vec![Box::new(Runner)] } } pub struct Runner; -impl LintRunner for Runner { +impl LintRunner for Runner { type Target = Statements; fn run( &self, @@ -50,7 +50,7 @@ impl LintRunner for Runner { config: &hemtt_common::config::LintConfig, processed: Option<&hemtt_workspace::reporting::Processed>, target: &Statements, - _data: &SqfLintData, + _data: &LintData, ) -> hemtt_workspace::reporting::Codes { if target.issues().is_empty() { return Vec::new();