diff --git a/crates/scriptcheck/src/lib.rs b/crates/scriptcheck/src/lib.rs index b9d9173fc..0c62419f1 100644 --- a/crates/scriptcheck/src/lib.rs +++ b/crates/scriptcheck/src/lib.rs @@ -15,8 +15,10 @@ //! //! The checker never recurses: it reads each file once, and each scan of a file goes through its //! tokens front to back, first for the names its functions and its global code declare and then -//! for the analysis. When it follows includes, it scans each file for the includes and what the -//! variables hold, again while that grows (at most three more times), and then for the report. It +//! for the analysis. It scans the script for the includes and what the variables hold, and each +//! file it includes when it follows them, again while that grows (at most three more times), and +//! then for the report; a scan that finds nothing more is the report scan of a script read without +//! its included files. It //! keeps open brackets and the levels of an XML literal on capped stacks. Its work is linear in the //! input's size, and no input can overflow the stack. #![forbid(unsafe_code)] @@ -146,7 +148,8 @@ pub use report::PROBLEMS; /// `loader`, the files its `#include` lines name are read through it and checked too; without one /// they are listed as [`IncludeStatus::NotRead`]. When the script or one of those files assigns a /// member chain or a File object to a variable, the check treats the variable as holding it in -/// every file. Nothing in the script runs. +/// every file, also in code before the assignment. A script that includes nothing gets the same +/// report with a loader and without one. Nothing in the script runs. pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: &Limits) -> Report { let mut acc = report::Acc::new(limits); if bytes.len() > limits.file_bytes { @@ -169,6 +172,7 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: // With a loader, this first scan finds the includes and what the script's variables hold; its // tallies are left out. let mut tallies = report::Acc::new(limits); + let pre = acc.clone(); let scan = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &none, if loader.is_some() { &mut tallies } else { &mut acc }); acc.steps = acc.steps.saturating_add(tallies.steps); if scan.module { @@ -179,6 +183,7 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: return acc.finish(Kind::Module, scan.lines); } let Some(loader) = loader else { + let mut acc = settle(acc, pre, &text, &declared, scan.known); includes::not_read(&mut acc, scan.includes); return acc.finish(Kind::Script, scan.lines); }; @@ -215,6 +220,40 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: acc.finish(Kind::Script, lines) } +/// The report of a script read without its included files: what the scans above find for a script +/// that includes nothing, so a variable a function uses before the global code assigns it holds +/// what it is assigned. `first` holds the scan made with nothing known, which found `known`, and +/// `pre` what came before it. A scan made with what the scans before it found, and that finds +/// nothing more, is the report scan; no other scan is repeated. +fn settle<'l>( + first: report::Acc<'l>, + pre: report::Acc<'l>, + text: &str, + declared: &analyze::Declared<'_>, + mut known: analyze::Known, +) -> report::Acc<'l> { + if known.len() == 0 { + return first; + } + let entries = pre.limits.entries; + let mut spent = first.steps.saturating_sub(pre.steps); + for _ in 0..3 { + let before = known.len(); + let mut acc = pre.clone(); + let more = analyze::scan(text, declared, None, std::rc::Rc::from([]), &known, &mut acc).known; + known.add(more, entries); + if known.len() == before { + acc.steps = acc.steps.saturating_add(spent); + return acc; + } + spent = spent.saturating_add(acc.steps.saturating_sub(pre.steps)); + } + let mut acc = pre; + analyze::scan(text, declared, None, std::rc::Rc::from([]), &known, &mut acc); + acc.steps = acc.steps.saturating_add(spent); + acc +} + /// The last part of a path (`lib/Tidy.jsx` → `Tidy.jsx`), whichever separator it uses. pub(crate) fn base_name(name: &str) -> &str { name.rsplit(['/', '\\']).next().unwrap_or(name) diff --git a/crates/scriptcheck/src/report.rs b/crates/scriptcheck/src/report.rs index 975ef4118..1c55ce1b4 100644 --- a/crates/scriptcheck/src/report.rs +++ b/crates/scriptcheck/src/report.rs @@ -253,6 +253,7 @@ impl Tally { } /// What a check has found so far, across the script and its included files. +#[derive(Clone)] pub(crate) struct Acc<'l> { pub limits: &'l Limits, pub steps: usize, diff --git a/crates/scriptcheck/src/tests.rs b/crates/scriptcheck/src/tests.rs index 9f08b94ea..21025a6e9 100644 --- a/crates/scriptcheck/src/tests.rs +++ b/crates/scriptcheck/src/tests.rs @@ -1757,3 +1757,63 @@ proptest! { prop_assert!(r.includes.iter().all(|i| i.status != IncludeStatus::NotRead)); } } + +/// A function that writes to a variable the global code assigns a File object after it: the +/// variable declaration is hoisted, so the function writes a file, with a loader and without one. +#[test] +fn a_variable_assigned_after_the_function_using_it_holds_its_value() { + let src = "function g(){ f.write('x'); } var f = new File('a');"; + let without = report(src); + let with = check("test.jsx", src.as_bytes(), Some(&mut mem(&[])), &Limits::default()); + assert!(feature(&without, "file.write").is_some(), "{:?}", without.features); + assert!(feature(&with, "file.write").is_some(), "{:?}", with.features); +} + +/// Script-like text with functions that use variables the code after them assigns, a chain of them +/// taking more than one scan to settle. +fn hoisting_text() -> impl Strategy { + let pieces = prop::sample::select(vec![ + "function g(){ ", + "function h(){ ", + "}", + "f.write('x'); ", + "b.remove(); ", + "e.acmeItems.add(); ", + "b = a; ", + "var a = f; ", + "var f = new File('a'); ", + "var d = app.activeDocument; ", + "var e = d; ", + "e = d.acmeLayers; ", + "f = d; ", + ]); + (prop::collection::vec(pieces, 0..30), script_text()).prop_map(|(v, tail)| v.concat() + &tail) +} + +proptest! { + #![proptest_config(ProptestConfig::with_cases(512))] + + /// A script that includes nothing gets the same report with a loader and without one. + #[test] + fn a_script_without_includes_reports_the_same_with_and_without_a_loader(text in hoisting_text()) { + let mut without = report(&text); + prop_assume!(without.includes.is_empty()); + let mut with = check("test.jsx", text.as_bytes(), Some(&mut mem(&[])), &Limits::default()); + without.steps = 0; + with.steps = 0; + prop_assert_eq!(without, with); + } +} + +/// Chains of variables a function assigns before the code after it assigns their source take a +/// scan each to settle: the script is read at most seven times. +#[test] +fn variables_that_settle_over_several_scans_stay_linear() { + let mut src = String::new(); + for i in 0..150 { + src += &format!("function a{i}(){{ e{i} = d{i}; }} function b{i}(){{ d{i} = c{i}; }} function c{i}(){{ c{i} = b{i}; }} "); + src += &format!("function d{i}(){{ b{i} = a{i}; }} var a{i} = new File('x');\n"); + } + let r = report(&src); + assert!(r.steps <= 9 * src.len() + 16, "{} steps for {} bytes", r.steps, src.len()); +}