mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-24 09:46:59 -06:00
fix(scriptcheck): settle what variables hold without a loader too
Without a loader, check() scanned the script once, so a function using a
variable the global code assigns after it (`function g(){ f.write('x'); }
var f = new File('a');`) missed file.write, while the loader path found it.
The no-loader path now runs the same fix-point, keeping a scan that finds
nothing new as the report scan, so a script that includes nothing gets the
same report with and without a loader.
Fixes #4011
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
bflatastic
co-authored by
Claude Opus 5.5
parent
4e9cd60540
commit
aa8adf0891
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<Value = String> {
|
||||
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());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user