Skip to content

sqf: Inspector reduce false positives inside loops #1169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PabstMirror wants to merge 4 commits into
base: main
Choose a base branch
from
Open

sqf: Inspector reduce false positives inside loops #1169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
22fcb71
sqf: Inspector reduce false positives inside loops
PabstMirror Jan 1, 2026
6e2c1a7
Merge branch 'main' into loopTrialRuns
PabstMirror Jan 31, 2026
24c0b04
rename var
PabstMirror Jan 31, 2026
83bbf20
Update test_iteration.sqf
PabstMirror Jan 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 31 additions & 40 deletions libs/sqf/src/analyze/inspector/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ use indexmap::IndexSet;
use crate::{
Expression, Statement,
analyze::inspector::{InvalidArgs, Issue, VarSource, game_value::NilSource},
parser::database::Database,
};

use super::{Inspector, game_value::GameValue};

type MagicVars<'a> = (&'a [(&'a str, GameValue)], &'a Range<usize>);

impl Inspector {
impl Inspector<'_> {
#[must_use]
pub fn cmd_u_private(&mut self, rhs: &IndexSet<GameValue>) -> IndexSet<GameValue> {
fn push_var(s: &mut Inspector, var: &str, source: &Range<usize>) {
Expand Down Expand Up @@ -104,7 +103,7 @@ impl Inspector {
}
}
if let Some(error) = error_type {
self.errors.insert(Issue::InvalidArgs {
self.error_insert(Issue::InvalidArgs {
command: debug_type.to_string(),
variant: error,
span: source.clone(),
Expand Down Expand Up @@ -205,7 +204,7 @@ impl Inspector {
&mut self,
code_possibilities: &IndexSet<GameValue>,
magic_opt: Option<MagicVars>,
database: &Database,
is_loop: bool,
) -> IndexSet<GameValue> {
let mut return_value = IndexSet::new();
for possible in code_possibilities {
Expand All @@ -217,7 +216,7 @@ impl Inspector {
return_value.insert(GameValue::Anything);
continue;
};
let stack_index = self.stack_push(Some(expression));
let stack_index = self.stack_push(Some(expression), is_loop);
if stack_index.is_none() {
return_value.insert(GameValue::Anything);
continue;
Expand All @@ -232,19 +231,27 @@ impl Inspector {
);
}
}
self.eval_statements(statements, true, database);
self.eval_statements(statements, true);
return_value.extend(self.stack_pop(stack_index));
}
if is_loop {
// repeat the loop, but with error suppression off
return self.cmd_generic_call(code_possibilities, magic_opt, false);
}
Comment on lines +237 to +240
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recursive call to cmd_generic_call will re-execute the loop body, but the variable assignments from the first run remain in the stack. This means variables assigned in the first run (like _oldLoc becoming _locNew) will still be set during the second run, which defeats the purpose of detecting uninitialized variable errors. The variable state should be restored or the stack should be reset between runs.

Copilot uses AI. Check for mistakes.
return_value
}
#[must_use]
pub fn cmd_b_do(
&mut self,
lhs: &IndexSet<GameValue>,
rhs: &IndexSet<GameValue>,
database: &Database,
check_loop: bool,
) -> IndexSet<GameValue> {
let mut return_value = IndexSet::new();
let is_loop = check_loop
&& lhs
.iter()
.any(|p| matches!(p, GameValue::ForType(_)) || matches!(p, GameValue::WhileType));
for possible in rhs {
let GameValue::Code(Some(expression)) = possible else {
return_value.insert(GameValue::Anything);
Expand All @@ -254,7 +261,7 @@ impl Inspector {
return_value.insert(GameValue::Anything);
continue;
};
let stack_index = self.stack_push(Some(expression));
let stack_index = self.stack_push(Some(expression), is_loop);
if stack_index.is_none() {
return_value.insert(GameValue::Anything);
continue;
Expand All @@ -278,7 +285,7 @@ impl Inspector {
}
Expression::Code(stage_statement) => {
self.code_used(stage);
self.eval_statements(stage_statement, false, database);
self.eval_statements(stage_statement, false);
}
_ => {}
}
Expand All @@ -297,10 +304,14 @@ impl Inspector {
} else {
true
};
self.eval_statements(statements, add_final, database);
self.eval_statements(statements, add_final);
}
return_value.extend(self.stack_pop(stack_index));
}
if is_loop {
// repeat the loop, but with error suppression off
return self.cmd_b_do(lhs, rhs, false);
}
Comment on lines +311 to +314
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the issue in cmd_generic_call, this recursive call to cmd_b_do will re-execute the loop body with variable assignments from the first run still present in the stack, which may hide errors that should be detected. The variable state should be managed between the two runs.

Copilot uses AI. Check for mistakes.
return_value
}
#[must_use]
Expand Down Expand Up @@ -350,11 +361,7 @@ impl Inspector {
lhs.clone()
}
#[must_use]
pub fn cmd_u_is_nil(
&mut self,
rhs: &IndexSet<GameValue>,
database: &Database,
) -> IndexSet<GameValue> {
pub fn cmd_u_is_nil(&mut self, rhs: &IndexSet<GameValue>) -> IndexSet<GameValue> {
let mut non_string = false;
for possible in rhs {
let GameValue::String(possible) = possible else {
Expand All @@ -370,7 +377,7 @@ impl Inspector {
let _ = self.var_retrieve(var, &expression.span(), true);
}
if non_string {
let _ = self.cmd_generic_call(rhs, None, database);
let _ = self.cmd_generic_call(rhs, None, false);
}
IndexSet::from([GameValue::Boolean(None)])
}
Expand All @@ -379,15 +386,14 @@ impl Inspector {
&mut self,
rhs: &Expression,
rhs_set: &IndexSet<GameValue>,
database: &Database,
) -> IndexSet<GameValue> {
let mut return_value = IndexSet::new();
for possible in rhs_set {
if let GameValue::Code(Some(Expression::Code(_statements))) = possible {
return_value.extend(self.cmd_generic_call(
&IndexSet::from([possible.clone()]),
None,
database,
false,
));
}
if let GameValue::Array(Some(gv_array), _) = possible {
Expand All @@ -397,7 +403,7 @@ impl Inspector {
return_value.extend(self.cmd_generic_call(
&IndexSet::from([GameValue::Code(Some(expression.clone()))]),
None,
database,
false,
));
}
}
Expand All @@ -420,11 +426,7 @@ impl Inspector {
lhs.iter().chain(rhs.iter()).cloned().collect()
}
#[must_use]
pub fn cmd_b_get_or_default_call(
&mut self,
rhs: &IndexSet<GameValue>,
database: &Database,
) -> IndexSet<GameValue> {
pub fn cmd_b_get_or_default_call(&mut self, rhs: &IndexSet<GameValue>) -> IndexSet<GameValue> {
let mut possible_code = IndexSet::new();
for possible_outer in rhs {
let GameValue::Array(Some(gv_array), _) = possible_outer else {
Expand All @@ -435,7 +437,7 @@ impl Inspector {
}
possible_code.extend(gv_array[1].iter().map(|(gv, _)| gv.clone()));
}
let _ = self.cmd_generic_call(&possible_code, None, database);
let _ = self.cmd_generic_call(&possible_code, None, false);
IndexSet::from([GameValue::Anything])
}
#[must_use]
Expand All @@ -459,15 +461,10 @@ impl Inspector {
rhs: &IndexSet<GameValue>,
cmd_set: &IndexSet<GameValue>,
source: &Range<usize>,
database: &Database,
) -> IndexSet<GameValue> {
let mut return_value = cmd_set.clone();
// Check: `array select expression`
let _ = self.cmd_generic_call(
rhs,
Some((&[("_x", GameValue::Anything)], source)),
database,
);
let _ = self.cmd_generic_call(rhs, Some((&[("_x", GameValue::Anything)], source)), true);
// if lhs is array, and rhs is bool/number then put array into return
if lhs.len() == 1
&& rhs
Expand All @@ -485,13 +482,7 @@ impl Inspector {
return_value
}

pub fn cmd_eqx_count_lint(
&mut self,
lhs: &Expression,
rhs: &Expression,
database: &Database,
equal_zero: bool,
) {
pub fn cmd_eqx_count_lint(&mut self, lhs: &Expression, rhs: &Expression, equal_zero: bool) {
let Expression::Number(float_ord::FloatOrd(0.0), _) = *rhs else {
return;
};
Expand All @@ -503,15 +494,15 @@ impl Inspector {
if lhs_cmd != "count" {
return;
}
let count_input_set = self.eval_expression(count_input, database);
let count_input_set = self.eval_expression(count_input);
if count_input_set.is_empty()
|| !count_input_set
.iter()
.all(|(arr, _)| matches!(arr, GameValue::Array(..)))
{
return;
}
self.errors.insert(Issue::CountArrayComparison(
self.error_insert(Issue::CountArrayComparison(
equal_zero,
lhs.span().start..rhs.span().end,
count_input.source(false),
Expand Down
36 changes: 11 additions & 25 deletions libs/sqf/src/analyze/inspector/external_functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ use std::ops::Range;

use indexmap::IndexSet;

use crate::{Expression, analyze::inspector::VarSource, parser::database::Database};
use crate::{Expression, analyze::inspector::VarSource};

use super::{Inspector, game_value::GameValue};

impl Inspector {
pub fn external_function(
&mut self,
lhs: &IndexSet<GameValue>,
rhs: &Expression,
database: &Database,
) {
impl Inspector<'_> {
pub fn external_function(&mut self, lhs: &IndexSet<GameValue>, rhs: &Expression) {
let Expression::Variable(ext_func, _) = rhs else {
return;
};
Expand All @@ -27,7 +22,6 @@ impl Inspector {
self.external_current_scope(
&vec![(GameValue::Code(Some(statements.clone())), statements.span())],
&vec![],
database,
);
}
}
Expand All @@ -41,7 +35,6 @@ impl Inspector {
("_key", GameValue::Anything),
("_value", GameValue::Anything),
],
database,
);
}
}
Expand All @@ -50,7 +43,6 @@ impl Inspector {
self.external_current_scope(
&gv_array[1],
&vec![("_x", GameValue::Anything)],
database,
);
}
}
Expand All @@ -62,18 +54,17 @@ impl Inspector {
("_x", GameValue::Anything),
("_accumulator", GameValue::Anything),
],
database,
);
}
}
"cba_fnc_directcall" => {
if !gv_array.is_empty() {
self.external_current_scope(&gv_array[0], &vec![], database);
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![], database);
self.external_current_scope(&gv_array[1], &vec![]);
}
}
// Functions that will start in a new scope
Expand All @@ -87,7 +78,6 @@ impl Inspector {
("_player", GameValue::Object),
("_actionParams", GameValue::Anything),
],
database,
);
}
}
Expand All @@ -96,12 +86,12 @@ impl Inspector {
| "cba_fnc_waitandexecute"
| "cba_fnc_execnextframe" => {
if !gv_array.is_empty() {
self.external_new_scope(&gv_array[0], &vec![], database);
self.external_new_scope(&gv_array[0], &vec![]);
}
}
"cba_fnc_addclasseventhandler" => {
if gv_array.len() > 2 {
self.external_new_scope(&gv_array[2], &vec![], database);
self.external_new_scope(&gv_array[2], &vec![]);
}
}
"cba_fnc_addbiseventhandler" => {
Expand All @@ -114,7 +104,6 @@ impl Inspector {
("_thisFnc", GameValue::Code(None)),
("_thisArgs", GameValue::Anything),
],
database,
);
}
}
Expand All @@ -128,7 +117,6 @@ impl Inspector {
("_thisFnc", GameValue::Code(None)),
("_thisArgs", GameValue::Anything),
],
database,
);
}
}
Expand All @@ -142,7 +130,6 @@ impl Inspector {
&mut self,
code_arg: &Vec<(GameValue, Range<usize>)>,
vars: &Vec<(&str, GameValue)>,
database: &Database,
) {
for (element, _) in code_arg {
let GameValue::Code(Some(expression)) = element else {
Expand All @@ -152,7 +139,7 @@ impl Inspector {
continue;
};
self.scope_push(false);
let stack_index = self.stack_push(Some(expression));
let stack_index = self.stack_push(Some(expression), false);
if stack_index.is_some() {
// prevent infinite recursion
for (var, value) in vars {
Expand All @@ -163,7 +150,7 @@ impl Inspector {
VarSource::Ignore,
);
}
self.eval_statements(statements, false, database);
self.eval_statements(statements, false);
let _ = self.stack_pop(stack_index);
}
self.scope_pop();
Expand All @@ -173,7 +160,6 @@ impl Inspector {
&mut self,
code_arg: &Vec<(GameValue, Range<usize>)>,
vars: &Vec<(&str, GameValue)>,
database: &Database,
) {
for (element, _) in code_arg {
let GameValue::Code(Some(expression)) = element else {
Expand All @@ -182,7 +168,7 @@ impl Inspector {
let Expression::Code(statements) = expression else {
continue;
};
let stack_index = self.stack_push(Some(expression));
let stack_index = self.stack_push(Some(expression), false);
if stack_index.is_none() {
continue;
}
Expand All @@ -194,7 +180,7 @@ impl Inspector {
VarSource::Ignore,
);
}
self.eval_statements(statements, true, database);
self.eval_statements(statements, true);
self.stack_pop(stack_index);
}
}
Expand Down
Loading
Loading