From d87207ec0fb746ea4bc21f6f1cc742388992f465 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:06:22 +0000 Subject: [PATCH 01/12] ci(deps): bump Swatinem/rust-cache Bumps the actions group with 1 update: [Swatinem/rust-cache](https://github.com/swatinem/rust-cache). Updates `Swatinem/rust-cache` from 2.9.2 to 2.9.3 - [Release notes](https://github.com/swatinem/rust-cache/releases) - [Changelog](https://github.com/Swatinem/rust-cache/blob/master/CHANGELOG.md) - [Commits](https://github.com/swatinem/rust-cache/compare/6323deb102c322ba6fcbdcafc7e3dddab59af2b6...d1527c8f67ef9af357290475d4af4b0fab8f12e0) --- updated-dependencies: - dependency-name: Swatinem/rust-cache dependency-version: 2.9.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] --- .github/workflows/print.yml | 2 +- .github/workflows/windows7.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/print.yml b/.github/workflows/print.yml index 344eae015..794c9affc 100644 --- a/.github/workflows/print.yml +++ b/.github/workflows/print.yml @@ -26,7 +26,7 @@ jobs: with: toolchain: stable components: clippy - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3 with: key: desktop-print - run: cargo clippy --locked -p vectorcraft --all-targets -- -D warnings diff --git a/.github/workflows/windows7.yml b/.github/workflows/windows7.yml index 0d252f11e..7aa11a54b 100644 --- a/.github/workflows/windows7.yml +++ b/.github/workflows/windows7.yml @@ -41,7 +41,7 @@ jobs: with: toolchain: nightly-2026-10-01 components: rust-src - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3 with: key: windows7-x64 - name: Build, check and test the portable package From 41c26c69972508d178467d114ccc1e1472196a42 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:42:29 +0200 Subject: [PATCH 02/12] Keep the denominator of DXF MTEXT stacked fractions written with ^ Found while testing crates/cad/src/import/text.rs:14-27 (mtext calls escapes at :14, which treats `^2` as a caret escape and drops it; the `\S` branch at :25 expects the `^`) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/cad/src/import/text.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/cad/src/import/text.rs b/crates/cad/src/import/text.rs index 8800aa270..5d2797d61 100644 --- a/crates/cad/src/import/text.rs +++ b/crates/cad/src/import/text.rs @@ -4,13 +4,13 @@ /// A single-line text value (TEXT, ATTRIB) as plain text. pub(crate) fn plain(s: &str) -> String { - percent_codes(&escapes(s)) + percent_codes(&escapes(s, false)) } /// A multiline text value (MTEXT) as plain text with `\n` between paragraphs, and the font family /// its first font change names. pub(crate) fn mtext(s: &str) -> (String, Option) { - let s = escapes(s); + let s = escapes(s, true); let mut out = String::with_capacity(s.len()); let mut family = None; let mut chars = s.chars().peekable(); @@ -25,7 +25,7 @@ pub(crate) fn mtext(s: &str) -> (String, Option) { Some('S') => { // A stacked fraction `\Snum^den;` (or / or #): written as num/den. let arg = until_semicolon(&mut chars); - out.push_str(&arg.replace(['^', '#'], "/")); + out.push_str(&arg.replace("^ ", "^").replace(['^', '#'], "/")); } Some(f @ ('f' | 'F')) => { let arg = until_semicolon(&mut chars); @@ -63,8 +63,9 @@ fn until_semicolon(chars: &mut std::iter::Peekable>) -> Stri } /// `\U+XXXX` Unicode escapes (how files before DXF 2007 hold characters outside their code page) -/// and caret escapes: `^ ` is a caret, `^I` a tab; other control characters are dropped. -fn escapes(s: &str) -> String { +/// and caret escapes: `^ ` is a caret, `^I` a tab; other control characters are dropped. With +/// `keep_stacks`, the `^` in an MTEXT stacked fraction `\Snum^den;` is left for the caller. +fn escapes(s: &str, keep_stacks: bool) -> String { let mut out = String::with_capacity(s.len()); let mut rest = s; while let Some(c) = rest.chars().next() { @@ -78,6 +79,16 @@ fn escapes(s: &str) -> String { rest = after.get(6..).unwrap_or(""); continue; } + if keep_stacks + && c == '\\' + && let Some(tail) = after.strip_prefix('S') + { + let end = tail.find(';').map_or(tail.len(), |i| i + 1); + out.push_str("\\S"); + out.push_str(tail.get(..end).unwrap_or("")); + rest = tail.get(end..).unwrap_or(""); + continue; + } if c == '^' && let Some(next) = after.chars().next() { @@ -161,6 +172,8 @@ mod tests { // Carets are written "^ " (a caret before another character is a control character). assert_eq!(mtext("x\\S3^ 4;").0, "x3/4"); assert_eq!(mtext("x\\S3#4;").0, "x3/4"); + assert_eq!(mtext("\\S1^2;").0, "1/2"); + assert_eq!(mtext("\\S+1^-2;^I").0, "+1/-2\t"); assert_eq!(mtext("trailing\\").0, "trailing\\"); } } From f27b1e74e6b5d0a585e9e2d0e7723fc7dce3e9a2 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:10:38 +0200 Subject: [PATCH 03/12] perf(scriptcheck): lex each file once, fewer copies Work out the unescaped text and the declared names of each file once and share them between the scans, keep included files as bytes and decode them on demand, check the cheap tokens before unquoting in the '=' case, and stop allocating a String per recorded chain. Fixes #4010 Fixes #4012 Co-Authored-By: Claude Sonnet 5.5 --- crates/scriptcheck/src/analyze.rs | 52 +++++++++++++++++------------- crates/scriptcheck/src/includes.rs | 16 ++++----- crates/scriptcheck/src/lib.rs | 22 ++++++++----- crates/scriptcheck/src/report.rs | 49 ++++++++++++++++++---------- crates/scriptcheck/src/tests.rs | 3 +- 5 files changed, 85 insertions(+), 57 deletions(-) diff --git a/crates/scriptcheck/src/analyze.rs b/crates/scriptcheck/src/analyze.rs index 965be88db..dffaae9bc 100644 --- a/crates/scriptcheck/src/analyze.rs +++ b/crates/scriptcheck/src/analyze.rs @@ -19,6 +19,7 @@ //! operand read so far at its depth, and a call on a File or Folder object in them (`(f).remove()`) //! is a call on the object. Nothing recurses. +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::rc::Rc; @@ -76,14 +77,11 @@ pub(crate) struct Scan { pub known: Known, } -/// Scan `src`: the script itself (`file` none) or a file it includes (`file`: its path inside the -/// script's folder), with the `#includepath` folders `paths` in force at its start and its variables -/// holding what `known` says. -pub(crate) fn scan(src: &str, file: Option<&str>, paths: Rc<[String]>, known: &Known, acc: &mut Acc) -> Scan { - // A name written with `\u` escapes is read as the name it spells. - let unescaped = unescape_names(src, acc); - let src = unescaped.as_deref().unwrap_or(src); - let declared = declarations(src, acc); +/// Scan `src` (already through [`unescaped`]): the script itself (`file` none) or a file it includes +/// (`file`: its path inside the script's folder), with the names it declares (`declared`, from +/// [`declarations`] on the same `src`), the `#includepath` folders `paths` in force at its start and +/// its variables holding what `known` says. +pub(crate) fn scan<'s>(src: &'s str, declared: &Declared<'s>, file: Option<&str>, paths: Rc<[String]>, known: &Known, acc: &mut Acc) -> Scan { let mut lexer = Lexer::new(src, acc.limits.nesting); let mut a = Analyzer::new(acc, file, paths, known, declared); let mut nl = false; @@ -119,8 +117,15 @@ pub(crate) fn scan(src: &str, file: Option<&str>, paths: Rc<[String]>, known: &K } /// `src` with the `\uXXXX` escapes in its names written as the characters they stand for (ECMA-262 -/// 3rd edition, section 7.6), or none when no name has one. An escape for a character no name may -/// hold stays as written. Line breaks don't change, so every line keeps its number. +/// 3rd edition, section 7.6), and `src` itself when no name has one. An escape for a character no +/// name may hold stays as written. Line breaks don't change, so every line keeps its number. +pub(crate) fn unescaped<'a>(src: Cow<'a, str>, acc: &mut Acc) -> Cow<'a, str> { + match unescape_names(&src, acc) { + Some(out) => Cow::Owned(out), + None => src, + } +} + fn unescape_names(src: &str, acc: &mut Acc) -> Option { if !src.contains("\\u") { return None; @@ -654,7 +659,7 @@ impl Brackets { /// `var` statements are not read here. A function declaration inside a block, which ECMAScript 3 /// does not allow (sections 12.1 and 14), makes its name the script's own from where it stands. #[derive(Default)] -struct Declared<'s> { +pub(crate) struct Declared<'s> { /// For each function body, by the offset of its `{` in the text, the names it declares, in the /// order the bodies start. bodies: Vec<(usize, Vec<&'s str>)>, @@ -695,7 +700,7 @@ struct Declarations<'s, 'a, 'l> { } /// What `src` declares ([`Declared`]), read in one pass over its tokens. -fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { +pub(crate) fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { let mut lexer = Lexer::new(src, acc.limits.nesting); let mut d = Declarations { acc, @@ -864,7 +869,7 @@ struct Analyzer<'s, 'a, 'l> { /// The function bodies being read, the innermost last, up to [`crate::Limits::nesting`]. scopes: Vec, /// The names each function body and the global code declare, read before the analysis. - declared: Declared<'s>, + declared: &'a Declared<'s>, /// The first entry of `declared.bodies` whose body's `{` hasn't been read yet. next_body: usize, /// `function` was read at this bracket depth, and its body's `{` is still to come. @@ -912,7 +917,7 @@ struct Analyzer<'s, 'a, 'l> { } impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { - fn new(acc: &'a mut Acc<'l>, file: Option<&'a str>, paths: Rc<[String]>, known: &Known, declared: Declared<'s>) -> Self { + fn new(acc: &'a mut Acc<'l>, file: Option<&'a str>, paths: Rc<[String]>, known: &Known, declared: &'a Declared<'s>) -> Self { let global: Vec<&'s str> = declared.global.iter().copied().collect(); let mut a = Analyzer { acc, @@ -1181,15 +1186,16 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } } - /// The names [`Declared`] has for the function body whose `{` starts at `start`, taken once. - fn declared_in(&mut self, start: usize) -> Vec<&'s str> { - let bodies = &mut self.declared.bodies; + /// The names [`Declared`] has for the function body whose `{` starts at `start`. + fn declared_in(&mut self, start: usize) -> &'a [&'s str] { + let declared = self.declared; + let bodies = &declared.bodies; while bodies.get(self.next_body).is_some_and(|(at, _)| *at < start) { self.next_body += 1; } - match bodies.get_mut(self.next_body) { - Some((at, names)) if *at == start => std::mem::take(names), - _ => vec![], + match bodies.get(self.next_body) { + Some((at, names)) if *at == start => names, + _ => &[], } } @@ -1356,7 +1362,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { self.keep_outer(&name); self.unbind(&name); } - for name in self.declared_in(t.start) { + for &name in self.declared_in(t.start) { self.acc.steps = self.acc.steps.saturating_add(1); self.hoist(name); } @@ -1929,8 +1935,8 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { "(" => self.call(line), // `Name.prototype["+"] = …` defines an operator. "=" => { - let operator = h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); - if is(h0, "]") && operator && is(h2, "[") && word(h3, "prototype") { + let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); + if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { self.acc.feature("operatorOverloading", line); } } diff --git a/crates/scriptcheck/src/includes.rs b/crates/scriptcheck/src/includes.rs index f47a83955..f6e9c82b0 100644 --- a/crates/scriptcheck/src/includes.rs +++ b/crates/scriptcheck/src/includes.rs @@ -20,13 +20,11 @@ use crate::analyze::{self, IncludeRef, Known}; use crate::directives::MAX_PATH; use crate::report::{Acc, IncludeStatus}; -/// An included file that was read and is checked: its path inside the script's folder, its text -/// (and whether it held bytes that aren't UTF-8), and the `#includepath` folders in force at its -/// start. +/// An included file that was read and is checked: its path inside the script's folder, its bytes, +/// and the `#includepath` folders in force at its start. pub(crate) struct ReadFile { pub path: String, - pub text: String, - pub invalid: bool, + pub bytes: Vec, pub paths: Rc<[String]>, } @@ -141,12 +139,14 @@ pub(crate) fn follow( files.push(path.clone()); let file = files.len() - 1; read.insert(path.clone(), (status, Some(file))); - let (text, invalid) = crate::decode(&b); + let (text, _) = crate::decode(&b); let mut tallies = Acc::new(limits); - let scan = analyze::scan(&text, Some(&path), Rc::clone(&inc.paths), known, &mut tallies); + let text = analyze::unescaped(text, &mut tallies); + let declared = analyze::declarations(&text, &mut tallies); + let scan = analyze::scan(&text, &declared, Some(&path), Rc::clone(&inc.paths), known, &mut tallies); acc.steps = acc.steps.saturating_add(tallies.steps); known.add(scan.known, limits.entries); - checked.push(ReadFile { path: path.clone(), text: text.into_owned(), invalid, paths: Rc::clone(&inc.paths) }); + checked.push(ReadFile { path: path.clone(), bytes: b, paths: Rc::clone(&inc.paths) }); work.push_back(Work { file, includes: scan.includes, depth: w.depth + 1 }); (found, to) = (Some(path), Some(file)); break; diff --git a/crates/scriptcheck/src/lib.rs b/crates/scriptcheck/src/lib.rs index f44c5d759..818949f05 100644 --- a/crates/scriptcheck/src/lib.rs +++ b/crates/scriptcheck/src/lib.rs @@ -165,11 +165,13 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: if invalid { acc.problem(report::NOT_UTF8, None, None, None); } + let text = analyze::unescaped(text, &mut acc); + let declared = analyze::declarations(&text, &mut acc); let none = analyze::Known::default(); // 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 scan = analyze::scan(&text, None, std::rc::Rc::from([]), &none, if loader.is_some() { &mut tallies } else { &mut acc }); + 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 { // A module's report holds its kind and its lines, and none of what the scan found. @@ -184,15 +186,19 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: }; let mut known = scan.known; let read = includes::follow(&mut acc, scan.includes, loader, name, bytes.len(), &mut known); + // Each file's text and the names it declares are worked out once for all the scans below. + let decoded: Vec<_> = read.iter().map(|file| decode(&file.bytes)).collect(); + let texts: Vec<_> = decoded.into_iter().map(|(text, invalid)| (analyze::unescaped(text, &mut acc), invalid)).collect(); + let declarations: Vec<_> = texts.iter().map(|(text, _)| analyze::declarations(text, &mut acc)).collect(); // A variable may get what it holds from another file's variable: the files are scanned again, // each with what is known so far, while that grows, at most three more times. for _ in 0..3 { let before = known.len(); let mut tallies = report::Acc::new(limits); - let more = analyze::scan(&text, None, std::rc::Rc::from([]), &known, &mut tallies).known; + let more = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &known, &mut tallies).known; known.add(more, limits.entries); - for file in &read { - let more = analyze::scan(&file.text, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut tallies).known; + for ((file, (text, _)), declared) in read.iter().zip(&texts).zip(&declarations) { + let more = analyze::scan(text, declared, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut tallies).known; known.add(more, limits.entries); } acc.steps = acc.steps.saturating_add(tallies.steps); @@ -201,12 +207,12 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: } } // The scans for the report: every file starts with what any file's variables hold. - let lines = analyze::scan(&text, None, std::rc::Rc::from([]), &known, &mut acc).lines; - for file in read { - if file.invalid { + let lines = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &known, &mut acc).lines; + for ((file, (text, invalid)), declared) in read.iter().zip(&texts).zip(&declarations) { + if *invalid { acc.problem(report::NOT_UTF8, None, Some(&file.path), None); } - analyze::scan(&file.text, Some(&file.path), file.paths, &known, &mut acc); + analyze::scan(text, declared, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut acc); } acc.finish(Kind::Script, lines) } diff --git a/crates/scriptcheck/src/report.rs b/crates/scriptcheck/src/report.rs index 1820db7cf..e24e12114 100644 --- a/crates/scriptcheck/src/report.rs +++ b/crates/scriptcheck/src/report.rs @@ -1,5 +1,6 @@ //! What a check finds ([`Report`]), and the tallies it is built from ([`Acc`]), each list capped. +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use crate::{Kind, Limits, Need, catalog}; @@ -265,6 +266,11 @@ impl<'l> Acc<'l> { echo(s, self.limits.echo) } + /// [`Acc::echo`] without a copy when `s` isn't cut. + fn echoed<'a>(&self, s: &'a str) -> Cow<'a, str> { + echo_cow(s, self.limits.echo) + } + /// Room for one more entry in a list of the report, or in a table of names the check keeps, of /// `len` entries: false (and the report marked truncated) when it is full. pub fn room(&mut self, len: usize) -> bool { @@ -290,12 +296,13 @@ impl<'l> Acc<'l> { } pub fn menu_string(&mut self, value: &str, line: Option) { - let key = self.echo(value); - if !self.menu.contains_key(&key) && !self.room(self.menu.len()) { - return; - } + let key = self.echoed(value); let max = self.limits.lines; - self.menu.entry(key).or_default().add(line, max); + if let Some(tally) = self.menu.get_mut(&*key) { + tally.add(line, max); + } else if self.room(self.menu.len()) { + self.menu.entry(key.into_owned()).or_default().add(line, max); + } } pub fn menu_dynamic(&mut self) { @@ -303,25 +310,28 @@ impl<'l> Acc<'l> { } pub fn member(&mut self, path: &str) { - if !self.members.contains_key(path) && !self.room(self.members.len()) { - return; + if let Some(n) = self.members.get_mut(path) { + *n = n.saturating_add(1); + } else if self.room(self.members.len()) { + self.members.insert(path.to_string(), 1); } - let n = self.members.entry(path.to_string()).or_default(); - *n = n.saturating_add(1); } pub fn type_name(&mut self, name: &str) { - let key = self.echo(name); - if !self.types.contains_key(&key) && !self.room(self.types.len()) { - return; + let key = self.echoed(name); + if let Some(n) = self.types.get_mut(&*key) { + *n = n.saturating_add(1); + } else if self.room(self.types.len()) { + self.types.insert(key.into_owned(), 1); } - let n = self.types.entry(key).or_default(); - *n = n.saturating_add(1); } pub fn declared(&mut self, name: &str) { if self.declared.len() < self.limits.entries { - self.declared.insert(self.echo(name)); + let name = self.echoed(name); + if !self.declared.contains(&*name) { + self.declared.insert(name.into_owned()); + } } } @@ -405,13 +415,18 @@ impl<'l> Acc<'l> { /// `s` cut to at most `max` characters, the last one `…` when it was cut. pub(crate) fn echo(s: &str, max: usize) -> String { + echo_cow(s, max).into_owned() +} + +/// [`echo`], borrowing `s` when it isn't cut. +fn echo_cow(s: &str, max: usize) -> Cow<'_, str> { match s.char_indices().nth(max) { - None => s.to_string(), + None => Cow::Borrowed(s), Some(_) => { let keep = s.char_indices().nth(max.saturating_sub(1)).map_or(0, |(i, _)| i); let mut out = s.get(..keep).unwrap_or_default().to_string(); out.push('…'); - out + Cow::Owned(out) } } } diff --git a/crates/scriptcheck/src/tests.rs b/crates/scriptcheck/src/tests.rs index 3884459d7..6934a9a9d 100644 --- a/crates/scriptcheck/src/tests.rs +++ b/crates/scriptcheck/src/tests.rs @@ -1475,7 +1475,8 @@ fn include_lines_share_the_folders_of_the_include_path_before_them() { } let limits = Limits::default(); let mut acc = report::Acc::new(&limits); - let scan = analyze::scan(&src, None, std::rc::Rc::from([]), &analyze::Known::default(), &mut acc); + let declared = analyze::declarations(&src, &mut acc); + let scan = analyze::scan(&src, &declared, None, std::rc::Rc::from([]), &analyze::Known::default(), &mut acc); let (first, rest) = scan.includes.split_first().unwrap(); assert_eq!((first.paths.len(), rest.len()), (64, 999)); assert!(rest.iter().all(|i| std::rc::Rc::ptr_eq(&i.paths, &first.paths)), "one list of folders, not a copy for each line"); From 4d2b628b9ece60f9ccb0e3b9ec706f9028b93985 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:34:57 +0200 Subject: [PATCH 04/12] refactor(scriptcheck): feature and outside ids as enums FeatureId and OutsideId replace the string ids the analyzer passed around. One table each gives the id, the need and the report order, so catalog::FEATURES and catalog::need() are gone. Feature.id and the engine JSON are unchanged; OUTSIDE is generated from the same table. Fixes #4013 Co-Authored-By: Claude Sonnet 5.5 --- crates/scriptcheck/src/analyze.rs | 118 ++++++++++++++-------------- crates/scriptcheck/src/catalog.rs | 125 +++++++++++++++--------------- crates/scriptcheck/src/lib.rs | 2 - crates/scriptcheck/src/report.rs | 60 ++++++++++---- crates/scriptcheck/src/tests.rs | 21 +++-- 5 files changed, 182 insertions(+), 144 deletions(-) diff --git a/crates/scriptcheck/src/analyze.rs b/crates/scriptcheck/src/analyze.rs index dffaae9bc..aa2119e43 100644 --- a/crates/scriptcheck/src/analyze.rs +++ b/crates/scriptcheck/src/analyze.rs @@ -23,10 +23,10 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::rc::Rc; -use crate::catalog; +use crate::catalog::{self, FeatureId}; use crate::directives; use crate::lex::{Lexer, T, Token, expr_after}; -use crate::report::{self, Acc}; +use crate::report::{self, Acc, OutsideId}; /// An `#include` line: what it names, its line, and the folders `#includepath` named at that point /// (one list, shared by every line it applies to). @@ -361,14 +361,14 @@ fn platform_name(n: &str) -> bool { } /// The feature of a window of type `value` (`"dialog"`, or a resource string that starts with it). -fn window_type(value: &str) -> &'static str { +fn window_type(value: &str) -> FeatureId { let word = value.trim_start().split(|c: char| c.is_whitespace() || c == '{').next().unwrap_or_default(); if word.eq_ignore_ascii_case("dialog") { - "scriptui.dialog" + FeatureId::ScriptuiDialog } else if word.eq_ignore_ascii_case("palette") { - "scriptui.palette" + FeatureId::ScriptuiPalette } else { - "scriptui.window" + FeatureId::ScriptuiWindow } } @@ -1101,13 +1101,13 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { self.acc.member(&c.path); } if c.path.strip_prefix("app.preferences").is_some_and(then) { - self.acc.feature("preferences", line); + self.acc.feature(FeatureId::Preferences, line); if c.change_call || ending.is_some_and(|t| t.is("=")) { - self.acc.outside("preferences", line, None); + self.acc.outside(OutsideId::Preferences, line, None); } } if ["app.name", "app.version"].iter().any(|p| c.path.strip_prefix(p).is_some_and(then)) { - self.acc.feature("hostCheck", line); + self.acc.feature(FeatureId::HostCheck, line); } } // A chain that an operand in a grouped value's parentheses starts with is complete: the @@ -1751,11 +1751,11 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { if t.is("(") { self.pending = Pending::WindowType(l); } else { - self.acc.feature("scriptui.window", l); + self.acc.feature(FeatureId::ScriptuiWindow, l); } } Pending::WindowType(l) => { - let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { "scriptui.window" }; + let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { FeatureId::ScriptuiWindow }; self.acc.feature(feature, l); } Pending::Menu(l) => { @@ -1776,7 +1776,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { T::Ident => self.name(t, line), T::Punct => self.punct(t, line), T::Str => self.string(t, line), - T::Xml => self.acc.feature("e4x", line), + T::Xml => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1790,15 +1790,15 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } None => self.acc.menu_dynamic(), } - self.acc.feature("menuCommand", line); + self.acc.feature(FeatureId::MenuCommand, line); } fn open_mode(&mut self, mode: &str, line: Option) { if mode.eq_ignore_ascii_case("r") { - self.acc.feature("file.read", line); + self.acc.feature(FeatureId::FileRead, line); } else if let Some(m) = catalog::WRITE_MODES.iter().copied().find(|m| m.eq_ignore_ascii_case(mode)) { - self.acc.feature("file.write", line); - self.acc.outside("filesWritten", line, Some(m)); + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, Some(m)); } } @@ -1810,7 +1810,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { pending = std::mem::take(&mut self.pending); } match pending { - Pending::Window(l) | Pending::WindowType(l) => self.acc.feature("scriptui.window", l), + Pending::Window(l) | Pending::WindowType(l) => self.acc.feature(FeatureId::ScriptuiWindow, l), Pending::Menu(l) | Pending::MenuString(l, _) => self.menu(None, l), Pending::Open(_) | Pending::Name(..) | Pending::None => {} } @@ -1820,23 +1820,23 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { /// outside the document. fn platform(&mut self, n: &'s str, line: Option) { match n { - catalog::UNIT_VALUE => self.acc.feature("unitValue", line), - catalog::FILE | catalog::FOLDER => self.acc.feature("file.objects", line), + catalog::UNIT_VALUE => self.acc.feature(FeatureId::UnitValue, line), + catalog::FILE | catalog::FOLDER => self.acc.feature(FeatureId::FileObjects, line), catalog::BRIDGE_TALK => { - self.acc.feature("bridgeTalk", line); - self.acc.outside("otherApps", line, None); + self.acc.feature(FeatureId::BridgeTalk, line); + self.acc.outside(OutsideId::OtherApps, line, None); } catalog::SOCKET => { - self.acc.feature("socket", line); - self.acc.outside("network", line, None); + self.acc.feature(FeatureId::Socket, line); + self.acc.outside(OutsideId::Network, line, None); } catalog::EXTERNAL_OBJECT => { - self.acc.feature("externalObject", line); - self.acc.outside("nativeCode", line, None); + self.acc.feature(FeatureId::ExternalObject, line); + self.acc.outside(OutsideId::NativeCode, line, None); } catalog::WINDOW => self.pending = Pending::Window(line), - _ if catalog::XMP.contains(&n) => self.acc.feature("xmp", line), - _ if catalog::XML.contains(&n) => self.acc.feature("xml", line), + _ if catalog::XMP.contains(&n) => self.acc.feature(FeatureId::Xmp, line), + _ if catalog::XML.contains(&n) => self.acc.feature(FeatureId::Xml, line), _ => {} } } @@ -1850,26 +1850,26 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { let of = |name: &str| word(h1, name) && !member_mark(h2) && !own.contains(name); if of(catalog::DOLLAR) { if catalog::DOLLAR_ENV.contains(&n) { - self.acc.feature("dollar.env", line); - self.acc.outside("environment", line, None); + self.acc.feature(FeatureId::DollarEnv, line); + self.acc.outside(OutsideId::Environment, line, None); } else if n == catalog::DOLLAR_EVAL_FILE { - self.acc.feature("dollar.evalFile", line); - self.acc.outside("unseenCode", line, None); + self.acc.feature(FeatureId::DollarEvalFile, line); + self.acc.outside(OutsideId::UnseenCode, line, None); } else if n == catalog::LOCALIZE { - self.acc.feature("localize", line); + self.acc.feature(FeatureId::Localize, line); } else { - self.acc.feature("dollar", line); + self.acc.feature(FeatureId::Dollar, line); } } else if (of(catalog::FILE) && catalog::FILE_DIALOGS.contains(&n)) || (of(catalog::FOLDER) && catalog::FOLDER_DIALOGS.contains(&n)) { - self.acc.feature("file.dialog", line); + self.acc.feature(FeatureId::FileDialog, line); } else if of(catalog::FOLDER) && catalog::FOLDER_PLACES.contains(&n) { - self.acc.feature("file.places", line); + self.acc.feature(FeatureId::FilePlaces, line); } if n == catalog::ON_DRAW { - self.acc.feature("scriptui.drawing", line); + self.acc.feature(FeatureId::ScriptuiDrawing, line); } if n == catalog::REFLECT { - self.acc.feature("reflection", line); + self.acc.feature(FeatureId::Reflection, line); } return; } @@ -1923,8 +1923,8 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } match n { // E4X: `default xml namespace = …` and `for each (…)`. - "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature("e4x", line), - "each" if word(h0, "for") => self.acc.feature("e4x", line), + "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature(FeatureId::E4x, line), + "each" if word(h0, "for") => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1937,11 +1937,11 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { "=" => { let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { - self.acc.feature("operatorOverloading", line); + self.acc.feature(FeatureId::OperatorOverloading, line); } } // E4X: attributes (`x.@id`, `@*`), descendants (`x..item`) and qualified names (`ns::x`). - ".@" | ".." | "::" | "@" => self.acc.feature("e4x", line), + ".@" | ".." | "::" | "@" => self.acc.feature(FeatureId::E4x, line), _ => {} } } @@ -1961,30 +1961,30 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } else if n == catalog::OPEN && file { self.pending = Pending::Open(line); } else if catalog::DRAWING.contains(&n) { - self.acc.feature("scriptui.drawing", line); + self.acc.feature(FeatureId::ScriptuiDrawing, line); } else if catalog::INSTANCE_DIALOGS.contains(&n) { - self.acc.feature("file.dialog", line); + self.acc.feature(FeatureId::FileDialog, line); } else if catalog::FOLDER_READS.contains(&n) || (file && catalog::FILE_READS.contains(&n)) { - self.acc.feature("file.read", line); + self.acc.feature(FeatureId::FileRead, line); } else if file && catalog::FILE_WRITES.contains(&n) { - self.acc.feature("file.write", line); - self.acc.outside("filesWritten", line, None); + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, None); } else if n == catalog::EXECUTE { - self.acc.feature("file.execute", line); - self.acc.outside("filesOpened", line, None); + self.acc.feature(FeatureId::FileExecute, line); + self.acc.outside(OutsideId::FilesOpened, line, None); } else if catalog::ACTIONS.contains(&n) { - self.acc.feature("actions", line); + self.acc.feature(FeatureId::Actions, line); } else if n == catalog::SELECT_TOOL { - self.acc.feature("selectTool", line); + self.acc.feature(FeatureId::SelectTool, line); } } else if !member_mark(h1) && !word(h1, "function") && !self.own_names.contains(n) { if catalog::NOTIFY.contains(&n) { - self.acc.feature("notify", line); + self.acc.feature(FeatureId::Notify, line); } else if n == catalog::LOCALIZE { - self.acc.feature("localize", line); + self.acc.feature(FeatureId::Localize, line); } else if matches!(n, "eval" | "Function") { - self.acc.feature("dynamicCode", line); - self.acc.outside("unseenCode", line, None); + self.acc.feature(FeatureId::DynamicCode, line); + self.acc.outside(OutsideId::UnseenCode, line, None); } } } @@ -1992,7 +1992,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { fn string(&mut self, t: &Sig<'s>, line: Option) { let [h0, h1, h2, _] = self.h; if t.text.contains(catalog::ACTION_TEXT) { - self.acc.feature("actions", line); + self.acc.feature(FeatureId::Actions, line); } let compares = h0.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "==" | "===" | "!=" | "!==")); if compares && word(h1, "typename") && is(h2, ".") { @@ -2055,10 +2055,10 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { return; }; let feature = match name { - "target" => "directive.target", - "include" | "includepath" => "directive.include", - "targetengine" => "directive.targetengine", - _ => "directive.other", + "target" => FeatureId::DirectiveTarget, + "include" | "includepath" => FeatureId::DirectiveInclude, + "targetengine" => FeatureId::DirectiveTargetengine, + _ => FeatureId::DirectiveOther, }; let line = self.agg(t.line); self.acc.feature(feature, line); diff --git a/crates/scriptcheck/src/catalog.rs b/crates/scriptcheck/src/catalog.rs index 720e5c097..50ddee3a1 100644 --- a/crates/scriptcheck/src/catalog.rs +++ b/crates/scriptcheck/src/catalog.rs @@ -114,66 +114,69 @@ pub(crate) const CONSTRUCTORS: &[&str] = &[ "XMPDateTime", ]; -/// Every feature id a report can list, in the order the report lists them. -pub const FEATURES: &[&str] = &[ - "actions", - "bridgeTalk", - "directive.include", - "directive.other", - "directive.target", - "directive.targetengine", - "dollar", - "dollar.env", - "dollar.evalFile", - "dynamicCode", - "e4x", - "externalObject", - "file.dialog", - "file.execute", - "file.objects", - "file.places", - "file.read", - "file.write", - "hostCheck", - "localize", - "menuCommand", - "notify", - "operatorOverloading", - "preferences", - "reflection", - "scriptui.dialog", - "scriptui.drawing", - "scriptui.palette", - "scriptui.window", - "selectTool", - "socket", - "unitValue", - "xml", - "xmp", -]; - -/// The need feature `id` sets. -pub(crate) fn need(id: &str) -> Option { - Some(match id { - "directive.target" | "directive.include" | "directive.other" | "dollar" | "dynamicCode" | "notify" | "localize" | "unitValue" - | "preferences" | "hostCheck" => Need::Language, - "dollar.env" | "dollar.evalFile" | "file.objects" | "file.read" | "file.write" | "file.dialog" | "file.places" | "file.execute" => { - Need::Files +/// Declares [`FeatureId`] from its table: each platform feature a report can list, with its id and +/// the need it sets. The table is sorted by id, which is the order the report lists them in. +macro_rules! features { + ($($variant:ident $id:literal $need:ident,)*) => { + /// A platform feature a report can list ([`crate::Feature::id`] is its [`FeatureId::id`]). + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] + pub(crate) enum FeatureId { + $($variant),* } - "menuCommand" => Need::MenuCommands, - "scriptui.dialog" | "scriptui.window" | "scriptui.drawing" => Need::Dialogs, - "directive.targetengine" - | "scriptui.palette" - | "bridgeTalk" - | "socket" - | "externalObject" - | "xmp" - | "xml" - | "e4x" - | "operatorOverloading" - | "reflection" - | "actions" - | "selectTool" => Need::HostOnly, - _ => return None, - }) + + impl FeatureId { + #[cfg(test)] + pub(crate) const ALL: &[FeatureId] = &[$(FeatureId::$variant),*]; + + pub(crate) fn id(self) -> &'static str { + match self { + $(FeatureId::$variant => $id),* + } + } + + /// The need the feature sets. + pub(crate) fn need(self) -> Need { + match self { + $(FeatureId::$variant => Need::$need),* + } + } + } + }; +} + +features! { + Actions "actions" HostOnly, + BridgeTalk "bridgeTalk" HostOnly, + DirectiveInclude "directive.include" Language, + DirectiveOther "directive.other" Language, + DirectiveTarget "directive.target" Language, + DirectiveTargetengine "directive.targetengine" HostOnly, + Dollar "dollar" Language, + DollarEnv "dollar.env" Files, + DollarEvalFile "dollar.evalFile" Files, + DynamicCode "dynamicCode" Language, + E4x "e4x" HostOnly, + ExternalObject "externalObject" HostOnly, + FileDialog "file.dialog" Files, + FileExecute "file.execute" Files, + FileObjects "file.objects" Files, + FilePlaces "file.places" Files, + FileRead "file.read" Files, + FileWrite "file.write" Files, + HostCheck "hostCheck" Language, + Localize "localize" Language, + MenuCommand "menuCommand" MenuCommands, + Notify "notify" Language, + OperatorOverloading "operatorOverloading" HostOnly, + Preferences "preferences" Language, + Reflection "reflection" HostOnly, + ScriptuiDialog "scriptui.dialog" Dialogs, + ScriptuiDrawing "scriptui.drawing" Dialogs, + ScriptuiPalette "scriptui.palette" HostOnly, + ScriptuiWindow "scriptui.window" Dialogs, + SelectTool "selectTool" HostOnly, + Socket "socket" HostOnly, + UnitValue "unitValue" Language, + Xml "xml" HostOnly, + Xmp "xmp" HostOnly, } diff --git a/crates/scriptcheck/src/lib.rs b/crates/scriptcheck/src/lib.rs index 818949f05..b9d9173fc 100644 --- a/crates/scriptcheck/src/lib.rs +++ b/crates/scriptcheck/src/lib.rs @@ -137,8 +137,6 @@ impl Need { } } -/// Every feature id a report can list, in the order the report lists them. -pub use catalog::FEATURES; /// Every id in [`Report::outside`], in the order the report lists them. pub use report::OUTSIDE; /// Every problem code a report can list. diff --git a/crates/scriptcheck/src/report.rs b/crates/scriptcheck/src/report.rs index e24e12114..975ef4118 100644 --- a/crates/scriptcheck/src/report.rs +++ b/crates/scriptcheck/src/report.rs @@ -3,7 +3,8 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; -use crate::{Kind, Limits, Need, catalog}; +use crate::catalog::FeatureId; +use crate::{Kind, Limits, Need}; pub(crate) const UNTERMINATED_STRING: &str = "unterminatedString"; pub(crate) const UNTERMINATED_COMMENT: &str = "unterminatedComment"; @@ -34,10 +35,40 @@ pub const PROBLEMS: &[&str] = &[ TOO_DEEP, ]; -/// Every id in [`Report::outside`], in the order the report lists them: files written, moved or -/// deleted; files opened in other apps; network connections; native code; messages to other apps; -/// environment variables; code the checker can't read; the reference app's preferences changed. -pub const OUTSIDE: &[&str] = &["filesWritten", "filesOpened", "network", "nativeCode", "otherApps", "environment", "unseenCode", "preferences"]; +/// Declares [`OutsideId`] and [`OUTSIDE`] from one table, in the order the report lists them. +macro_rules! outside { + ($($variant:ident $id:literal,)*) => { + /// Something the script does outside the document ([`Outside::id`] is its [`OutsideId::id`]). + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] + pub(crate) enum OutsideId { + $($variant),* + } + + impl OutsideId { + pub(crate) fn id(self) -> &'static str { + match self { + $(OutsideId::$variant => $id),* + } + } + } + + /// Every id in [`Report::outside`], in the order the report lists them: files written, moved or + /// deleted; files opened in other apps; network connections; native code; messages to other apps; + /// environment variables; code the checker can't read; the reference app's preferences changed. + pub const OUTSIDE: &[&str] = &[$($id),*]; + }; +} + +outside! { + FilesWritten "filesWritten", + FilesOpened "filesOpened", + Network "network", + NativeCode "nativeCode", + OtherApps "otherApps", + Environment "environment", + UnseenCode "unseenCode", + Preferences "preferences", +} /// What [`crate::check`] found in a script. Lists of located things (targets, directives, /// includes, problems) give a line and, for an included file, its path (`file`); tallies (features, @@ -54,7 +85,7 @@ pub struct Report { pub targets: Vec, pub directives: Vec, pub includes: Vec, - /// The platform features used, by id ([`crate::FEATURES`]). + /// The platform features used, by id. pub features: Vec, /// The document-model member chains used, as written (`app.activeDocument.layers[].name`, with /// `[]` for an index and `()` for a call), a variable assigned from a chain replaced by that @@ -225,8 +256,8 @@ impl Tally { pub(crate) struct Acc<'l> { pub limits: &'l Limits, pub steps: usize, - features: BTreeMap<&'static str, Tally>, - outside: BTreeMap<&'static str, (Tally, Vec<&'static str>)>, + features: BTreeMap, + outside: BTreeMap)>, menu: BTreeMap, menu_dynamic: usize, members: BTreeMap, @@ -279,12 +310,12 @@ impl<'l> Acc<'l> { room } - pub fn feature(&mut self, id: &'static str, line: Option) { + pub fn feature(&mut self, id: FeatureId, line: Option) { let max = self.limits.lines; self.features.entry(id).or_default().add(line, max); } - pub fn outside(&mut self, id: &'static str, line: Option, mode: Option<&'static str>) { + pub fn outside(&mut self, id: OutsideId, line: Option, mode: Option<&'static str>) { let max = self.limits.lines; let (tally, modes) = self.outside.entry(id).or_default(); tally.add(line, max); @@ -385,11 +416,10 @@ impl<'l> Acc<'l> { let mut menu_strings: Vec = self.menu.into_iter().map(|(value, t)| MenuString { value, count: t.count, lines: t.lines }).collect(); menu_strings.sort_by(|a, b| b.count.cmp(&a.count).then_with(|| a.value.cmp(&b.value))); - let features: Vec = self.features.into_iter().map(|(id, t)| Feature { id, count: t.count, lines: t.lines }).collect(); - let mut outside: Vec = - self.outside.into_iter().map(|(id, (t, modes))| Outside { id, count: t.count, lines: t.lines, modes }).collect(); - outside.sort_by_key(|o| OUTSIDE.iter().position(|id| *id == o.id)); - let mut needs: BTreeSet = features.iter().filter_map(|f| catalog::need(f.id)).collect(); + let mut needs: BTreeSet = self.features.keys().map(|id| id.need()).collect(); + let features: Vec = self.features.into_iter().map(|(id, t)| Feature { id: id.id(), count: t.count, lines: t.lines }).collect(); + let outside: Vec = + self.outside.into_iter().map(|(id, (t, modes))| Outside { id: id.id(), count: t.count, lines: t.lines, modes }).collect(); if !members.is_empty() || !types.is_empty() { needs.insert(Need::Dom); } diff --git a/crates/scriptcheck/src/tests.rs b/crates/scriptcheck/src/tests.rs index 6934a9a9d..9f08b94ea 100644 --- a/crates/scriptcheck/src/tests.rs +++ b/crates/scriptcheck/src/tests.rs @@ -2,6 +2,7 @@ use proptest::prelude::*; +use crate::catalog::FeatureId; use crate::lex::{Lexer, T}; use crate::*; @@ -95,7 +96,7 @@ fn each_feature_is_found_where_it_is_and_nowhere_else() { assert_eq!(r.kind, Kind::Script, "{with}"); let f = feature(&r, id).unwrap_or_else(|| panic!("{id} not found in {with:?}: {:?}", r.features)); assert_eq!(f.lines, [1], "{with}"); - let need = catalog::need(id).unwrap_or_else(|| panic!("{id} sets no need")); + let need = FeatureId::ALL.iter().find(|f| f.id() == *id).map(|f| f.need()).unwrap_or_else(|| panic!("{id} sets no need")); assert!(r.needs.contains(&need), "{with}: {:?}", r.needs); let r = report(without); assert!(feature(&r, id).is_none(), "{id} found in {without:?}"); @@ -103,14 +104,20 @@ fn each_feature_is_found_where_it_is_and_nowhere_else() { } #[test] -fn every_feature_id_has_a_need_and_a_case() { - assert!(FEATURES.windows(2).all(|w| w[0] < w[1]), "FEATURES is sorted, as the report lists them"); - for id in FEATURES { - assert!(catalog::need(id).is_some(), "{id}"); - assert!(FEATURE_CASES.iter().any(|(case, ..)| case == id), "{id} has no case"); +fn features_are_listed_in_alphabetical_order_of_their_ids() { + let ids: Vec<&str> = FeatureId::ALL.iter().map(|f| f.id()).collect(); + assert!(ids.windows(2).all(|w| w[0] < w[1]), "{ids:?}"); + assert!(FeatureId::ALL.windows(2).all(|w| w[0] < w[1]), "the enum is declared in the order of its ids"); +} + +#[test] +fn every_feature_id_has_a_case() { + for f in FeatureId::ALL { + let id = f.id(); + assert!(FEATURE_CASES.iter().any(|(case, ..)| *case == id), "{id} has no case"); } for (id, ..) in FEATURE_CASES { - assert!(FEATURES.contains(id), "{id} isn't in FEATURES"); + assert!(FeatureId::ALL.iter().any(|f| f.id() == *id), "{id} isn't a FeatureId"); } let unique = |ids: &[&str]| ids.iter().collect::>().len() == ids.len(); assert!(unique(PROBLEMS) && unique(OUTSIDE)); From e57773d45f2dcb41382aa7c20599c03f22b36a38 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:36:08 +0200 Subject: [PATCH 05/12] refactor(scriptcheck): split analyze.rs, share the token filter Move the declaration pass, the significant-token helpers and the feature recognisers into analyze/declared.rs, sig.rs and features.rs without changing their code, and replace the duplicated line-break and directive dispatch of scan and declarations with LineBreaks::read. Part of #4014 Co-Authored-By: Claude Sonnet 5.5 --- crates/scriptcheck/src/analyze.rs | 724 +-------------------- crates/scriptcheck/src/analyze/declared.rs | 189 ++++++ crates/scriptcheck/src/analyze/features.rs | 339 ++++++++++ crates/scriptcheck/src/analyze/sig.rs | 203 ++++++ 4 files changed, 744 insertions(+), 711 deletions(-) create mode 100644 crates/scriptcheck/src/analyze/declared.rs create mode 100644 crates/scriptcheck/src/analyze/features.rs create mode 100644 crates/scriptcheck/src/analyze/sig.rs diff --git a/crates/scriptcheck/src/analyze.rs b/crates/scriptcheck/src/analyze.rs index aa2119e43..f16b86c57 100644 --- a/crates/scriptcheck/src/analyze.rs +++ b/crates/scriptcheck/src/analyze.rs @@ -28,6 +28,14 @@ use crate::directives; use crate::lex::{Lexer, T, Token, expr_after}; use crate::report::{self, Acc, OutsideId}; +mod declared; +mod features; +mod sig; + +pub(crate) use declared::{Declared, declarations}; +use features::platform_name; +use sig::*; + /// An `#include` line: what it names, its line, and the folders `#includepath` named at that point /// (one list, shared by every line it applies to). #[derive(Clone, Debug)] @@ -84,24 +92,15 @@ pub(crate) struct Scan { pub(crate) fn scan<'s>(src: &'s str, declared: &Declared<'s>, file: Option<&str>, paths: Rc<[String]>, known: &Known, acc: &mut Acc) -> Scan { let mut lexer = Lexer::new(src, acc.limits.nesting); let mut a = Analyzer::new(acc, file, paths, known, declared); - let mut nl = false; + let mut breaks = LineBreaks::default(); for t in lexer.by_ref() { if let Some(code) = t.problem { a.problem(code, t.line); } - match t.kind { - T::Space => {} - // A comment with a line break in it ends a line, as a line break does. - T::Comment => nl |= t.text.contains(['\n', '\r', '\u{2028}', '\u{2029}']), - T::Newline => nl = true, - T::Directive => { - a.directive(&t); - nl = true; - } - _ => { - a.token(Sig::of(&t, nl)); - nl = false; - } + match breaks.read(&t) { + Read::Skip => {} + Read::Directive => a.directive(&t), + Read::Token(sig) => a.token(sig), } } if let Some(line) = lexer.unclosed_xml { @@ -175,203 +174,6 @@ fn unescape_name(text: &str) -> String { out } -/// A significant token: what it is, its text, its line, and whether a line break comes before it. -#[derive(Clone, Copy, Debug)] -struct Sig<'s> { - kind: T, - text: &'s str, - line: usize, - nl: bool, - /// The token ends a File or Folder object: the `)` that ends a call of `File` or `Folder` (with - /// or without `new`) or of `File.openDialog`, `File.saveDialog` or `Folder.selectDialog`, the - /// name in `Folder.temp` and the other places of [`catalog::FOLDER_PLACES`], or the `)` of - /// grouping parentheses that hold only such an object or a variable holding one (`(f)`). - file: bool, - /// A statement may start at the token ([`crate::lex::Token::stmt`]). - stmt: bool, - /// An expression may start at the token ([`crate::lex::Token::expr`]). - expr: bool, - /// The byte offset in the text where the token starts. - start: usize, -} - -impl<'s> Sig<'s> { - /// The significant token `t`, after a line break when `nl` is set. - fn of(t: &Token<'s>, nl: bool) -> Self { - Sig { kind: t.kind, text: t.text, line: t.line, nl, file: false, stmt: t.stmt, expr: t.expr, start: t.start } - } - - fn is(&self, punct: &str) -> bool { - self.kind == T::Punct && self.text == punct - } - - fn word(&self, word: &str) -> bool { - self.kind == T::Ident && self.text == word - } -} - -fn is(s: Option, punct: &str) -> bool { - s.is_some_and(|s| s.is(punct)) -} - -fn word(s: Option, w: &str) -> bool { - s.is_some_and(|s| s.word(w)) -} - -/// A token after which a name is a member, not a variable: `.`, `.@`, `..` and `::`. -fn member_mark(s: Option) -> bool { - s.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "." | ".@" | ".." | "::")) -} - -/// A token after which an expression goes on, also past a line break: a punctuator other than `)`, -/// `]`, `}`, `++`, `--` and `;`, or `in`, `instanceof`, `typeof`, `new`, `delete` or `void`. -fn goes_on_after(s: Option) -> bool { - s.is_some_and(|s| match s.kind { - T::Punct => !matches!(s.text, ")" | "]" | "}" | "++" | "--" | ";"), - T::Ident => matches!(s.text, "in" | "instanceof" | "typeof" | "new" | "delete" | "void"), - _ => false, - }) -} - -/// A token that goes on with the expression on the line before when it starts a line: a -/// punctuator other than `{`, `}`, `;`, `++`, `--`, `!` and `~`, or `in` or `instanceof`. -fn goes_on_with(t: &Sig) -> bool { - match t.kind { - T::Punct => !matches!(t.text, "{" | "}" | ";" | "++" | "--" | "!" | "~"), - T::Ident => matches!(t.text, "in" | "instanceof"), - _ => false, - } -} - -/// `t` is the `in` of `for (var name in …)`, which ends the `var` statement's declarations: it comes -/// right after a name the statement declares (ECMA-262 3rd edition, section 12.6.4). Outside a `for` -/// statement's parentheses, `in` can't stand there. -fn for_in(t: &Sig, h0: Option, h1: Option) -> bool { - t.word("in") && h0.is_some_and(|s| s.kind == T::Ident) && (word(h1, "var") || is(h1, ",")) -} - -/// A token after which the value before it is complete: `;`, `,`, `:` (a conditional expression's -/// colon, ECMA-262 3rd edition, section 11.12), `)`, `]` or `}`, or a token on a new line that -/// doesn't go on with the expression. -fn ends_value(t: &Sig) -> bool { - (t.nl && !goes_on_with(t)) || (t.kind == T::Punct && matches!(t.text, ";" | "," | ":" | ")" | "]" | "}")) -} - -/// Punctuators of later syntax. -const NEWER: &[&str] = &["=>", "...", "?.", "??", "**", "**=", "??=", "&&=", "||="]; - -/// The compound assignment operators (ECMA-262 3rd edition, section 11.13.2): each gives the name a -/// number or a string worked out from its old value. -const COMPOUND: &[&str] = &["*=", "/=", "%=", "+=", "-=", "<<=", ">>=", ">>>=", "&=", "^=", "|="]; - -/// A number in a later form: binary, octal (`0b1`, `0o7`), with separators (`1_000`) or big (`1n`). -fn newer_number(text: &str) -> bool { - let b = text.as_bytes(); - text.contains('_') || (b.len() > 1 && b.last() == Some(&b'n')) || matches!((b.first(), b.get(1)), (Some(b'0'), Some(b'b' | b'B' | b'o' | b'O'))) -} - -/// A string literal's value, its quotes removed and its escapes undone (`\n`, `\xHH`, `\uHHHH`, -/// octal escapes, a backslash before a line break), up to `max` characters. -fn unquote(text: &str, max: usize) -> String { - let mut chars = text.chars(); - let body = match chars.next() { - Some(q @ ('"' | '\'')) => chars.as_str().strip_suffix(q).unwrap_or(chars.as_str()), - _ => text, - }; - let mut out = String::new(); - let mut n = 0; - let mut it = body.chars().peekable(); - while n < max - && let Some(c) = it.next() - { - let c = if c == '\\' { - match it.next() { - None => break, - Some('n') => '\n', - Some('t') => '\t', - Some('r') => '\r', - Some('b') => '\u{8}', - Some('f') => '\u{c}', - Some('v') => '\u{b}', - // A legacy octal escape (ECMA-262 3rd edition, annex B.1.2): up to three digits - // after `\0` to `\3`, two after `\4` to `\7`. - Some(d @ '0'..='7') => { - let mut v = d.to_digit(8).unwrap_or_default(); - for _ in 0..if v <= 3 { 2 } else { 1 } { - match it.peek().and_then(|o| o.to_digit(8)) { - Some(o) => { - v = v * 8 + o; - it.next(); - } - None => break, - } - } - char::from_u32(v).unwrap_or('\u{fffd}') - } - Some(x @ ('x' | 'u')) => { - let digits = if x == 'x' { 2 } else { 4 }; - let mut v = 0u32; - for _ in 0..digits { - match it.peek().and_then(|d| d.to_digit(16)) { - Some(d) => { - v = v * 16 + d; - it.next(); - } - None => break, - } - } - char::from_u32(v).unwrap_or('\u{fffd}') - } - Some('\r') => { - if it.peek() == Some(&'\n') { - it.next(); - } - continue; - } - Some('\n' | '\u{2028}' | '\u{2029}') => continue, - Some(c) => c, - } - } else { - c - }; - out.push(c); - n += 1; - } - out -} - -/// A global name of the platform the report looks for (`$`, `File`, `Socket`, `alert`, `eval`…). -fn platform_name(n: &str) -> bool { - matches!( - n, - catalog::DOLLAR - | catalog::LOCALIZE - | catalog::UNIT_VALUE - | catalog::FILE - | catalog::FOLDER - | catalog::WINDOW - | catalog::BRIDGE_TALK - | catalog::SOCKET - | catalog::EXTERNAL_OBJECT - | "eval" - | "Function" - ) || catalog::NOTIFY.contains(&n) - || catalog::XMP.contains(&n) - || catalog::XML.contains(&n) -} - -/// The feature of a window of type `value` (`"dialog"`, or a resource string that starts with it). -fn window_type(value: &str) -> FeatureId { - let word = value.trim_start().split(|c: char| c.is_whitespace() || c == '{').next().unwrap_or_default(); - if word.eq_ignore_ascii_case("dialog") { - FeatureId::ScriptuiDialog - } else if word.eq_ignore_ascii_case("palette") { - FeatureId::ScriptuiPalette - } else { - FeatureId::ScriptuiWindow - } -} - /// What the token after a few others decides. #[derive(Clone, Copy, Debug, Default)] enum Pending<'s> { @@ -648,202 +450,6 @@ impl Brackets { } } -// ---------- the names declared, read before the analysis ---------- - -/// The names a file's function bodies and its global code declare, read before the analysis. A -/// function's `var` statements and the function declarations directly in its body make the names -/// they declare the function's own before any statement in it runs, and the function declarations -/// directly in the global code do the same for the whole file (ECMA-262 3rd edition, section -/// 10.1.3). A `var` statement in the global code does not change a property the global object -/// already has, such as the host's `alert` (sections 10.1.3, 10.1.5 and 10.2.1). The global code's -/// `var` statements are not read here. A function declaration inside a block, which ECMAScript 3 -/// does not allow (sections 12.1 and 14), makes its name the script's own from where it stands. -#[derive(Default)] -pub(crate) struct Declared<'s> { - /// For each function body, by the offset of its `{` in the text, the names it declares, in the - /// order the bodies start. - bodies: Vec<(usize, Vec<&'s str>)>, - /// The names the function declarations directly in the global code declare. - global: HashSet<&'s str>, -} - -/// A function body [`Declarations`] is reading: the bracket depth inside it, its entry in -/// [`Declared::bodies`], the names it declares so far, and the bracket depth of the `var` statement -/// the body is in, which goes on when the body closes. -struct Body<'s> { - depth: usize, - index: usize, - names: HashSet<&'s str>, - var_depth: Option, -} - -/// The pass that finds [`Declared`]. It follows brackets, function bodies and `var` statements by -/// the rules [`Analyzer`] follows them by, so that both passes find the same function bodies. -struct Declarations<'s, 'a, 'l> { - acc: &'a mut Acc<'l>, - brackets: Brackets, - /// The two significant tokens before this one, the latest first. - h: [Option>; 2], - /// `function` was read at this bracket depth, and its body's `{` is still to come. - function: Option, - /// The bracket depth of the `var` statement being read. - var_depth: Option, - /// `name =` was read, and the first token of the value is next (`true`: after `new`). As in - /// [`Analyzer::assignment`], that token doesn't start or end a `var` statement. - assign: Option, - /// The function bodies being read, the innermost last, up to [`crate::Limits::nesting`]. - bodies: Vec>, - /// The bracket depth inside the outermost function body past that limit being read. The names - /// declared inside it are not kept. - untracked: Option, - out: Declared<'s>, -} - -/// What `src` declares ([`Declared`]), read in one pass over its tokens. -pub(crate) fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { - let mut lexer = Lexer::new(src, acc.limits.nesting); - let mut d = Declarations { - acc, - brackets: Brackets::default(), - h: [None; 2], - function: None, - var_depth: None, - assign: None, - bodies: vec![], - untracked: None, - out: Declared::default(), - }; - let mut nl = false; - for t in lexer.by_ref() { - match t.kind { - T::Space => {} - T::Comment => nl |= t.text.contains(['\n', '\r', '\u{2028}', '\u{2029}']), - T::Newline => nl = true, - T::Directive => { - d.directive(); - nl = true; - } - _ => { - d.token(Sig::of(&t, nl)); - nl = false; - } - } - } - let out = d.out; - acc.steps = acc.steps.saturating_add(lexer.steps); - out -} - -impl<'s> Declarations<'s, '_, '_> { - fn depth(&self) -> usize { - self.brackets.depth() - } - - fn token(&mut self, t: Sig<'s>) { - let [h0, h1] = self.h; - // As in [`Analyzer::token`], a line break ends a `var` statement when it is outside the - // statement's brackets, and the tokens on either side of it don't go on with the expression. - if t.nl && self.var_depth == Some(self.depth()) && !goes_on_after(h0) && !goes_on_with(&t) { - self.var_depth = None; - } - if t.kind == T::Punct { - self.bracket(&t); - } else if t.kind == T::Ident { - self.declaration(&t); - } - let depth = self.depth(); - if let Some(after_new) = self.assign.take() { - if t.word("new") && !after_new { - self.assign = Some(true); - } - } else { - if t.word("var") { - self.var_depth = Some(depth); - } - if (t.is(";") || for_in(&t, h0, h1)) && self.var_depth == Some(depth) { - self.var_depth = None; - } - if t.is("=") && h0.is_some_and(|s| s.kind == T::Ident) && !member_mark(h1) && !is(h1, "@") { - self.assign = Some(false); - } - } - // `function` that isn't a property's name: its body's `{` comes after its name and - // parameters. - if t.word("function") && !member_mark(h0) && !word(h0, "function") { - self.function = Some(depth); - } - self.h = [Some(t), h0]; - } - - /// An open or close bracket, by the rules of [`Analyzer::bracket`]: the first `{` after - /// `function` at the depth of `function` opens the function's body. - fn bracket(&mut self, t: &Sig<'s>) { - match t.text { - "(" | "[" | "{" => { - let body = t.text == "{" && self.function.take() == Some(self.depth()); - self.brackets.open(t, self.acc.limits.nesting); - if !body { - return; - } - if self.bodies.len() < self.acc.limits.nesting { - let index = self.out.bodies.len(); - self.out.bodies.push((t.start, vec![])); - self.bodies.push(Body { depth: self.depth(), index, names: HashSet::new(), var_depth: self.var_depth.take() }); - } else if self.untracked.is_none() { - self.untracked = Some(self.depth()); - } - } - ")" | "]" | "}" => { - if !self.brackets.close(t) { - return; - } - let depth = self.depth(); - if self.var_depth.is_some_and(|d| depth < d) { - self.var_depth = None; - } - while let Some(body) = self.bodies.pop_if(|b| depth < b.depth) { - self.var_depth = body.var_depth; - } - if self.untracked.is_some_and(|d| depth < d) { - self.untracked = None; - } - } - _ => {} - } - } - - /// The name `t`, when a `var` statement or a function declaration declares it: kept for the - /// innermost function body when the `var` statement is in that body or the declaration is - /// directly in it, and for the global code when the declaration is directly in the global code. - /// Each body keeps up to [`crate::Limits::entries`] names, and so does the global code. - fn declaration(&mut self, t: &Sig<'s>) { - let [h0, _] = self.h; - let depth = self.depth(); - let var = word(h0, "var") || (is(h0, ",") && self.var_depth == Some(depth)); - let function = word(h0, "function") && h0.is_some_and(|f| f.stmt); - if !(var || function) || self.untracked.is_some() { - return; - } - if let Some(body) = self.bodies.last_mut() { - if (var || body.depth == depth) && !body.names.contains(t.text) && self.acc.room(body.names.len()) { - body.names.insert(t.text); - if let Some((_, names)) = self.out.bodies.get_mut(body.index) { - names.push(t.text); - } - } - } else if function && depth == 0 && !self.out.global.contains(t.text) && self.acc.room(self.out.global.len()) { - self.out.global.insert(t.text); - } - } - - /// A preprocessor line ends the statement before it, as in [`Analyzer::directive`]. - fn directive(&mut self) { - self.h = [None; 2]; - self.var_depth = None; - self.assign = None; - } -} - struct Analyzer<'s, 'a, 'l> { acc: &'a mut Acc<'l>, file: Option<&'a str>, @@ -1732,310 +1338,6 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } } - // ---------- features ---------- - - fn features(&mut self, t: &Sig<'s>) { - let line = self.agg(t.line); - let mut pending = std::mem::take(&mut self.pending); - // A platform name that no `:` follows stands for the platform's object. - if let Pending::Name(n, l) = pending { - pending = Pending::None; - if !t.is(":") { - self.platform(n, l); - pending = std::mem::take(&mut self.pending); - } - } - match pending { - Pending::None | Pending::Name(..) => {} - Pending::Window(l) => { - if t.is("(") { - self.pending = Pending::WindowType(l); - } else { - self.acc.feature(FeatureId::ScriptuiWindow, l); - } - } - Pending::WindowType(l) => { - let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { FeatureId::ScriptuiWindow }; - self.acc.feature(feature, l); - } - Pending::Menu(l) => { - if t.kind == T::Str { - self.pending = Pending::MenuString(l, t.text); - } else { - self.menu(None, l); - } - } - Pending::MenuString(l, s) => self.menu(t.is(")").then_some(s), l), - Pending::Open(l) => { - if t.kind == T::Str { - self.open_mode(&unquote(t.text, 2), l); - } - } - } - match t.kind { - T::Ident => self.name(t, line), - T::Punct => self.punct(t, line), - T::Str => self.string(t, line), - T::Xml => self.acc.feature(FeatureId::E4x, line), - _ => {} - } - } - - /// A menu command run with the string literal `literal`, or with another value. - fn menu(&mut self, literal: Option<&str>, line: Option) { - match literal { - Some(s) => { - let value = unquote(s, self.acc.limits.echo.saturating_add(1)); - self.acc.menu_string(&value, line); - } - None => self.acc.menu_dynamic(), - } - self.acc.feature(FeatureId::MenuCommand, line); - } - - fn open_mode(&mut self, mode: &str, line: Option) { - if mode.eq_ignore_ascii_case("r") { - self.acc.feature(FeatureId::FileRead, line); - } else if let Some(m) = catalog::WRITE_MODES.iter().copied().find(|m| m.eq_ignore_ascii_case(mode)) { - self.acc.feature(FeatureId::FileWrite, line); - self.acc.outside(OutsideId::FilesWritten, line, Some(m)); - } - } - - /// What a pending feature waits for that never came. - fn settle_pending(&mut self) { - let mut pending = std::mem::take(&mut self.pending); - if let Pending::Name(n, l) = pending { - self.platform(n, l); - pending = std::mem::take(&mut self.pending); - } - match pending { - Pending::Window(l) | Pending::WindowType(l) => self.acc.feature(FeatureId::ScriptuiWindow, l), - Pending::Menu(l) | Pending::MenuString(l, _) => self.menu(None, l), - Pending::Open(_) | Pending::Name(..) | Pending::None => {} - } - } - - /// The platform name `n`, used at `line` (a tally's line): the feature it is, and what it does - /// outside the document. - fn platform(&mut self, n: &'s str, line: Option) { - match n { - catalog::UNIT_VALUE => self.acc.feature(FeatureId::UnitValue, line), - catalog::FILE | catalog::FOLDER => self.acc.feature(FeatureId::FileObjects, line), - catalog::BRIDGE_TALK => { - self.acc.feature(FeatureId::BridgeTalk, line); - self.acc.outside(OutsideId::OtherApps, line, None); - } - catalog::SOCKET => { - self.acc.feature(FeatureId::Socket, line); - self.acc.outside(OutsideId::Network, line, None); - } - catalog::EXTERNAL_OBJECT => { - self.acc.feature(FeatureId::ExternalObject, line); - self.acc.outside(OutsideId::NativeCode, line, None); - } - catalog::WINDOW => self.pending = Pending::Window(line), - _ if catalog::XMP.contains(&n) => self.acc.feature(FeatureId::Xmp, line), - _ if catalog::XML.contains(&n) => self.acc.feature(FeatureId::Xml, line), - _ => {} - } - } - - fn name(&mut self, t: &Sig<'s>, line: Option) { - let [h0, h1, h2, _] = self.h; - let n = t.text; - if is(h0, ".") { - // A member: of `$`, `File` or `Folder` when they come first and are the platform's. - let own = &self.own_names; - let of = |name: &str| word(h1, name) && !member_mark(h2) && !own.contains(name); - if of(catalog::DOLLAR) { - if catalog::DOLLAR_ENV.contains(&n) { - self.acc.feature(FeatureId::DollarEnv, line); - self.acc.outside(OutsideId::Environment, line, None); - } else if n == catalog::DOLLAR_EVAL_FILE { - self.acc.feature(FeatureId::DollarEvalFile, line); - self.acc.outside(OutsideId::UnseenCode, line, None); - } else if n == catalog::LOCALIZE { - self.acc.feature(FeatureId::Localize, line); - } else { - self.acc.feature(FeatureId::Dollar, line); - } - } else if (of(catalog::FILE) && catalog::FILE_DIALOGS.contains(&n)) || (of(catalog::FOLDER) && catalog::FOLDER_DIALOGS.contains(&n)) { - self.acc.feature(FeatureId::FileDialog, line); - } else if of(catalog::FOLDER) && catalog::FOLDER_PLACES.contains(&n) { - self.acc.feature(FeatureId::FilePlaces, line); - } - if n == catalog::ON_DRAW { - self.acc.feature(FeatureId::ScriptuiDrawing, line); - } - if n == catalog::REFLECT { - self.acc.feature(FeatureId::Reflection, line); - } - return; - } - if member_mark(h0) { - return; - } - if word(h0, "function") { - self.acc.declared(n); - if h0.is_some_and(|f| f.stmt) { - // A function declaration's name is a variable where the function is declared. A - // declaration directly in a function body or in the global code makes it one from the - // body's `{` or from the start of the file ([`Declared`]), and any other declaration - // makes it one from here. - if !self.hoisted(n) { - self.keep_outer(n); - self.unbind(n); - } - } else { - // A function expression's name is a variable inside the function only. - self.expression_name = Some(n.to_string()); - } - return; - } - if n == "catch" { - // Its block's `{` comes after its parameter's parentheses. - self.catch = Some((self.depth(), None)); - } - if n == "function" { - // Its body's `{` comes after its name and parameters. - self.function = Some(self.depth()); - self.expression_name = None; - // `name = function`: the script's own function too. - if is(h0, "=") - && let Some(f) = h1.filter(|s| s.kind == T::Ident) - { - self.acc.declared(f.text); - } - } - if word(h0, "new") && n.starts_with(|c: char| c.is_ascii_uppercase()) && !catalog::CONSTRUCTORS.contains(&n) { - self.acc.type_name(n); - } - // A platform name stands for the platform's object, except where the script declares a - // variable, function or parameter of that name, and where it has a meaning of its own. - // Where a property key or a label may stand, the token after it decides. - if platform_name(n) && !self.declaring() && !self.own_names.contains(n) { - if h0.is_none() || [",", "{", ";", "}"].iter().any(|p| is(h0, p)) { - self.pending = Pending::Name(n, line); - } else { - self.platform(n, line); - } - } - match n { - // E4X: `default xml namespace = …` and `for each (…)`. - "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature(FeatureId::E4x, line), - "each" if word(h0, "for") => self.acc.feature(FeatureId::E4x, line), - _ => {} - } - } - - fn punct(&mut self, t: &Sig<'s>, line: Option) { - let [h0, h1, h2, h3] = self.h; - match t.text { - "(" => self.call(line), - // `Name.prototype["+"] = …` defines an operator. - "=" => { - let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); - if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { - self.acc.feature(FeatureId::OperatorOverloading, line); - } - } - // E4X: attributes (`x.@id`, `@*`), descendants (`x..item`) and qualified names (`ns::x`). - ".@" | ".." | "::" | "@" => self.acc.feature(FeatureId::E4x, line), - _ => {} - } - } - - /// `(` after a name: a call. - fn call(&mut self, line: Option) { - let [h0, h1, h2, h3] = self.h; - let Some(callee) = h0.filter(|s| s.kind == T::Ident) else { return }; - let n = callee.text; - if is(h1, ".") { - // The receiver is a variable holding a File or Folder object (not a property of the same - // name), or a File or Folder object made in place or held in grouping parentheses - // ([`Sig::file`]). - let file = h2.is_some_and(|r| r.file || (r.kind == T::Ident && !member_mark(h3) && self.files.contains(r.text))); - if n == catalog::MENU_COMMAND { - self.pending = Pending::Menu(line); - } else if n == catalog::OPEN && file { - self.pending = Pending::Open(line); - } else if catalog::DRAWING.contains(&n) { - self.acc.feature(FeatureId::ScriptuiDrawing, line); - } else if catalog::INSTANCE_DIALOGS.contains(&n) { - self.acc.feature(FeatureId::FileDialog, line); - } else if catalog::FOLDER_READS.contains(&n) || (file && catalog::FILE_READS.contains(&n)) { - self.acc.feature(FeatureId::FileRead, line); - } else if file && catalog::FILE_WRITES.contains(&n) { - self.acc.feature(FeatureId::FileWrite, line); - self.acc.outside(OutsideId::FilesWritten, line, None); - } else if n == catalog::EXECUTE { - self.acc.feature(FeatureId::FileExecute, line); - self.acc.outside(OutsideId::FilesOpened, line, None); - } else if catalog::ACTIONS.contains(&n) { - self.acc.feature(FeatureId::Actions, line); - } else if n == catalog::SELECT_TOOL { - self.acc.feature(FeatureId::SelectTool, line); - } - } else if !member_mark(h1) && !word(h1, "function") && !self.own_names.contains(n) { - if catalog::NOTIFY.contains(&n) { - self.acc.feature(FeatureId::Notify, line); - } else if n == catalog::LOCALIZE { - self.acc.feature(FeatureId::Localize, line); - } else if matches!(n, "eval" | "Function") { - self.acc.feature(FeatureId::DynamicCode, line); - self.acc.outside(OutsideId::UnseenCode, line, None); - } - } - } - - fn string(&mut self, t: &Sig<'s>, line: Option) { - let [h0, h1, h2, _] = self.h; - if t.text.contains(catalog::ACTION_TEXT) { - self.acc.feature(FeatureId::Actions, line); - } - let compares = h0.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "==" | "===" | "!=" | "!==")); - if compares && word(h1, "typename") && is(h2, ".") { - let name = unquote(t.text, self.acc.limits.echo.saturating_add(1)); - self.acc.type_name(&name); - } - } - - /// Syntax later than ECMAScript 3: reported once per construct and line. - fn newer(&mut self, t: &Sig<'s>) { - let [h0, h1, ..] = self.h; - let construct: Option<&'s str> = match t.kind { - T::Template => Some("`"), - T::Punct if NEWER.contains(&t.text) => Some(t.text), - T::Number if newer_number(t.text) => Some(t.text), - T::Other if t.text.len() > 1 && t.text.starts_with('#') => Some(t.text), - T::Ident if word(h0, "class") && !member_mark(h1) => Some("class"), - // `let name` declares a variable. ES3 reads `let` as a variable's name in `let = 1`, - // `let.x` and `let in o`. - T::Ident if word(h0, "let") && !t.nl && !member_mark(h1) && !matches!(t.text, "in" | "instanceof") => Some("let"), - T::Ident if t.text == "function" && word(h0, "async") && !t.nl => Some("async"), - _ => None, - }; - if let Some(c) = construct - && self.last_newer != Some((t.line, c)) - { - self.last_newer = Some((t.line, c)); - self.acc.problem(report::NEWER_SYNTAX, Some(t.line), self.file, Some(c)); - } - } - - /// `import` or `export` starting a statement outside any bracket, unless a call, member or - /// property (`import(…)`, `export.x`, `import: 1`) follows: a module. - fn module_check(&mut self, t: &Sig<'s>) { - if std::mem::take(&mut self.module_candidate) && !(t.kind == T::Punct && matches!(t.text, "(" | "." | "=" | ":" | "," | ")")) { - self.module = true; - } - let starts = self.h[0].is_none() || t.nl || is(self.h[0], ";") || is(self.h[0], "}"); - if t.kind == T::Ident && matches!(t.text, "import" | "export") && self.depth() == 0 && !member_mark(self.h[0]) && starts { - self.module_candidate = true; - } - } - // ---------- preprocessor lines ---------- fn directive(&mut self, t: &Token<'s>) { diff --git a/crates/scriptcheck/src/analyze/declared.rs b/crates/scriptcheck/src/analyze/declared.rs new file mode 100644 index 000000000..2f790a347 --- /dev/null +++ b/crates/scriptcheck/src/analyze/declared.rs @@ -0,0 +1,189 @@ +//! The names declared, read before the analysis. + +use super::*; + +/// The names a file's function bodies and its global code declare, read before the analysis. A +/// function's `var` statements and the function declarations directly in its body make the names +/// they declare the function's own before any statement in it runs, and the function declarations +/// directly in the global code do the same for the whole file (ECMA-262 3rd edition, section +/// 10.1.3). A `var` statement in the global code does not change a property the global object +/// already has, such as the host's `alert` (sections 10.1.3, 10.1.5 and 10.2.1). The global code's +/// `var` statements are not read here. A function declaration inside a block, which ECMAScript 3 +/// does not allow (sections 12.1 and 14), makes its name the script's own from where it stands. +#[derive(Default)] +pub(crate) struct Declared<'s> { + /// For each function body, by the offset of its `{` in the text, the names it declares, in the + /// order the bodies start. + pub(super) bodies: Vec<(usize, Vec<&'s str>)>, + /// The names the function declarations directly in the global code declare. + pub(super) global: HashSet<&'s str>, +} + +/// A function body [`Declarations`] is reading: the bracket depth inside it, its entry in +/// [`Declared::bodies`], the names it declares so far, and the bracket depth of the `var` statement +/// the body is in, which goes on when the body closes. +struct Body<'s> { + depth: usize, + index: usize, + names: HashSet<&'s str>, + var_depth: Option, +} + +/// The pass that finds [`Declared`]. It follows brackets, function bodies and `var` statements by +/// the rules [`Analyzer`] follows them by, so that both passes find the same function bodies. +struct Declarations<'s, 'a, 'l> { + acc: &'a mut Acc<'l>, + brackets: Brackets, + /// The two significant tokens before this one, the latest first. + h: [Option>; 2], + /// `function` was read at this bracket depth, and its body's `{` is still to come. + function: Option, + /// The bracket depth of the `var` statement being read. + var_depth: Option, + /// `name =` was read, and the first token of the value is next (`true`: after `new`). As in + /// [`Analyzer::assignment`], that token doesn't start or end a `var` statement. + assign: Option, + /// The function bodies being read, the innermost last, up to [`crate::Limits::nesting`]. + bodies: Vec>, + /// The bracket depth inside the outermost function body past that limit being read. The names + /// declared inside it are not kept. + untracked: Option, + out: Declared<'s>, +} + +/// What `src` declares ([`Declared`]), read in one pass over its tokens. +pub(crate) fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { + let mut lexer = Lexer::new(src, acc.limits.nesting); + let mut d = Declarations { + acc, + brackets: Brackets::default(), + h: [None; 2], + function: None, + var_depth: None, + assign: None, + bodies: vec![], + untracked: None, + out: Declared::default(), + }; + let mut breaks = LineBreaks::default(); + for t in lexer.by_ref() { + match breaks.read(&t) { + Read::Skip => {} + Read::Directive => d.directive(), + Read::Token(sig) => d.token(sig), + } + } + let out = d.out; + acc.steps = acc.steps.saturating_add(lexer.steps); + out +} + +impl<'s> Declarations<'s, '_, '_> { + fn depth(&self) -> usize { + self.brackets.depth() + } + + fn token(&mut self, t: Sig<'s>) { + let [h0, h1] = self.h; + // As in [`Analyzer::token`], a line break ends a `var` statement when it is outside the + // statement's brackets, and the tokens on either side of it don't go on with the expression. + if t.nl && self.var_depth == Some(self.depth()) && !goes_on_after(h0) && !goes_on_with(&t) { + self.var_depth = None; + } + if t.kind == T::Punct { + self.bracket(&t); + } else if t.kind == T::Ident { + self.declaration(&t); + } + let depth = self.depth(); + if let Some(after_new) = self.assign.take() { + if t.word("new") && !after_new { + self.assign = Some(true); + } + } else { + if t.word("var") { + self.var_depth = Some(depth); + } + if (t.is(";") || for_in(&t, h0, h1)) && self.var_depth == Some(depth) { + self.var_depth = None; + } + if t.is("=") && h0.is_some_and(|s| s.kind == T::Ident) && !member_mark(h1) && !is(h1, "@") { + self.assign = Some(false); + } + } + // `function` that isn't a property's name: its body's `{` comes after its name and + // parameters. + if t.word("function") && !member_mark(h0) && !word(h0, "function") { + self.function = Some(depth); + } + self.h = [Some(t), h0]; + } + + /// An open or close bracket, by the rules of [`Analyzer::bracket`]: the first `{` after + /// `function` at the depth of `function` opens the function's body. + fn bracket(&mut self, t: &Sig<'s>) { + match t.text { + "(" | "[" | "{" => { + let body = t.text == "{" && self.function.take() == Some(self.depth()); + self.brackets.open(t, self.acc.limits.nesting); + if !body { + return; + } + if self.bodies.len() < self.acc.limits.nesting { + let index = self.out.bodies.len(); + self.out.bodies.push((t.start, vec![])); + self.bodies.push(Body { depth: self.depth(), index, names: HashSet::new(), var_depth: self.var_depth.take() }); + } else if self.untracked.is_none() { + self.untracked = Some(self.depth()); + } + } + ")" | "]" | "}" => { + if !self.brackets.close(t) { + return; + } + let depth = self.depth(); + if self.var_depth.is_some_and(|d| depth < d) { + self.var_depth = None; + } + while let Some(body) = self.bodies.pop_if(|b| depth < b.depth) { + self.var_depth = body.var_depth; + } + if self.untracked.is_some_and(|d| depth < d) { + self.untracked = None; + } + } + _ => {} + } + } + + /// The name `t`, when a `var` statement or a function declaration declares it: kept for the + /// innermost function body when the `var` statement is in that body or the declaration is + /// directly in it, and for the global code when the declaration is directly in the global code. + /// Each body keeps up to [`crate::Limits::entries`] names, and so does the global code. + fn declaration(&mut self, t: &Sig<'s>) { + let [h0, _] = self.h; + let depth = self.depth(); + let var = word(h0, "var") || (is(h0, ",") && self.var_depth == Some(depth)); + let function = word(h0, "function") && h0.is_some_and(|f| f.stmt); + if !(var || function) || self.untracked.is_some() { + return; + } + if let Some(body) = self.bodies.last_mut() { + if (var || body.depth == depth) && !body.names.contains(t.text) && self.acc.room(body.names.len()) { + body.names.insert(t.text); + if let Some((_, names)) = self.out.bodies.get_mut(body.index) { + names.push(t.text); + } + } + } else if function && depth == 0 && !self.out.global.contains(t.text) && self.acc.room(self.out.global.len()) { + self.out.global.insert(t.text); + } + } + + /// A preprocessor line ends the statement before it, as in [`Analyzer::directive`]. + fn directive(&mut self) { + self.h = [None; 2]; + self.var_depth = None; + self.assign = None; + } +} diff --git a/crates/scriptcheck/src/analyze/features.rs b/crates/scriptcheck/src/analyze/features.rs new file mode 100644 index 000000000..0a8ba97d8 --- /dev/null +++ b/crates/scriptcheck/src/analyze/features.rs @@ -0,0 +1,339 @@ +//! The platform features, menu commands and later syntax the analysis recognises. + +use super::*; + +/// A global name of the platform the report looks for (`$`, `File`, `Socket`, `alert`, `eval`…). +pub(super) fn platform_name(n: &str) -> bool { + matches!( + n, + catalog::DOLLAR + | catalog::LOCALIZE + | catalog::UNIT_VALUE + | catalog::FILE + | catalog::FOLDER + | catalog::WINDOW + | catalog::BRIDGE_TALK + | catalog::SOCKET + | catalog::EXTERNAL_OBJECT + | "eval" + | "Function" + ) || catalog::NOTIFY.contains(&n) + || catalog::XMP.contains(&n) + || catalog::XML.contains(&n) +} + +/// The feature of a window of type `value` (`"dialog"`, or a resource string that starts with it). +pub(super) fn window_type(value: &str) -> FeatureId { + let word = value.trim_start().split(|c: char| c.is_whitespace() || c == '{').next().unwrap_or_default(); + if word.eq_ignore_ascii_case("dialog") { + FeatureId::ScriptuiDialog + } else if word.eq_ignore_ascii_case("palette") { + FeatureId::ScriptuiPalette + } else { + FeatureId::ScriptuiWindow + } +} + +impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { + pub(super) fn features(&mut self, t: &Sig<'s>) { + let line = self.agg(t.line); + let mut pending = std::mem::take(&mut self.pending); + // A platform name that no `:` follows stands for the platform's object. + if let Pending::Name(n, l) = pending { + pending = Pending::None; + if !t.is(":") { + self.platform(n, l); + pending = std::mem::take(&mut self.pending); + } + } + match pending { + Pending::None | Pending::Name(..) => {} + Pending::Window(l) => { + if t.is("(") { + self.pending = Pending::WindowType(l); + } else { + self.acc.feature(FeatureId::ScriptuiWindow, l); + } + } + Pending::WindowType(l) => { + let feature = if t.kind == T::Str { window_type(&unquote(t.text, 64)) } else { FeatureId::ScriptuiWindow }; + self.acc.feature(feature, l); + } + Pending::Menu(l) => { + if t.kind == T::Str { + self.pending = Pending::MenuString(l, t.text); + } else { + self.menu(None, l); + } + } + Pending::MenuString(l, s) => self.menu(t.is(")").then_some(s), l), + Pending::Open(l) => { + if t.kind == T::Str { + self.open_mode(&unquote(t.text, 2), l); + } + } + } + match t.kind { + T::Ident => self.name(t, line), + T::Punct => self.punct(t, line), + T::Str => self.string(t, line), + T::Xml => self.acc.feature(FeatureId::E4x, line), + _ => {} + } + } + + /// A menu command run with the string literal `literal`, or with another value. + fn menu(&mut self, literal: Option<&str>, line: Option) { + match literal { + Some(s) => { + let value = unquote(s, self.acc.limits.echo.saturating_add(1)); + self.acc.menu_string(&value, line); + } + None => self.acc.menu_dynamic(), + } + self.acc.feature(FeatureId::MenuCommand, line); + } + + fn open_mode(&mut self, mode: &str, line: Option) { + if mode.eq_ignore_ascii_case("r") { + self.acc.feature(FeatureId::FileRead, line); + } else if let Some(m) = catalog::WRITE_MODES.iter().copied().find(|m| m.eq_ignore_ascii_case(mode)) { + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, Some(m)); + } + } + + /// What a pending feature waits for that never came. + pub(super) fn settle_pending(&mut self) { + let mut pending = std::mem::take(&mut self.pending); + if let Pending::Name(n, l) = pending { + self.platform(n, l); + pending = std::mem::take(&mut self.pending); + } + match pending { + Pending::Window(l) | Pending::WindowType(l) => self.acc.feature(FeatureId::ScriptuiWindow, l), + Pending::Menu(l) | Pending::MenuString(l, _) => self.menu(None, l), + Pending::Open(_) | Pending::Name(..) | Pending::None => {} + } + } + + /// The platform name `n`, used at `line` (a tally's line): the feature it is, and what it does + /// outside the document. + fn platform(&mut self, n: &'s str, line: Option) { + match n { + catalog::UNIT_VALUE => self.acc.feature(FeatureId::UnitValue, line), + catalog::FILE | catalog::FOLDER => self.acc.feature(FeatureId::FileObjects, line), + catalog::BRIDGE_TALK => { + self.acc.feature(FeatureId::BridgeTalk, line); + self.acc.outside(OutsideId::OtherApps, line, None); + } + catalog::SOCKET => { + self.acc.feature(FeatureId::Socket, line); + self.acc.outside(OutsideId::Network, line, None); + } + catalog::EXTERNAL_OBJECT => { + self.acc.feature(FeatureId::ExternalObject, line); + self.acc.outside(OutsideId::NativeCode, line, None); + } + catalog::WINDOW => self.pending = Pending::Window(line), + _ if catalog::XMP.contains(&n) => self.acc.feature(FeatureId::Xmp, line), + _ if catalog::XML.contains(&n) => self.acc.feature(FeatureId::Xml, line), + _ => {} + } + } + + fn name(&mut self, t: &Sig<'s>, line: Option) { + let [h0, h1, h2, _] = self.h; + let n = t.text; + if is(h0, ".") { + // A member: of `$`, `File` or `Folder` when they come first and are the platform's. + let own = &self.own_names; + let of = |name: &str| word(h1, name) && !member_mark(h2) && !own.contains(name); + if of(catalog::DOLLAR) { + if catalog::DOLLAR_ENV.contains(&n) { + self.acc.feature(FeatureId::DollarEnv, line); + self.acc.outside(OutsideId::Environment, line, None); + } else if n == catalog::DOLLAR_EVAL_FILE { + self.acc.feature(FeatureId::DollarEvalFile, line); + self.acc.outside(OutsideId::UnseenCode, line, None); + } else if n == catalog::LOCALIZE { + self.acc.feature(FeatureId::Localize, line); + } else { + self.acc.feature(FeatureId::Dollar, line); + } + } else if (of(catalog::FILE) && catalog::FILE_DIALOGS.contains(&n)) || (of(catalog::FOLDER) && catalog::FOLDER_DIALOGS.contains(&n)) { + self.acc.feature(FeatureId::FileDialog, line); + } else if of(catalog::FOLDER) && catalog::FOLDER_PLACES.contains(&n) { + self.acc.feature(FeatureId::FilePlaces, line); + } + if n == catalog::ON_DRAW { + self.acc.feature(FeatureId::ScriptuiDrawing, line); + } + if n == catalog::REFLECT { + self.acc.feature(FeatureId::Reflection, line); + } + return; + } + if member_mark(h0) { + return; + } + if word(h0, "function") { + self.acc.declared(n); + if h0.is_some_and(|f| f.stmt) { + // A function declaration's name is a variable where the function is declared. A + // declaration directly in a function body or in the global code makes it one from the + // body's `{` or from the start of the file ([`Declared`]), and any other declaration + // makes it one from here. + if !self.hoisted(n) { + self.keep_outer(n); + self.unbind(n); + } + } else { + // A function expression's name is a variable inside the function only. + self.expression_name = Some(n.to_string()); + } + return; + } + if n == "catch" { + // Its block's `{` comes after its parameter's parentheses. + self.catch = Some((self.depth(), None)); + } + if n == "function" { + // Its body's `{` comes after its name and parameters. + self.function = Some(self.depth()); + self.expression_name = None; + // `name = function`: the script's own function too. + if is(h0, "=") + && let Some(f) = h1.filter(|s| s.kind == T::Ident) + { + self.acc.declared(f.text); + } + } + if word(h0, "new") && n.starts_with(|c: char| c.is_ascii_uppercase()) && !catalog::CONSTRUCTORS.contains(&n) { + self.acc.type_name(n); + } + // A platform name stands for the platform's object, except where the script declares a + // variable, function or parameter of that name, and where it has a meaning of its own. + // Where a property key or a label may stand, the token after it decides. + if platform_name(n) && !self.declaring() && !self.own_names.contains(n) { + if h0.is_none() || [",", "{", ";", "}"].iter().any(|p| is(h0, p)) { + self.pending = Pending::Name(n, line); + } else { + self.platform(n, line); + } + } + match n { + // E4X: `default xml namespace = …` and `for each (…)`. + "namespace" if word(h0, "xml") && word(h1, "default") => self.acc.feature(FeatureId::E4x, line), + "each" if word(h0, "for") => self.acc.feature(FeatureId::E4x, line), + _ => {} + } + } + + fn punct(&mut self, t: &Sig<'s>, line: Option) { + let [h0, h1, h2, h3] = self.h; + match t.text { + "(" => self.call(line), + // `Name.prototype["+"] = …` defines an operator. + "=" => { + let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); + if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { + self.acc.feature(FeatureId::OperatorOverloading, line); + } + } + // E4X: attributes (`x.@id`, `@*`), descendants (`x..item`) and qualified names (`ns::x`). + ".@" | ".." | "::" | "@" => self.acc.feature(FeatureId::E4x, line), + _ => {} + } + } + + /// `(` after a name: a call. + fn call(&mut self, line: Option) { + let [h0, h1, h2, h3] = self.h; + let Some(callee) = h0.filter(|s| s.kind == T::Ident) else { return }; + let n = callee.text; + if is(h1, ".") { + // The receiver is a variable holding a File or Folder object (not a property of the same + // name), or a File or Folder object made in place or held in grouping parentheses + // ([`Sig::file`]). + let file = h2.is_some_and(|r| r.file || (r.kind == T::Ident && !member_mark(h3) && self.files.contains(r.text))); + if n == catalog::MENU_COMMAND { + self.pending = Pending::Menu(line); + } else if n == catalog::OPEN && file { + self.pending = Pending::Open(line); + } else if catalog::DRAWING.contains(&n) { + self.acc.feature(FeatureId::ScriptuiDrawing, line); + } else if catalog::INSTANCE_DIALOGS.contains(&n) { + self.acc.feature(FeatureId::FileDialog, line); + } else if catalog::FOLDER_READS.contains(&n) || (file && catalog::FILE_READS.contains(&n)) { + self.acc.feature(FeatureId::FileRead, line); + } else if file && catalog::FILE_WRITES.contains(&n) { + self.acc.feature(FeatureId::FileWrite, line); + self.acc.outside(OutsideId::FilesWritten, line, None); + } else if n == catalog::EXECUTE { + self.acc.feature(FeatureId::FileExecute, line); + self.acc.outside(OutsideId::FilesOpened, line, None); + } else if catalog::ACTIONS.contains(&n) { + self.acc.feature(FeatureId::Actions, line); + } else if n == catalog::SELECT_TOOL { + self.acc.feature(FeatureId::SelectTool, line); + } + } else if !member_mark(h1) && !word(h1, "function") && !self.own_names.contains(n) { + if catalog::NOTIFY.contains(&n) { + self.acc.feature(FeatureId::Notify, line); + } else if n == catalog::LOCALIZE { + self.acc.feature(FeatureId::Localize, line); + } else if matches!(n, "eval" | "Function") { + self.acc.feature(FeatureId::DynamicCode, line); + self.acc.outside(OutsideId::UnseenCode, line, None); + } + } + } + + fn string(&mut self, t: &Sig<'s>, line: Option) { + let [h0, h1, h2, _] = self.h; + if t.text.contains(catalog::ACTION_TEXT) { + self.acc.feature(FeatureId::Actions, line); + } + let compares = h0.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "==" | "===" | "!=" | "!==")); + if compares && word(h1, "typename") && is(h2, ".") { + let name = unquote(t.text, self.acc.limits.echo.saturating_add(1)); + self.acc.type_name(&name); + } + } + + /// Syntax later than ECMAScript 3: reported once per construct and line. + pub(super) fn newer(&mut self, t: &Sig<'s>) { + let [h0, h1, ..] = self.h; + let construct: Option<&'s str> = match t.kind { + T::Template => Some("`"), + T::Punct if NEWER.contains(&t.text) => Some(t.text), + T::Number if newer_number(t.text) => Some(t.text), + T::Other if t.text.len() > 1 && t.text.starts_with('#') => Some(t.text), + T::Ident if word(h0, "class") && !member_mark(h1) => Some("class"), + // `let name` declares a variable. ES3 reads `let` as a variable's name in `let = 1`, + // `let.x` and `let in o`. + T::Ident if word(h0, "let") && !t.nl && !member_mark(h1) && !matches!(t.text, "in" | "instanceof") => Some("let"), + T::Ident if t.text == "function" && word(h0, "async") && !t.nl => Some("async"), + _ => None, + }; + if let Some(c) = construct + && self.last_newer != Some((t.line, c)) + { + self.last_newer = Some((t.line, c)); + self.acc.problem(report::NEWER_SYNTAX, Some(t.line), self.file, Some(c)); + } + } + + /// `import` or `export` starting a statement outside any bracket, unless a call, member or + /// property (`import(…)`, `export.x`, `import: 1`) follows: a module. + pub(super) fn module_check(&mut self, t: &Sig<'s>) { + if std::mem::take(&mut self.module_candidate) && !(t.kind == T::Punct && matches!(t.text, "(" | "." | "=" | ":" | "," | ")")) { + self.module = true; + } + let starts = self.h[0].is_none() || t.nl || is(self.h[0], ";") || is(self.h[0], "}"); + if t.kind == T::Ident && matches!(t.text, "import" | "export") && self.depth() == 0 && !member_mark(self.h[0]) && starts { + self.module_candidate = true; + } + } +} diff --git a/crates/scriptcheck/src/analyze/sig.rs b/crates/scriptcheck/src/analyze/sig.rs new file mode 100644 index 000000000..1cde2df0f --- /dev/null +++ b/crates/scriptcheck/src/analyze/sig.rs @@ -0,0 +1,203 @@ +//! The significant tokens the analysis reads, and the checks on them that both passes share. + +use crate::lex::{T, Token}; + +/// A significant token: what it is, its text, its line, and whether a line break comes before it. +#[derive(Clone, Copy, Debug)] +pub(super) struct Sig<'s> { + pub(super) kind: T, + pub(super) text: &'s str, + pub(super) line: usize, + pub(super) nl: bool, + /// The token ends a File or Folder object: the `)` that ends a call of `File` or `Folder` (with + /// or without `new`) or of `File.openDialog`, `File.saveDialog` or `Folder.selectDialog`, the + /// name in `Folder.temp` and the other places of [`catalog::FOLDER_PLACES`], or the `)` of + /// grouping parentheses that hold only such an object or a variable holding one (`(f)`). + pub(super) file: bool, + /// A statement may start at the token ([`crate::lex::Token::stmt`]). + pub(super) stmt: bool, + /// An expression may start at the token ([`crate::lex::Token::expr`]). + pub(super) expr: bool, + /// The byte offset in the text where the token starts. + pub(super) start: usize, +} + +impl<'s> Sig<'s> { + /// The significant token `t`, after a line break when `nl` is set. + pub(super) fn of(t: &Token<'s>, nl: bool) -> Self { + Sig { kind: t.kind, text: t.text, line: t.line, nl, file: false, stmt: t.stmt, expr: t.expr, start: t.start } + } + + pub(super) fn is(&self, punct: &str) -> bool { + self.kind == T::Punct && self.text == punct + } + + pub(super) fn word(&self, word: &str) -> bool { + self.kind == T::Ident && self.text == word + } +} + +/// What a token means to the passes that follow the significant tokens. +pub(super) enum Read<'s> { + /// A blank, or a comment or line break (remembered for the next token). + Skip, + /// A preprocessor line. + Directive, + Token(Sig<'s>), +} + +/// Whether a line break came since the last significant token. +#[derive(Default)] +pub(super) struct LineBreaks(bool); + +impl LineBreaks { + pub(super) fn read<'s>(&mut self, t: &Token<'s>) -> Read<'s> { + match t.kind { + T::Space => Read::Skip, + // A comment with a line break in it ends a line, as a line break does. + T::Comment => { + self.0 |= t.text.contains(['\n', '\r', '\u{2028}', '\u{2029}']); + Read::Skip + } + T::Newline => { + self.0 = true; + Read::Skip + } + T::Directive => { + self.0 = true; + Read::Directive + } + _ => Read::Token(Sig::of(t, std::mem::take(&mut self.0))), + } + } +} + +pub(super) fn is(s: Option, punct: &str) -> bool { + s.is_some_and(|s| s.is(punct)) +} + +pub(super) fn word(s: Option, w: &str) -> bool { + s.is_some_and(|s| s.word(w)) +} + +/// A token after which a name is a member, not a variable: `.`, `.@`, `..` and `::`. +pub(super) fn member_mark(s: Option) -> bool { + s.is_some_and(|s| s.kind == T::Punct && matches!(s.text, "." | ".@" | ".." | "::")) +} + +/// A token after which an expression goes on, also past a line break: a punctuator other than `)`, +/// `]`, `}`, `++`, `--` and `;`, or `in`, `instanceof`, `typeof`, `new`, `delete` or `void`. +pub(super) fn goes_on_after(s: Option) -> bool { + s.is_some_and(|s| match s.kind { + T::Punct => !matches!(s.text, ")" | "]" | "}" | "++" | "--" | ";"), + T::Ident => matches!(s.text, "in" | "instanceof" | "typeof" | "new" | "delete" | "void"), + _ => false, + }) +} + +/// A token that goes on with the expression on the line before when it starts a line: a +/// punctuator other than `{`, `}`, `;`, `++`, `--`, `!` and `~`, or `in` or `instanceof`. +pub(super) fn goes_on_with(t: &Sig) -> bool { + match t.kind { + T::Punct => !matches!(t.text, "{" | "}" | ";" | "++" | "--" | "!" | "~"), + T::Ident => matches!(t.text, "in" | "instanceof"), + _ => false, + } +} + +/// `t` is the `in` of `for (var name in …)`, which ends the `var` statement's declarations: it comes +/// right after a name the statement declares (ECMA-262 3rd edition, section 12.6.4). Outside a `for` +/// statement's parentheses, `in` can't stand there. +pub(super) fn for_in(t: &Sig, h0: Option, h1: Option) -> bool { + t.word("in") && h0.is_some_and(|s| s.kind == T::Ident) && (word(h1, "var") || is(h1, ",")) +} + +/// A token after which the value before it is complete: `;`, `,`, `:` (a conditional expression's +/// colon, ECMA-262 3rd edition, section 11.12), `)`, `]` or `}`, or a token on a new line that +/// doesn't go on with the expression. +pub(super) fn ends_value(t: &Sig) -> bool { + (t.nl && !goes_on_with(t)) || (t.kind == T::Punct && matches!(t.text, ";" | "," | ":" | ")" | "]" | "}")) +} + +/// Punctuators of later syntax. +pub(super) const NEWER: &[&str] = &["=>", "...", "?.", "??", "**", "**=", "??=", "&&=", "||="]; + +/// The compound assignment operators (ECMA-262 3rd edition, section 11.13.2): each gives the name a +/// number or a string worked out from its old value. +pub(super) const COMPOUND: &[&str] = &["*=", "/=", "%=", "+=", "-=", "<<=", ">>=", ">>>=", "&=", "^=", "|="]; + +/// A number in a later form: binary, octal (`0b1`, `0o7`), with separators (`1_000`) or big (`1n`). +pub(super) fn newer_number(text: &str) -> bool { + let b = text.as_bytes(); + text.contains('_') || (b.len() > 1 && b.last() == Some(&b'n')) || matches!((b.first(), b.get(1)), (Some(b'0'), Some(b'b' | b'B' | b'o' | b'O'))) +} + +/// A string literal's value, its quotes removed and its escapes undone (`\n`, `\xHH`, `\uHHHH`, +/// octal escapes, a backslash before a line break), up to `max` characters. +pub(super) fn unquote(text: &str, max: usize) -> String { + let mut chars = text.chars(); + let body = match chars.next() { + Some(q @ ('"' | '\'')) => chars.as_str().strip_suffix(q).unwrap_or(chars.as_str()), + _ => text, + }; + let mut out = String::new(); + let mut n = 0; + let mut it = body.chars().peekable(); + while n < max + && let Some(c) = it.next() + { + let c = if c == '\\' { + match it.next() { + None => break, + Some('n') => '\n', + Some('t') => '\t', + Some('r') => '\r', + Some('b') => '\u{8}', + Some('f') => '\u{c}', + Some('v') => '\u{b}', + // A legacy octal escape (ECMA-262 3rd edition, annex B.1.2): up to three digits + // after `\0` to `\3`, two after `\4` to `\7`. + Some(d @ '0'..='7') => { + let mut v = d.to_digit(8).unwrap_or_default(); + for _ in 0..if v <= 3 { 2 } else { 1 } { + match it.peek().and_then(|o| o.to_digit(8)) { + Some(o) => { + v = v * 8 + o; + it.next(); + } + None => break, + } + } + char::from_u32(v).unwrap_or('\u{fffd}') + } + Some(x @ ('x' | 'u')) => { + let digits = if x == 'x' { 2 } else { 4 }; + let mut v = 0u32; + for _ in 0..digits { + match it.peek().and_then(|d| d.to_digit(16)) { + Some(d) => { + v = v * 16 + d; + it.next(); + } + None => break, + } + } + char::from_u32(v).unwrap_or('\u{fffd}') + } + Some('\r') => { + if it.peek() == Some(&'\n') { + it.next(); + } + continue; + } + Some('\n' | '\u{2028}' | '\u{2029}') => continue, + Some(c) => c, + } + } else { + c + }; + out.push(c); + n += 1; + } + out +} From 472cd812f9b4ab9f0900a2632282e893983f18bd Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:54:42 +0700 Subject: [PATCH 06/12] Keep escaped backslash literal before U+XXXX in DXF MTEXT Squashed from #4000 onto main. --- crates/cad/src/import/text.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/cad/src/import/text.rs b/crates/cad/src/import/text.rs index 5d2797d61..137d9cc31 100644 --- a/crates/cad/src/import/text.rs +++ b/crates/cad/src/import/text.rs @@ -63,13 +63,19 @@ fn until_semicolon(chars: &mut std::iter::Peekable>) -> Stri } /// `\U+XXXX` Unicode escapes (how files before DXF 2007 hold characters outside their code page) -/// and caret escapes: `^ ` is a caret, `^I` a tab; other control characters are dropped. With -/// `keep_stacks`, the `^` in an MTEXT stacked fraction `\Snum^den;` is left for the caller. -fn escapes(s: &str, keep_stacks: bool) -> String { +/// and caret escapes: `^ ` is a caret, `^I` a tab; other control characters are dropped. In +/// multiline text (`mtext`) `\\` is an escaped backslash and is kept as is, so that a `U+` after +/// it stays literal, and the `^` in a stacked fraction `\Snum^den;` is left for the caller. +fn escapes(s: &str, mtext: bool) -> String { let mut out = String::with_capacity(s.len()); let mut rest = s; while let Some(c) = rest.chars().next() { let after = rest.get(c.len_utf8()..).unwrap_or(""); + if mtext && c == '\\' && after.starts_with('\\') { + out.push_str("\\\\"); + rest = after.get(1..).unwrap_or(""); + continue; + } if c == '\\' && let Some(hex) = after.strip_prefix("U+").or_else(|| after.strip_prefix("u+")).and_then(|h| h.get(..4)) && hex.bytes().all(|b| b.is_ascii_hexdigit()) @@ -79,7 +85,7 @@ fn escapes(s: &str, keep_stacks: bool) -> String { rest = after.get(6..).unwrap_or(""); continue; } - if keep_stacks + if mtext && c == '\\' && let Some(tail) = after.strip_prefix('S') { @@ -175,5 +181,7 @@ mod tests { assert_eq!(mtext("\\S1^2;").0, "1/2"); assert_eq!(mtext("\\S+1^-2;^I").0, "+1/-2\t"); assert_eq!(mtext("trailing\\").0, "trailing\\"); + assert_eq!(mtext("a\\\\U+0041b").0, "a\\U+0041b", "an escaped backslash is not a Unicode escape"); + assert_eq!(mtext("a\\U+0041b").0, "aAb"); } } From 4821ec92c2f950220cb2844995a14b5d305dfd6b Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 22 Oct 2026 17:31:59 -0700 Subject: [PATCH 07/12] Control: requests are answered at full speed while the window is hidden on macOS Within about a minute of its window no longer being visible, macOS App Nap lowers the app's threads to the lowest priority and coalesces its timers. On a loaded machine, control requests then took seconds to answer, and the 60 s timeout reply arrived up to 26 s late. When the control channel is on (--control or VECTORCRAFT_CONTROL_PORT), the app now holds an NSProcessInfo activity (user initiated, allowing idle system sleep, latency critical) for as long as it runs, so App Nap does not apply. The activity does not keep the Mac awake. docs/control-protocol.md states this in the paragraph on the 60 s timeout. --- apps/vectorcraft/Cargo.toml | 4 +-- apps/vectorcraft/src/control_server.rs | 34 ++++++++++++++++++++++++++ docs/control-protocol.md | 3 ++- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/apps/vectorcraft/Cargo.toml b/apps/vectorcraft/Cargo.toml index 86235afbb..fedc4acbe 100644 --- a/apps/vectorcraft/Cargo.toml +++ b/apps/vectorcraft/Cargo.toml @@ -62,8 +62,8 @@ windows = { version = "0.62", features = ["Win32_Graphics_DirectWrite", "Win32_U muda = { version = "=0.21.1", default-features = false } objc2 = "0.6" objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "block2", "objc2-core-foundation", "NSApplication", "NSButton", "NSColor", "NSColorSampler", "NSColorSpace", "NSControl", "NSMenu", "NSEvent", "NSPasteboard", "NSPopUpButton", "NSResponder", "NSSavePanel", "NSTextField", "NSTextInputContext", "NSView", "NSWindow"] } -# Files opened from Finder and the Dock (src/open_documents.rs) and pasted bitmaps (src/clipboard.rs); both already come with winit, muda and rfd. -objc2-foundation = { version = "0.3", default-features = false, features = ["std", "block2", "NSArray", "NSData", "NSGeometry", "NSNotification", "NSOperation", "NSString", "NSURL"] } +# Files opened from Finder and the Dock (src/open_documents.rs), pasted bitmaps (src/clipboard.rs) and the control channel's App Nap activity (src/control_server.rs); both already come with winit, muda and rfd. +objc2-foundation = { version = "0.3", default-features = false, features = ["std", "block2", "NSArray", "NSData", "NSGeometry", "NSNotification", "NSOperation", "NSProcessInfo", "NSString", "NSURL"] } block2 = "0.6" # The window's AppKit view, to centre its buttons on the app bar (src/mac_window.rs). MIT OR # Apache-2.0 OR Zlib, already in the build through eframe and winit. diff --git a/apps/vectorcraft/src/control_server.rs b/apps/vectorcraft/src/control_server.rs index 529100513..e163ffb95 100644 --- a/apps/vectorcraft/src/control_server.rs +++ b/apps/vectorcraft/src/control_server.rs @@ -67,10 +67,31 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver { } }; eprintln!("vectorcraft: control server listening on 127.0.0.1:{port}"); + #[cfg(target_os = "macos")] + keep_answering(); std::thread::spawn(move || accept_loop(listener, tx, ctx)); rx } +/// The activity the control channel holds on macOS: work the user asked for, which may let the +/// system sleep when idle, with full timer precision. Without it, App Nap lowers every thread to +/// the lowest priority within about a minute of the window no longer being visible. +#[cfg(target_os = "macos")] +fn activity_options() -> objc2_foundation::NSActivityOptions { + use objc2_foundation::NSActivityOptions; + NSActivityOptions::UserInitiatedAllowingIdleSystemSleep | NSActivityOptions::LatencyCritical +} + +/// Begins the control channel's activity. It lasts until the process ends. +#[cfg(target_os = "macos")] +fn keep_answering() { + use objc2_foundation::{NSProcessInfo, NSString}; + let reason = NSString::from_str("Answering control requests"); + let activity = NSProcessInfo::processInfo().beginActivityWithOptions_reason(activity_options(), &reason); + // The token is never released or passed to `endActivity`, so the activity lasts for the process's lifetime. + std::mem::forget(activity); +} + fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Context) { let open = Arc::new(AtomicUsize::new(0)); for stream in listener.incoming().flatten() { @@ -642,4 +663,17 @@ mod tests { assert!(line.contains("too many control connections"), "{line}"); drop(held); } + + // A test process is not an app that App Nap applies to, so this checks the activity the + // channel asks for; the throttling itself shows only in a running app with its window hidden. + #[cfg(target_os = "macos")] + #[test] + fn the_control_channel_keeps_the_app_at_full_priority_on_macos() { + use objc2_foundation::NSActivityOptions; + let options = activity_options(); + assert!(options.contains(NSActivityOptions::LatencyCritical), "without it App Nap still lowers the priority to 20"); + assert!(options.contains(NSActivityOptions::UserInitiatedAllowingIdleSystemSleep)); + assert!(!options.contains(NSActivityOptions::IdleSystemSleepDisabled), "the channel must not keep the Mac awake"); + keep_answering(); + } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 0ec4cfd87..3f60aae80 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -51,7 +51,8 @@ A request the app hasn't answered after **60 s** gets `{"ok": false, "error": "t app had not started it (it was busy, or its window wasn't being drawn), the request is withdrawn and never runs, and the error says "will not run": retrying is safe. If the app was already running it, the error says "still running": it may yet complete, so check the document before -retrying. +retrying. On macOS the app keeps itself out of App Nap while the control channel is on, so it keeps +answering with its window hidden, minimized or covered, or with the screen locked. ## Confining file access From 4e9cd60540f8a139c53ebb7d8a0e34515f964074 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:58:30 +0000 Subject: [PATCH 08/12] Escape spaces before x and a separator in XML ids Found while testing crates/doc/src/xml_id.rs:32 (from_object_name), decoded by escaped() at :72 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/doc/src/xml_id.rs | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/doc/src/xml_id.rs b/crates/doc/src/xml_id.rs index ba454de01..08dd5edbe 100644 --- a/crates/doc/src/xml_id.rs +++ b/crates/doc/src/xml_id.rs @@ -32,9 +32,11 @@ pub fn to_object_name(id: &str) -> String { pub fn from_object_name(name: &str) -> String { let mut out = String::with_capacity(name.len().saturating_mul(2)); let last = name.chars().count().saturating_sub(1); - for (i, c) in name.chars().enumerate() { - if c == ' ' && i == last { - // A trailing bare `_` could read back as the end of a uniqueness suffix. + let chars: Vec = name.chars().collect(); + for (i, &c) in chars.iter().enumerate() { + if c == ' ' && (i == last || starts_fake_escape(chars.get(i + 1..).unwrap_or_default())) { + // A trailing bare `_` could read back as the end of a uniqueness suffix, and a bare + // `_` before `x_` would read back as an `_xHH_` escape. out.push_str("_x20_"); } else if c == ' ' { out.push('_'); @@ -68,6 +70,19 @@ pub fn is_valid(id: &str) -> bool { chars.all(|c| c.is_alphanumeric() || matches!(c, '_' | '.' | '-' | ':')) } +/// Whether the encoded `rest` (after a space) starts with `x`, 1 to 6 hex digits and a character +/// whose encoding begins with `_`, so that a bare `_` before it would form an escape. +fn starts_fake_escape(rest: &[char]) -> bool { + let Some(('x', digits)) = rest.split_first() else { + return false; + }; + let n = digits.iter().take_while(|c| c.is_ascii_hexdigit()).count(); + if n == 0 || n > 6 { + return false; + } + digits.get(n).is_some_and(|&c| c == ' ' || c == '_' || !(c.is_alphanumeric() || matches!(c, '.' | '-' | ':'))) +} + /// `_xHHHH_` at the start of `s`: the character and the escape's length. fn escaped(s: &str) -> Option<(char, usize)> { let body = s.strip_prefix("_x")?; @@ -119,6 +134,15 @@ mod tests { assert_eq!(to_object_name("a_x-41_b"), "a x-41 b"); } + #[test] + fn space_before_x_hex_and_underscore_is_not_an_escape() { + for name in ["layer x1 copy", "a xa b", "a xe b", "a xb b", "a x1(b", "a x1_b", "a x123456 c"] { + let id = from_object_name(name); + assert_eq!(to_object_name(&id), name, "name {name:?} -> id {id:?}"); + } + assert_eq!(from_object_name("layer x1 copy"), "layer_x20_x1_copy"); + } + #[test] fn round_trip_names_that_cannot_start_an_xml_name() { for name in ["-abc", ".5 mm", "²x", "Ⅷ", "", "1abc", "Layer 1", " lead", "_a", ":a", "#"] { From aa8adf0891317bd49b84d9b2642944d1f30dafc7 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:50:46 +0200 Subject: [PATCH 09/12] 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 --- crates/scriptcheck/src/lib.rs | 45 ++++++++++++++++++++++-- crates/scriptcheck/src/report.rs | 1 + crates/scriptcheck/src/tests.rs | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 3 deletions(-) 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()); +} From c607d2b448cec5e5e4a570bf23bcd7f2664bf93c Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 03:30:13 -0700 Subject: [PATCH 10/12] Pencil: the closing cursor shows while a stroke is drawn back near its start A Pencil or Paintbrush stroke released within Close Paths When Ends Are Within of its start closes, but the cursor gave no sign of it. While the pointer is where a release would close the stroke, the cursor is now PencilClose (PaintbrushClose for the Paintbrush), the crosshair with a ring, as Draw straight lines with the Pencil tool describes: https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/draw-shapes/draw-straight-lines-with-the-pencil-tool.html The check is the one the release makes, on the end the stroke would take (snapped for the Pencil). A straight line (Shift or Alt held at the press), a stroke that continues a selected path, a stroke that redraws one and a smoothing drag show no ring. Docs: docs/target-app-parity.md, docs/ui-parity.md. --- crates/tools/src/cursors.toml | 10 +++- crates/tools/src/draw2/gesture.rs | 79 ++++++++++++++++++++++++++++++- crates/tools/src/lib.rs | 6 ++- docs/target-app-parity.md | 2 +- docs/ui-parity.md | 1 + 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/crates/tools/src/cursors.toml b/crates/tools/src/cursors.toml index 9777db652..76c0f9ba6 100644 --- a/crates/tools/src/cursors.toml +++ b/crates/tools/src/cursors.toml @@ -536,7 +536,10 @@ hotspot = "The lead tip." [[cursor]] id = "PencilClose" group = "Drawing and editing tools" -shown_when = ["The Pencil tool with Alt pressed after the drag starts, when Alt key toggles to Smooth Tool is off: releasing the mouse button with Alt still held closes the path. A drag started with Shift or Alt held draws an open straight line and does not show this cursor."] +shown_when = [ + "The Pencil tool with Alt pressed after the drag starts, when Alt key toggles to Smooth Tool is off: releasing the mouse button with Alt still held closes the path. A drag started with Shift or Alt held draws an open straight line and does not show this cursor.", + "The Pencil tool while a freehand stroke is drawn back to within Close Paths When Ends Are Within of its start: releasing the mouse button there closes the path.", +] hotspot = "The lead tip." look = "The Pencil cursor with a ring badge (closing the loop), like PenClose." @@ -556,7 +559,10 @@ hotspot = "The bristle tip." [[cursor]] id = "PaintbrushClose" group = "Drawing and editing tools" -shown_when = ["The Paintbrush tool with Alt held, before a press and while drawing: releasing the mouse button with Alt still held closes the path."] +shown_when = [ + "The Paintbrush tool with Alt held, before a press and while drawing: releasing the mouse button with Alt still held closes the path.", + "The Paintbrush tool while a stroke is drawn back to within Close Paths When Ends Are Within of its start: releasing the mouse button there closes the path.", +] hotspot = "The bristle tip." look = "The Paintbrush cursor with a ring badge (closing the loop), like PenClose." diff --git a/crates/tools/src/draw2/gesture.rs b/crates/tools/src/draw2/gesture.rs index daca68d6a..0f2274670 100644 --- a/crates/tools/src/draw2/gesture.rs +++ b/crates/tools/src/draw2/gesture.rs @@ -30,6 +30,9 @@ pub struct GestureTool { pressures: Vec, active: bool, began: bool, + /// A release where the pointer last dragged to would close the stroke because it ends near + /// its start (Close Paths When Ends Are Within): the cursor shows the closing ring. + near_start: bool, /// Pencil/Paintbrush continuing a selected open path: (path, continue from its start). extend: Option<(NodeId, bool)>, /// Pencil/Paintbrush starting on a selected path away from its ends: (path, Edit Selected @@ -93,6 +96,7 @@ impl GestureTool { pressures: vec![], active: false, began: false, + near_start: false, extend: None, redraw: None, fidelity, @@ -250,6 +254,19 @@ impl GestureTool { }) } + /// Does a stroke that ends at `end` end near its start (Close Paths When Ends Are Within, off + /// at 0 pixels)? + fn ends_near_start(&self, cx: &ToolContext, end: Point) -> bool { + self.close_within > 0.0 && self.points.len() > 2 && self.points[0].distance(end) <= cx.tol(self.close_within) + } + + /// Does a release now, with the stroke ending at `end`, close a freehand stroke because it ends + /// near its start (`finish`)? A stroke that redraws a selected path closes only when the engine + /// finds it does not end on that path, so it counts as not closing. + fn release_closes_near_start(&self, cx: &ToolContext, end: Point) -> bool { + self.draws_path() && !self.smoothing && !self.straight && self.extend.is_none() && self.redraw.is_none() && self.ends_near_start(cx, end) + } + /// Parameters of `path.freehand` for the current stroke. pub fn freehand_params(&self, closed: bool) -> Value { let mut v = json!({ @@ -284,7 +301,7 @@ impl GestureTool { return vec![]; } // Close Paths When Ends Are Within is off at 0 pixels. - let near = self.close_within > 0.0 && pts.len() > 2 && pts[0].distance(pts[pts.len() - 1]) <= cx.tol(self.close_within); + let near = pts.last().is_some_and(|&end| self.ends_near_start(cx, end)); // A straight line has two points: closing it would draw it back over itself. let closed = !self.straight && ((m.alt && !self.alt_smooth) || (self.extend.is_none() && near)); vec![Action::Preview("path.freehand".into(), self.freehand_params(closed)), Action::Commit] @@ -327,6 +344,7 @@ impl Tool for GestureTool { PointerKind::Down => { self.active = true; self.began = false; + self.near_start = false; self.smoothing = self.id == "pencil" && self.alt_smooth && ev.mods.alt; // Alt draws a straight line only while it doesn't lend the Smooth tool; with Shift // too, the line keeps Shift's 45° steps. @@ -356,9 +374,10 @@ impl Tool for GestureTool { if !self.began && !self.straight && self.id == "pencil" && self.alt_smooth && ev.mods.alt { self.smoothing = true; } + let mut end = p; if self.snaps() && !self.smoothing && !self.straight { // Where the stroke would end: shown, taken on release. - self.snap.drag(cx, p, None); + end = self.snap.drag(cx, p, None); } let p = if self.straight && self.unconstrained && self.snaps() { // Alt's line ends where a stroke would: shown, taken on release. @@ -379,9 +398,11 @@ impl Tool for GestureTool { } self.points.truncate(1); } else if self.points.last().is_some_and(|l| l.distance(p) < cx.tol(1.0)) || self.points.len() >= crate::MAX_STROKE_SAMPLES { + self.near_start = self.release_closes_near_start(cx, end); return vec![]; } self.push(p, ev.pressure); + self.near_start = self.release_closes_near_start(cx, end); if !self.draws_path() || self.smoothing { return vec![]; } @@ -424,6 +445,7 @@ impl Tool for GestureTool { let out = if cancel { vec![Action::Cancel] } else { self.finish(cx, ev.mods) }; self.active = false; self.began = false; + self.near_start = false; self.smoothing = false; self.straight = false; self.unconstrained = false; @@ -446,6 +468,7 @@ impl Tool for GestureTool { self.snap.clear(); self.active = false; self.began = false; + self.near_start = false; self.smoothing = false; self.straight = false; self.unconstrained = false; @@ -470,6 +493,7 @@ impl Tool for GestureTool { .then(|| Action::Preview("path.freehand".into(), self.freehand_params(false))); self.snap.clear(); self.active = false; + self.near_start = false; self.began = false; self.straight = false; self.unconstrained = false; @@ -505,6 +529,11 @@ impl Tool for GestureTool { if self.draws_path() && !self.alt_smooth && m.alt && !self.straight && (self.active || self.id != "pencil") { return if self.id == "paintbrush" { Cursor::PaintbrushClose } else { Cursor::PencilClose }; } + // A release near a freehand stroke's start closes it too (`finish`): the ring shows while + // the pointer is there. + if self.draws_path() && self.active && self.near_start { + return if self.id == "paintbrush" { Cursor::PaintbrushClose } else { Cursor::PencilClose }; + } match self.id { "paintbrush" => Cursor::Paintbrush, "blobBrush" => Cursor::BlobBrush, @@ -1142,6 +1171,52 @@ mod tests { } } + /// While a stroke is drawn back near its start, the cursor shows the closing ring when a + /// release there closes the path (Close Paths When Ends Are Within), and the tool's own cursor + /// away from the start, at 0 pixels and after the release. + #[test] + fn a_stroke_near_its_start_shows_the_closing_cursor() { + let (d, _) = doc_with_rect(); + let s = Selection::default(); + let p = paint(); + let cx = cx(&d, &s, &p); + let none = Mods::default(); + let tools = [("pencil", Cursor::Pencil, Cursor::PencilClose), ("paintbrush", Cursor::Paintbrush, Cursor::PaintbrushClose)]; + for (tool, own, closing) in tools { + for within in [15.0, 0.0] { + let near = if within > 0.0 { closing } else { own }; + let mut t = GestureTool::new(tool); + t.set_option("closeWithin", &json!(within)); + let mut at = |k, x, y| { + let acts = t.pointer(&cx, &PointerEvent::new(k, x, y)); + (acts, t.cursor(&cx, Point::new(x, y), none)) + }; + assert_eq!(at(PointerKind::Down, 310.0, 330.0).1, own, "{tool} at the press"); + assert_eq!(at(PointerKind::Drag, 360.0, 350.0).1, own, "{tool} away from the start"); + assert_eq!(at(PointerKind::Drag, 330.0, 390.0).1, own, "{tool} away from the start"); + assert_eq!(at(PointerKind::Drag, 318.0, 335.0).1, near, "{tool} near the start at {within} px"); + assert_eq!(at(PointerKind::Drag, 345.0, 340.0).1, own, "{tool} away from the start again"); + assert_eq!(at(PointerKind::Drag, 318.0, 335.0).1, near, "{tool} back near the start at {within} px"); + let (acts, after) = at(PointerKind::Up, 318.0, 335.0); + let Some(Action::Preview(_, v)) = acts.first() else { panic!("{tool}: {acts:?}") }; + assert_eq!(v["closed"], json!(within > 0.0), "{tool}: the release does what the cursor showed"); + assert_eq!(after, own, "{tool} after the release"); + } + } + // A straight Pencil line (Shift or Alt held at the press) stays open, so it shows no ring + // near its start. + for held in [Mods { shift: true, ..Mods::default() }, Mods { alt: true, ..Mods::default() }] { + let mut t = GestureTool::new("pencil"); + t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 310.0, 330.0).with_mods(held)); + for (x, y) in [(360.0, 330.0), (330.0, 390.0), (318.0, 335.0)] { + t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, x, y).with_mods(held)); + } + for m in [held, none] { + assert_eq!(t.cursor(&cx, Point::new(318.0, 335.0), m), Cursor::Pencil, "a straight line, {held:?}, {m:?}"); + } + } + } + #[test] fn eraser_and_knife_exec_on_release() { let (d, _) = doc_with_rect(); diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index d06e9a2da..2f3373da1 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -576,7 +576,8 @@ pub enum Cursor { Pencil, /// The Pencil tool with Alt pressed after the drag starts and Alt key toggles to Smooth Tool off: /// releasing the mouse button with Alt still held closes the path. A drag started with Shift or - /// Alt held draws an open straight line instead. A crosshair with a ring. + /// Alt held draws an open straight line instead. Also while a freehand stroke is drawn back near + /// its start, where releasing the mouse button closes the path. A crosshair with a ring. PencilClose, /// The Pencil tool before a press that draws a straight line (Shift or Alt held). A crosshair with a /// short straight line. @@ -584,7 +585,8 @@ pub enum Cursor { /// The Paintbrush tool. Paintbrush, /// The Paintbrush tool with Alt held: releasing the mouse button with Alt still held closes - /// the path. A crosshair with a ring. + /// the path. Also while a stroke is drawn back near its start, where releasing the mouse button + /// closes the path. A crosshair with a ring. PaintbrushClose, /// The Blob Brush tool. BlobBrush, diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index bc1fe1fdd..16dd4b6c7 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -631,7 +631,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Saved selections:** Select ▸ Save Selection… asks for a name and keeps the selection in the document (saved with the file, undoable, up to 25); each one is listed at the bottom of the Select menu and a click selects its objects again; Edit Selection… lists them to rename or delete, applied together on OK. A file's saved selections are capped and checked against its objects when it opens. Select ▸ Update Selection replaces the objects of the saved selection last recalled with the selected objects, in one undo step that marks the document changed (`select.update`). After Edit Selection… renames that saved selection, Update Selection replaces it under the new name; after Edit Selection… deletes it, Update Selection is disabled until another saved selection is recalled or the delete is undone. Undo and Redo do not change which saved selection is recalled. - **Drawing modes:** in Draw Inside, a double-click with a selection tool outside the object being drawn into sets the drawing mode to Draw Normal, as Shift+D does. Pastes follow the drawing mode ([drawing modes](https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/learn-drawing-basics/drawing-modes-overview.html)) (#3705): in Draw Inside, Paste, Paste in Place and Paste on All Artboards put the copies inside the object drawn into, in one undo step, Paste Remembers Layers aside; in Draw Behind, Paste in Place and Paste on All Artboards put them just below the selection; Paste in Front and Paste in Back ignore the mode. Shift+D cycles Draw Normal → Draw Behind → Draw Inside → Draw Normal, skipping Draw Inside unless one path, compound path or type is selected. With the object drawn into isolated (a double-click on it with the Selection tool), art drawn or pasted inside it takes isolation mode into its clip group, so it can be clicked and isn't dimmed, as art added in isolation mode belongs with what is isolated ([isolation mode](https://helpx.adobe.com/illustrator/desktop/manage-objects/select-objects/isolate-objects.html)) (#3705); undo or redo that removes the isolated object ends isolation mode. - - **Drawing:** Pen, Curvature, anchor tools, Pencil, Paintbrush, Blob Brush, Smooth, Path Eraser, Join. Direction handles dragged with Direct Selection or the Anchor Point tool snap to smart guides, and Shift keeps them at 45° steps from their anchor. The Pen continues any open path from the end clicked, and while drawing, a click on an end of another open path joins the two into one (#776). The Curvature tool edits any selected path, whatever drew it, keeping its shape except where edited (#798): a click on a point makes it current and a drag moves it, re-curving only its two segments; Alt-click or double-click toggles it smooth or corner and Delete removes it; a click on a segment adds a point without changing the shape; with an end of an open path current, clicks go on from it and its other end closes it (`path.curvatureEdit`). Every point a drawing tool places snaps to Smart Guides through one snapper (#506): the Pen's and Curvature tool's anchors, the Pencil's ends, the shape tools' start and dragged corner or radius (Rectangle, Rounded Rectangle, Ellipse, Polygon, Star, Line Segment, Arc, Spiral, the grids), the Type tool's click and frame, a new artboard's corners. A filled Pencil path keeps a stroke of None (#1804, part of #1679). The [ and ] keys reach the Blob Brush, the Eraser and the Symbolism tools in the app (#1734, part of #1661). + - **Drawing:** Pen, Curvature, anchor tools, Pencil, Paintbrush, Blob Brush, Smooth, Path Eraser, Join. Direction handles dragged with Direct Selection or the Anchor Point tool snap to smart guides, and Shift keeps them at 45° steps from their anchor. The Pen continues any open path from the end clicked, and while drawing, a click on an end of another open path joins the two into one (#776). The Curvature tool edits any selected path, whatever drew it, keeping its shape except where edited (#798): a click on a point makes it current and a drag moves it, re-curving only its two segments; Alt-click or double-click toggles it smooth or corner and Delete removes it; a click on a segment adds a point without changing the shape; with an end of an open path current, clicks go on from it and its other end closes it (`path.curvatureEdit`). Every point a drawing tool places snaps to Smart Guides through one snapper (#506): the Pen's and Curvature tool's anchors, the Pencil's ends, the shape tools' start and dragged corner or radius (Rectangle, Rounded Rectangle, Ellipse, Polygon, Star, Line Segment, Arc, Spiral, the grids), the Type tool's click and frame, a new artboard's corners. A filled Pencil path keeps a stroke of None (#1804, part of #1679). While a Pencil or Paintbrush stroke is drawn back to within Close Paths When Ends Are Within of its start, the cursor shows the closing ring and a release there closes the path, as [Draw straight lines with the Pencil tool](https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/draw-shapes/draw-straight-lines-with-the-pencil-tool.html) describes for the Pencil. The [ and ] keys reach the Blob Brush, the Eraser and the Symbolism tools in the app (#1734, part of #1661). With the Blob Brush, Shift held during a drag keeps the stroke on the horizontal or vertical line through the press point, whichever is nearer to the pointer (90° steps from the Constrain Angle preference), and the points dragged since Shift went down move to the other line when the pointer crosses to it; the points dragged while Shift is up follow the pointer. Curvature point markers are circular, with no direction handles. While drawing, its own point feedback replaces generic selection markers; existing paths use the selection overlay, including partial selections and Hide Edges. Anchor Size applies, and Direct Selection and Pen retain their handles. The real egui-frame regressions in `crates/ui-egui/src/canvas.rs` verify marker positions, counts and unchanged documents (#1236, in part); the existing-path interpolation request remains open. The curve through the points (`curvature_spline` in `crates/tools/src/draw2.rs`) makes each stretch of three points follow the circle through them: a smooth point's handles lie along that circle's tangent, each `chord / (3 cos²(α/2))` long (α between handle and chord, capped at 90°), and the ends of an open path mirror their neighbour's tangent in their chord, so a J or a U curves evenly, a turn tighter than 30° comes to a point, and uneven spacing doesn't loop; the rubber band, a dragged or inserted point and `path.convertAnchor` to smooth use the same rule, while the Pencil's fitting keeps its own; `tests_draw2.rs` checks that the rubber band is the path the click makes (#1236, in part). The Smooth tool smooths only the stretch of path under the drag (#1547, fixes #1512). While Alt is held, the Paintbrush, and the Pencil with Alt key toggles to Smooth Tool off, show a closing cursor (a crosshair with a ring); releasing the mouse button with Alt still held closes the path. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 5eada8944..a6d050d4c 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -91,6 +91,7 @@ tools, never checked side by side) isn't captured row by row. | Curvature tool: click/drag points, double-click or Alt-click for a corner, temporary Direct Selection, point conversion | partial | circular point markers without direction handles or duplicate markers, Anchor Size and Hide Edges regression-tested (#1236, in part); curve geometry: each stretch of three points follows the circle through them (handles along its tangent, chord/(3·cos²(α/2)) long, shorter in a turn tighter than 30°), open ends curving in, the same rule for the rubber band, dragged, inserted and converted points (#1236, in part); neighbours of a point dragged on an existing path keep their tangents, and modifiers requested in #975 remain | | Blend spine Anchor Point conversion and handle editing (#2544) | implemented | symmetric handles, independent drag/retraction, visible-handle policy and selection/stacking covered by tool and engine regressions; Windows native canvas checks and screenshots reviewed in PR #2643 | | Add/Delete/Anchor Point tools; Anchor Point tool drags segments to reshape | done | #2267: a click on a handle end retracts that handle and leaves the anchor a corner with the other one; Shift held at the press on an anchor moves it, handles and all (Shift kept down: at 45° steps) instead of pulling out handles | +| Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | done | Alt key toggles to Smooth Tool (#2001); Shift held at the press draws a straight line at 45° steps from the Constrain Angle, and Alt held at the press (with Alt key toggles to Smooth Tool off) a straight line at any angle (#3799); Edit Selected Paths continues a selected path from an end and, from a stroke that starts and ends on a selected path, redraws the section between (the shorter way round a closed path, #2850); with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679); while a freehand stroke is drawn back to within Close Paths When Ends Are Within of its start, the Pencil and the Paintbrush show the closing cursor, and a release there closes the path | | Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | done | Alt key toggles to Smooth Tool (#2001); Shift held at the press draws a straight line at 45° steps from the Constrain Angle, with a straight-line cursor while Shift is held before the press (part of #3799; Alt's unconstrained straight line is not done); Edit Selected Paths continues a selected path from an end and, from a stroke that starts and ends on a selected path, redraws the section between (the shorter way round a closed path, #2850); with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679) | | Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | done | Alt key toggles to Smooth Tool (#2001); Shift held at the press draws a straight line at 45° steps from the Constrain Angle, and Alt held at the press (with Alt key toggles to Smooth Tool off) a straight line at any angle (#3799); Edit Selected Paths continues a selected path from an end and, from a stroke that starts and ends on a selected path, redraws the section between (the shorter way round a closed path, #2850); with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679) | | Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | partial | Alt to Smooth is missing; Edit Selected Paths continues a selected path from an end and, from a stroke that starts and ends on a selected path, redraws the section between (the shorter way round a closed path, #2850); with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679) | From aabf48728ea10f96517f6e1888550657adbb1203 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 10:04:41 -0400 Subject: [PATCH 11/12] Pencil: the closing ring tests the point a release keeps A Paintbrush sample dropped for being under a pixel from the last point left the ring computed from the pointer, while the release keeps the last point; the ring now follows that point (CodeRabbit). Co-Authored-By: Claude Opus 5.5 --- crates/tools/src/draw2/gesture.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/tools/src/draw2/gesture.rs b/crates/tools/src/draw2/gesture.rs index 0f2274670..4806b42a2 100644 --- a/crates/tools/src/draw2/gesture.rs +++ b/crates/tools/src/draw2/gesture.rs @@ -398,7 +398,11 @@ impl Tool for GestureTool { } self.points.truncate(1); } else if self.points.last().is_some_and(|l| l.distance(p) < cx.tol(1.0)) || self.points.len() >= crate::MAX_STROKE_SAMPLES { - self.near_start = self.release_closes_near_start(cx, end); + // A release here keeps the last point unless it snaps (`Up`), so the ring + // follows the point `finish` will test. + let kept = self.points.last().copied().filter(|l| l.distance(p) < cx.tol(1.0) && !self.snaps()); + let release_end = kept.unwrap_or(end); + self.near_start = self.release_closes_near_start(cx, release_end); return vec![]; } self.push(p, ev.pressure); From efff645a8e97e649081c3a31ad056c17bbe87a79 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 06:58:19 -0700 Subject: [PATCH 12/12] Shape Builder: rebuild the region map for each document revision The Shape Builder, Live Paint Bucket and Live Paint Selection keep the selection's region map between pointer events, keyed on the document's address and the selected ids. Every edit replaces the document, and after an undo followed by an edit a later document can be allocated at the address of the one the map was built from. The key then matched a map of an earlier revision, so a drag across the art merged nothing and a Live Paint Bucket click filled nothing. The key now also holds the tool context's revision, which moves with every edit, undo and selection change. --- crates/tools/src/builder.rs | 55 +++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/crates/tools/src/builder.rs b/crates/tools/src/builder.rs index 5c228b932..385ed7bd9 100644 --- a/crates/tools/src/builder.rs +++ b/crates/tools/src/builder.rs @@ -538,11 +538,15 @@ pub fn hatch(path: &BezPath, gap: f64) -> BezPath { out } -/// The planar map of the current selection, cached by document identity, selection and whether -/// filled open paths count as closed. +/// What a [`RegionCache`] map was built for: the document's address and revision, the selected ids +/// and whether filled open paths count as closed. +type CacheKey = (usize, (u64, u64), Vec, bool); + +/// The planar map of the current selection, cached by document identity, revision, selection and +/// whether filled open paths count as closed. #[derive(Default)] struct RegionCache { - key: Option<(usize, Vec, bool)>, + key: Option, map: BuilderMap, /// Live Paint faces (every path an edge, filled or not) rather than Shape Builder regions. live: bool, @@ -561,7 +565,7 @@ impl RegionCache { /// The map, with the outline edges when `edges` (built the first time they're wanted), and /// filled open paths closed when `open_filled_as_closed` (Shape Builder only). fn get(&mut self, cx: &ToolContext, edges: bool, open_filled_as_closed: bool) -> &BuilderMap { - let key = (cx.doc as *const Document as usize, cx.selection.objects.clone(), open_filled_as_closed); + let key = (cx.doc as *const Document as usize, cx.revision, cx.selection.objects.clone(), open_filled_as_closed); if self.key.as_ref() != Some(&key) || (edges && !self.edges) { let roots = sorted_roots(cx.doc, &cx.selection.objects); self.map = if self.live { @@ -1284,12 +1288,17 @@ mod tests { use vectorcraft_geom::shapes; fn two_rects() -> (Document, NodeId, NodeId) { + two_rects_at(0.0) + } + + /// Squares A (`x`..`x`+100) and B (`x`+50..`x`+150), 100 pt tall, overlapping by half. + fn two_rects_at(x: f64) -> (Document, NodeId, NodeId) { let mut d = Document::new(500.0, 500.0); let l = d.layers[0].id; let a = d.alloc_id(); - d.insert(Some(l), 0, Node::path(a, shapes::rectangle(Rect::new(0.0, 0.0, 100.0, 100.0)), Appearance::default_art())).unwrap(); + d.insert(Some(l), 0, Node::path(a, shapes::rectangle(Rect::new(x, 0.0, x + 100.0, 100.0)), Appearance::default_art())).unwrap(); let b = d.alloc_id(); - d.insert(Some(l), 1, Node::path(b, shapes::rectangle(Rect::new(50.0, 0.0, 150.0, 100.0)), Appearance::default_art())).unwrap(); + d.insert(Some(l), 1, Node::path(b, shapes::rectangle(Rect::new(x + 50.0, 0.0, x + 150.0, 100.0)), Appearance::default_art())).unwrap(); (d, a, b) } @@ -1335,6 +1344,24 @@ mod tests { assert!(v["points"].as_array().unwrap().len() >= 2); } + #[test] + fn shape_builder_rebuilds_its_map_for_a_new_revision_at_the_same_address() { + let (mut d, a, b) = two_rects(); + let mut s = Selection::default(); + s.set([a, b]); + let p = paint(); + let mut t = ShapeBuilderTool::default(); + let cx1 = ToolContext { revision: (1, 1), ..cx(&d, &s, &p) }; + t.pointer(&cx1, &ev(PointerKind::Move, 75.0, 50.0)); + // A later revision of the document, with the squares 300 pt to the right, at the same address. + d = two_rects_at(300.0).0; + let cx2 = ToolContext { revision: (1, 2), ..cx(&d, &s, &p) }; + t.pointer(&cx2, &ev(PointerKind::Down, 325.0, 50.0)); + t.pointer(&cx2, &ev(PointerKind::Drag, 375.0, 50.0)); + let acts = t.pointer(&cx2, &ev(PointerKind::Up, 375.0, 50.0)); + assert!(matches!(acts.as_slice(), [Action::Exec(id, _)] if id == "shapeBuilder.merge"), "{acts:?}"); + } + #[test] fn shape_builder_alt_drag_erases_and_empty_drag_does_nothing() { let (d, a, b) = two_rects(); @@ -1737,6 +1764,22 @@ mod tests { assert_eq!(v["ids"], json!([a.0, b.0])); } + #[test] + fn bucket_rebuilds_its_map_for_a_new_revision_at_the_same_address() { + let (mut d, a, b) = two_rects(); + let mut s = Selection::default(); + s.set([a, b]); + let p = paint(); + let mut t = LivePaintBucketTool::default(); + let cx1 = ToolContext { revision: (1, 1), ..cx(&d, &s, &p) }; + t.pointer(&cx1, &ev(PointerKind::Move, 75.0, 50.0)); + // A later revision of the document, with the squares 300 pt to the right, at the same address. + d = two_rects_at(300.0).0; + let cx2 = ToolContext { revision: (1, 2), ..cx(&d, &s, &p) }; + let acts = t.pointer(&cx2, &ev(PointerKind::Down, 375.0, 50.0)); + assert_eq!(acts, vec![Action::Exec("livePaint.fill".into(), json!({ "ids": [a.0, b.0], "point": [375.0, 50.0] }))]); + } + #[test] fn live_paint_selection_selects_faces() { let (d, _gid, f2, e) = live_group_doc();