From b2718533f9558a45315eaa806389909673491f2d Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Sat, 31 Oct 2026 03:34:33 +0800 Subject: [PATCH] fix(lint): diagnose nested current array element scope Refine the existing nested-binder-lambda warning to require an element or index read in the nested iterator's source or per-element arguments. Keep reads confined to deeper iterator bodies from creating redundant warnings, and explain the unconfirmed client import risk in diagnostics and docs. Cover all iterator pairs, source-only reads, deeper scopes, CLI locations, severity overrides, and rule disablement without changing OPY lowering. Fixes #1032 --- .../wright-analyzer/src/canonical/analysis.rs | 44 ++++++------ crates/wright-analyzer/src/registry.rs | 15 ++-- .../tests/nested_array_scopes.rs | 69 ++++++++++++++++++ .../wright-cli/tests/nested_array_scopes.rs | 72 +++++++++++++++++++ docs/cli/lint.md | 13 +++- 5 files changed, 183 insertions(+), 30 deletions(-) create mode 100644 crates/wright-analyzer/tests/nested_array_scopes.rs create mode 100644 crates/wright-cli/tests/nested_array_scopes.rs diff --git a/crates/wright-analyzer/src/canonical/analysis.rs b/crates/wright-analyzer/src/canonical/analysis.rs index cda80c08..7b911e32 100644 --- a/crates/wright-analyzer/src/canonical/analysis.rs +++ b/crates/wright-analyzer/src/canonical/analysis.rs @@ -380,17 +380,7 @@ const ELEMENT_BINDER_VALUES: &[&str] = &[ "sortedArray", ]; -/// `nested-binder-lambda` (#713): report each binder call that occurs -/// inside an enclosing binder's per-element argument — any argument after -/// the array source at index 0. The inner call rebinds `Current Array -/// Element`/`Current Array Index` inside its own per-element argument, -/// shadowing the enclosing element, and the shape is the one structural -/// signature measured so far that separates client-rejected Workshop -/// builds from accepted ones: the `OWBastion/Bastion#391`-era build and the -/// import-limits verbatim reproducer carry it (1 and 16 occurrences), the -/// measured accepted builds carry none (`workshop-rs` -/// `docs/import-limits.md`). Correlation, not a confirmed client limit — -/// the finding's evidence class says so. +// Client-rejected nested-binding evidence: wrightkit/wright#713 and #1032. fn nested_binder_findings( program: &Program, rule_id: RuleId, @@ -432,10 +422,10 @@ fn nested_binder_findings( code: NESTED_BINDER_LAMBDA.id.into(), severity: NESTED_BINDER_LAMBDA.default_severity, message: format!( - "`{inner}` nested in `{outer}`'s per-element argument binds \ - `Current Array Element` to the inner array, shadowing the \ - outer element; nested-binder shapes appear in Workshop \ - builds the game client rejected on import" + "`{inner}` nested in `{outer}`'s per-element argument reads \ + `Current Array Element`/`Current Array Index` across nested \ + binding scopes; this shape may be rejected by the Workshop \ + client on import (the exact client limit is unconfirmed)" ), span, rule: rule_id, @@ -456,8 +446,10 @@ fn collect_nested_binders<'a>( out: &mut Vec<(Vec, &'a str, &'a Value, &'a str)>, ) { let binder = match value { - Value::Call { name, .. } if ELEMENT_BINDER_VALUES.contains(&name.as_str()) => { - if let Some(outer) = enclosing { + Value::Call { name, args } if ELEMENT_BINDER_VALUES.contains(&name.as_str()) => { + if let Some(outer) = enclosing + && args.iter().any(reads_array_binding) + { out.push((path.clone(), name.as_str(), value, outer)); } Some(name.as_str()) @@ -468,11 +460,7 @@ fn collect_nested_binders<'a>( path.push(index); collect_nested_binders( child, - if index >= 1 { - binder.or(enclosing) - } else { - enclosing - }, + binder.filter(|_| index >= 1).or(enclosing), path, out, ); @@ -480,6 +468,18 @@ fn collect_nested_binders<'a>( } } +fn reads_array_binding(value: &Value) -> bool { + if let Value::Call { name, args } = value { + if matches!(name.as_str(), "currentArrayElement" | "currentArrayIndex") { + return true; + } + if ELEMENT_BINDER_VALUES.contains(&name.as_str()) { + return args.first().is_some_and(reads_array_binding); + } + } + value_children(value).into_iter().any(reads_array_binding) +} + fn duplicate_condition_findings( program: &Program, rule_id: RuleId, diff --git a/crates/wright-analyzer/src/registry.rs b/crates/wright-analyzer/src/registry.rs index dbc119cc..e3cf5a80 100644 --- a/crates/wright-analyzer/src/registry.rs +++ b/crates/wright-analyzer/src/registry.rs @@ -258,17 +258,18 @@ pub(crate) const NESTED_BINDER_LAMBDA: RuleMeta = RuleMeta { id: "nested-binder-lambda", default_severity: Severity::Warning, evidence: EvidenceClass::StaticIndicator, - summary: "element-binding array call nested inside another binder's per-element argument", + summary: "nested element-binding array call reads the current array element or index", rationale: "Surface a Workshop shape observed only in client-rejected builds.", documentation: concat!( "The element-binding values — `filteredArray`, `sortedArray`, ", "`mappedArray`, `isTrueForAll`, `isTrueForAny` — evaluate every ", "argument after the first once per array element, binding ", "`Current Array Element`/`Current Array Index` to that element. ", - "A binder inside that per-element argument creates a nested ", - "binding scope: inside the inner call's own per-element ", - "argument the element values denote the inner array and the ", - "outer element is shadowed. The shape compiles and is ", + "A binder inside that per-element argument that reads either binding ", + "in its array source or its own per-element arguments creates a nested ", + "binding risk. The array source can still read the outer element, ", + "while the inner per-element arguments bind the inner element. ", + "The shape compiles and is ", "semantically defined, but it is the one structural signature ", "measured so far that separates client-rejected Workshop builds ", "from accepted ones: the `OWBastion/Bastion#391`-era build and ", @@ -286,7 +287,9 @@ pub(crate) const NESTED_BINDER_LAMBDA: RuleMeta = RuleMeta { "`Filtered Array(Sorted Array(x, r), c)`, creates no nested ", "scope and is not reported. The rule names a suspect shape, not ", "a measured client boundary: no confirmed import depth or ", - "nesting limit exists yet in workshop-rs.", + "nesting limit exists yet in workshop-rs. Calls that read neither ", + "binding outside a deeper binder's per-element arguments are not ", + "reported. This does not prove that other nested calls import safely.", ), tags: &["correctness", "stability"], }; diff --git a/crates/wright-analyzer/tests/nested_array_scopes.rs b/crates/wright-analyzer/tests/nested_array_scopes.rs new file mode 100644 index 00000000..4dbd500a --- /dev/null +++ b/crates/wright-analyzer/tests/nested_array_scopes.rs @@ -0,0 +1,69 @@ +use workshop_rs::{Event, Program, Rule, Value}; +use wright_analyzer::canonical::analyze; +use wright_analyzer::registry::LintConfig; + +#[test] +fn nested_binders_require_a_read_of_their_array_binding() { + let binders = [ + "filteredArray", + "sortedArray", + "mappedArray", + "isTrueForAny", + "isTrueForAll", + ]; + for outer in binders { + for inner in binders { + for (source, body, expected) in [ + (Value::Array(vec![]), Value::Bool(true), None), + ( + Value::Array(vec![]), + Value::call("currentArrayElement", []), + Some(inner), + ), + ( + Value::Array(vec![]), + Value::call("currentArrayIndex", []), + Some(inner), + ), + ( + Value::Array(vec![Value::call("currentArrayElement", [])]), + Value::Bool(true), + Some(inner), + ), + ( + Value::Array(vec![]), + Value::call( + "isTrueForAny", + [Value::Array(vec![]), Value::call("currentArrayElement", [])], + ), + Some("isTrueForAny"), + ), + ] { + let mut program = Program::new(); + program.rule(Rule::new("nested binding reads", Event::Global).condition( + Value::call( + outer, + [Value::Array(vec![]), Value::call(inner, [source, body])], + ), + )); + let findings = analyze(&program, &LintConfig::default()); + let nested: Vec<_> = findings + .iter() + .filter(|finding| finding.code == "nested-binder-lambda") + .collect(); + assert_eq!( + nested.len(), + usize::from(expected.is_some()), + "{outer}/{inner}: {nested:?}" + ); + if let Some(reported_inner) = expected { + assert!( + nested[0] + .message + .starts_with(&format!("`{reported_inner}` nested")) + ); + } + } + } + } +} diff --git a/crates/wright-cli/tests/nested_array_scopes.rs b/crates/wright-cli/tests/nested_array_scopes.rs new file mode 100644 index 00000000..c0d7bebb --- /dev/null +++ b/crates/wright-cli/tests/nested_array_scopes.rs @@ -0,0 +1,72 @@ +use std::io::Write; +use std::process::{Command, Stdio}; + +#[test] +fn lint_reports_nested_current_array_element_import_risk() { + let source = r#" +variables { + global: + 0: pool +} +rule ("nested any in filter") { + event { + Ongoing - Global; + } + actions { + Set Global Variable(pool, Filtered Array(All Players(Team 1), And(Is Alive(Current Array Element), Is True For Any(Array(Compare(Hero Of(Current Array Element), ==, Hero(Ana)), Is Alive(Current Array Element)), Current Array Element)))); + Set Global Variable(pool, Is True For Any(Array(Is Alive(Event Player)), Current Array Element)); + Set Global Variable(pool, Filtered Array(Sorted Array(All Players(Team 1), Current Array Element), Is Alive(Current Array Element))); + Set Global Variable(pool, Filtered Array(All Players(Team 1), Is True For Any(Array(True), True))); + } +} +"#; + for (flags, exit, severity) in [ + (vec![], 0, Some("warning")), + ( + vec!["--rule-severity", "nested-binder-lambda:error"], + 1, + Some("error"), + ), + (vec!["--disable-rule", "nested-binder-lambda"], 0, None), + ] { + let mut child = Command::new(env!("CARGO_BIN_EXE_wright")) + .args(["lint", "-", "-f", "json"]) + .args(flags) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(source.as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.code(), Some(exit), "{output:?}"); + let envelope: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let findings: Vec<_> = envelope["result"]["findings"] + .as_array() + .unwrap() + .iter() + .filter(|finding| finding["code"] == "nested-binder-lambda") + .collect(); + assert_eq!(findings.len(), usize::from(severity.is_some())); + if let Some(severity) = severity { + assert_eq!(findings[0]["severity"], severity); + assert_eq!(findings[0]["evidence"], "static-indicator"); + let message = findings[0]["message"].as_str().unwrap(); + assert!(message.contains("isTrueForAny")); + assert!(message.contains("filteredArray")); + assert!(message.contains("Current Array Element")); + assert!(message.contains("unconfirmed")); + assert!( + findings[0]["location"] + .as_str() + .unwrap() + .starts_with(":11:") + ); + } + } +} diff --git a/docs/cli/lint.md b/docs/cli/lint.md index ce3c7f4f..e4765ff1 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -93,8 +93,17 @@ scopes. The binder values (`filteredArray`, `sortedArray`, `mappedArray`, `isTrueForAll`, `isTrueForAny`) evaluate every argument after the first per array element, binding `Current Array Element`/`Current Array Index`; a binder nested inside that per-element argument shadows the outer element -inside the inner binder's own argument. The shape is semantically valid and -may be intentional, but it is the structural signature measured on +inside the inner binder's own argument. The warning requires a read of +`Current Array Element` or `Current Array Index` in the inner call's array +source or its own per-element arguments. In particular, an `any([...])` or +`all([...])` inside an OPY `filter(lambda p: ...)` can build its input array +from the outer element, then rebind the element in its predicate. Calls +without either read, reads confined to a deeper binder's per-element +arguments, and binders chained through array-source arguments do not trigger +this rule. Silence does not establish client importability. + +The shape is semantically valid and may be intentional, but it is the +structural signature measured on client-rejected Workshop builds and absent from measured accepted ones (`workshop-rs` `docs/import-limits.md`; #713). It is a warning, not a confirmed client import boundary.