starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-22 08:02:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(mobile): make the flip comparator refuse what it was accepting

Browse Source 
Round 2 items 6 and 7, both in the equivalence instrument.

6. `isPrinterDisambiguation` accepted any `name2` facing `name` without proving
   the two were the same binding, so an unrelated rename ending in a digit would
   have been counted rather than refused. It is replaced by `UNSHADOWED_RENAMES`,
   an explicit list of pre-flip name, generated name and declaring module. The
   whole script has one entry: `term2` -> `term` in `query-reply`, which is the
   `term` parameter of `attachTerminalQueryReplyBridge` and its six uses, seven
   sites in all. That is stated in the docstring rather than encoded as a second
   pin, since the flip test already pins the total.

7. Brace absorption treated every unexpected `{` as a linter-added body and
   absorbed any later `}` while one was outstanding, so a bare block anywhere
   would have been swallowed. `isBraceableHeadBody` now requires the open to be
   the body of `if`, `for`, `while`, `else` or `do` — walking a `)` back to its
   `(` and reading the keyword before it — and `matchingCloseIndex` records the
   index the close must appear at, so the absorbed `}` is that body's own.

   That check had to move ahead of the equality check. Wherever a braced body
   ends a block, the baseline's next token is a `}` as well, so pairing them
   would consume the wrong one and leave the counts right for the wrong reason.

Both refusals are tested over snippets:

  function f() { return value2; }  vs  return value;
    -> token 6: expected name value2, generated name value
  let value = 1; use(value);       vs  { let value = 1; } use(value);
    -> token 0: expected name let, generated {

and the braceable heads are tested one by one, `if`, `for`, `while`,
`if`/`else` and `do`, so the new rule is shown to accept every shape the `curly`
rule produces and not only the one the document happens to exercise.

Controls: restoring the shape rule fails the first refusal case and nothing
else; restoring the accept-any-brace rule fails the second and nothing else.

The eight counts did not move: 609, 73, 373, 279, 36, 17, 4, 7.

Splitting out `terminal-document-tokens.test-support.ts` is not cosmetic. The
tightened rules put the file over the 300-line cap, and a `max-lines` disable is
forbidden, so the token reader moved to its own module: that side answers what a
script says, and says nothing about which differences between two of them are
allowed.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
2026-09-20 12:06:30 -04:00
parent 35ad788f17
commit 0ce0fc99a2
3 changed files with 246 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mobile/src/terminal/document/terminal-document-equivalence.test-support.ts
+113 -113
View File
@@ -1,5 +1,8 @@
import { tokenizer } from 'acorn'
import { transformSync } from 'esbuild'
import {
describeToken,
readScriptTokens,
type DocumentToken
} from './terminal-document-tokens.test-support'
/**
* Whether two versions of the in-WebView document script are the same program, allowing only the
@@ -62,14 +65,28 @@ export type TerminalDocumentNormalisations = {
}
/**
* Whether `printed` is the printer's disambiguated form of `original`: the same name with a decimal
* suffix it appends when two bindings of that name are visible at once.
* The bindings the printer renamed on the baseline and leaves alone in the modules, listed.
*
* A parameter named for a document variable shadowed it while both lived in one function scope, so
* the printer gave the inner one a decimal suffix; once the outer name is a scope field there is no
* shadow and the inner one keeps its own name. Listed rather than matched by shape: a rule that
* accepted any `name2` facing `name` would also accept an unrelated rename that happens to end in a
* digit, which is a changed program, not a normalisation.
*
* One entry covers all seven sites the whole script has: the `term` parameter of
* `attachTerminalQueryReplyBridge` in `query-reply.ts` and its six uses.
*/
function isPrinterDisambiguation(printed: string, original: string): boolean {
if (!printed.startsWith(original) || printed.length === original.length) {
return false
}
return /^[2-9][0-9]*$/.test(printed.slice(original.length))
const UNSHADOWED_RENAMES: readonly {
readonly baseline: string
readonly generated: string
readonly module: string
}[] = [{ baseline: 'term2', generated: 'term', module: 'query-reply' }]
/** Whether this exact baseline-to-generated pair is one of the listed unshadowed renames. */
function isListedUnshadowedRename(baseline: string, generated: string): boolean {
return UNSHADOWED_RENAMES.some(
(entry) => entry.baseline === baseline && entry.generated === generated
)
}
/**
@@ -86,98 +103,70 @@ export type TerminalDocumentEquivalence =
| { readonly equivalent: true; readonly normalisations: TerminalDocumentNormalisations }
| { readonly equivalent: false; readonly reason: string }
/** One token as this comparison reads it: what kind it is, and the text it carried. */
type DocumentToken = { readonly label: string; readonly text: string }
/**
* Acorn's `Token` class declares `type`, `start` and `end` and not `value`, which it does carry,
* so the field is read through a narrowing check rather than asserted onto the declared type.
*/
function readDocumentToken(token: unknown): DocumentToken | null {
if (typeof token !== 'object' || token === null || !('type' in token) || !('value' in token)) {
return null
}
const type: unknown = token.type
if (typeof type !== 'object' || type === null || !('label' in type)) {
return null
}
const label: unknown = type.label
if (typeof label !== 'string') {
return null
}
const value: unknown = token.value
return { label, text: value === undefined || value === null ? '' : String(value) }
}
/** The directive prepended to both sides, and checked to have survived printing. */
const STRICT_DIRECTIVE = 'use strict'
/**
* Both sides are printed by the generator's own printer before being read.
*
* Otherwise every choice the printer makes — semicolons, property shorthand, quote style — reads as
* a difference in the program, when it is a difference in who typed it. Printing both sides with
* one printer removes that whole class by construction rather than by a rule per symptom, and
* leaves only what the eight counted classes cover.
*/
function significantTokens(source: string): DocumentToken[] {
// Read strict on both sides. A loose script has to defend Annex B's block-scoped function
// declarations, and the printer does that by hoisting a `var` and renaming the function; a module
// does not, so one side would carry a rename the other cannot. Neither name escapes its block, so
// the two readings agree on behaviour and only the strict one can be compared.
const printed = transformSync(`'${STRICT_DIRECTIVE}';\n${source}`, {
loader: 'js',
target: 'chrome74',
minify: false
}).code
const kept: DocumentToken[] = []
for (const raw of tokenizer(printed, { ecmaVersion: 2020 })) {
const token = readDocumentToken(raw)
if (token === null) {
throw new Error('acorn produced a token this comparison cannot read')
}
if (token.label === ';' || token.label === 'eof') {
continue
}
kept.push(token)
}
if (kept[0]?.text !== STRICT_DIRECTIVE) {
throw new Error('the strict directive this comparison prepends did not survive printing')
}
return kept.slice(1)
}
/**
* The tokens of one side, or the reason it could not be read.
*
* A script that does not parse is a refusal with the printer's own message rather than an
* exception out of the comparison: a generator that emitted something broken should say so where
* the other differences are reported.
*/
function readScriptTokens(
source: string,
side: string
): { ok: true; tokens: DocumentToken[] } | { ok: false; reason: string } {
try {
return { ok: true, tokens: significantTokens(source) }
} catch (error) {
return {
ok: false,
reason: `${side} does not parse: ${error instanceof Error ? error.message.split('\n')[0] : String(error)}`
}
}
}
function describeToken(token: DocumentToken | undefined): string {
return token === undefined ? '(end of script)' : `${token.label} ${token.text}`.trim()
}
/**
* `baseline` is the script as it stood before the move, `candidate` the one the modules generate.
*
* The qualifier is read from `qualifier`, not assumed, so the test names the object it expects and
* a rename cannot quietly satisfy this.
*/
/** The statement heads `curly` braces: everything whose body may be a single unbraced statement. */
const BRACEABLE_HEAD_KEYWORDS = new Set(['if', 'for', 'while'])
/**
* Whether the `{` at `open` is the body of a braceable head rather than some other block.
*
* `else` and `do` are followed by their body directly. The rest put a parenthesised head first, so
* the `)` is walked back to its `(` and the keyword before that is what decides. Without this a
* bare block anywhere in the generated script would be absorbed as a linter-added body, when it is
* a statement the baseline does not have.
*/
function isBraceableHeadBody(tokens: readonly DocumentToken[], open: number): boolean {
const previous = tokens[open - 1]
if (previous === undefined) {
return false
}
if (previous.label === 'else' || previous.label === 'do') {
return true
}
if (previous.label !== ')') {
return false
}
let depth = 0
for (let i = open - 1; i >= 0; i--) {
const label = tokens[i]?.label
if (label === ')') {
depth += 1
continue
}
if (label === '(') {
depth -= 1
if (depth === 0) {
return BRACEABLE_HEAD_KEYWORDS.has(tokens[i - 1]?.label ?? '')
}
}
}
return false
}
/** The index of the `}` closing the `{` at `open`, or -1 when the generated script has none. */
function matchingCloseIndex(tokens: readonly DocumentToken[], open: number): number {
let depth = 0
for (let i = open; i < tokens.length; i++) {
const label = tokens[i]?.label
if (label === '{') {
depth += 1
continue
}
if (label === '}') {
depth -= 1
if (depth === 0) {
return i
}
}
}
return -1
}
export function compareTerminalDocumentScripts(
baseline: string,
candidate: string,
@@ -201,15 +190,23 @@ export function compareTerminalDocumentScripts(
let numberProperties = 0
let shorthandProperties = 0
let unshadowedNames = 0
// Braces arrive in pairs around one statement, so a counter is enough: a close is only ever
// absorbed while an inserted open is outstanding, which bounds how far this can mask a real one.
let openInsertedBraces = 0
// The generated index each inserted `{` expects its `}` at, innermost last. Recording the index
// rather than counting means an absorbed close is the one that closes that body and no other.
const insertedBraceCloses: number[] = []
let lastMatched: DocumentToken | undefined
let left = 0
let right = 0
while (left < before.length && right < after.length) {
const expected = before[left]
const actual = after[right]
// Ahead of the equality check on purpose: the baseline's next token is a `}` too wherever a
// braced body ends a block, and this index is known to close the inserted body, so matching
// them as a pair would consume the wrong one and leave the counts right for the wrong reason.
if (actual.label === '}' && insertedBraceCloses.at(-1) === right) {
insertedBraceCloses.pop()
right += 1
continue
}
if (expected.label === actual.label && expected.text === actual.text) {
lastMatched = expected
left += 1
@@ -221,7 +218,7 @@ export function compareTerminalDocumentScripts(
if (
expected.label === 'name' &&
actual.label === 'name' &&
isPrinterDisambiguation(expected.text, actual.text)
isListedUnshadowedRename(expected.text, actual.text)
) {
unshadowedNames += 1
lastMatched = actual
@@ -299,16 +296,16 @@ export function compareTerminalDocumentScripts(
left += 3
continue
}
if (actual.label === '{') {
bracedBodies += 1
openInsertedBraces += 1
right += 1
continue
}
if (actual.label === '}' && openInsertedBraces > 0) {
openInsertedBraces -= 1
right += 1
continue
// `if (a) b;` -> `if (a) { b; }`: the body the repository's `curly` rule braced. Only a
// braceable head's body qualifies, and only that body's own close is absorbed.
if (actual.label === '{' && isBraceableHeadBody(after, right)) {
const close = matchingCloseIndex(after, right)
if (close !== -1) {
bracedBodies += 1
insertedBraceCloses.push(close)
right += 1
continue
}
}
return {
equivalent: false,
@@ -316,8 +313,8 @@ export function compareTerminalDocumentScripts(
}
}
// A body braced at the very end of the script leaves its close after the baseline has run out.
while (openInsertedBraces > 0 && after[right]?.label === '}') {
openInsertedBraces -= 1
while (insertedBraceCloses.at(-1) === right && after[right]?.label === '}') {
insertedBraceCloses.pop()
right += 1
}
if (left !== before.length || right !== after.length) {
@@ -326,8 +323,11 @@ export function compareTerminalDocumentScripts(
reason: `length: ${before.length - left} token(s) left in the baseline, ${after.length - right} in the generated script`
}
}
if (openInsertedBraces !== 0) {
return { equivalent: false, reason: `${openInsertedBraces} inserted brace(s) never closed` }
if (insertedBraceCloses.length !== 0) {
return {
equivalent: false,
reason: `${insertedBraceCloses.length} inserted brace(s) never closed`
}
}
return {
equivalent: true,
mobile/src/terminal/document/terminal-document-equivalence.test.ts
+39
View File
@@ -144,6 +144,45 @@ describe('terminal document script equivalence', () => {
).toEqual({ ...NONE, scopeFieldDeclarations: 1, unshadowedNames: 2 })
})
it('refuses a numeric-suffix rename that is not a listed unshadowed binding', () => {
// The shape `value2` -> `value` is what the printer does to a shadow, but this pair is not one
// of the document's, so it is a renamed local: a changed program, not a normalisation.
expect(
normalisationsOf('function f() { return value2; }', 'function f() { return value; }')
).toBe('token 6: expected name value2, generated name value')
})
it('refuses a bare block the baseline does not have', () => {
// A block that is nobody's body cannot be the `curly` rule's work, so absorbing it would hide
// a statement boundary the baseline never had.
expect(normalisationsOf('let value = 1; use(value);', '{ let value = 1; } use(value);')).toBe(
'token 0: expected name let, generated {'
)
})
it('counts a braced body only for a head that can carry an unbraced one', () => {
expect(normalisationsOf('if (a) b();', 'if (a) { b(); }')).toEqual({
...NONE,
bracedBodies: 1
})
expect(normalisationsOf('for (;;) b();', 'for (;;) { b(); }')).toEqual({
...NONE,
bracedBodies: 1
})
expect(normalisationsOf('while (a) b();', 'while (a) { b(); }')).toEqual({
...NONE,
bracedBodies: 1
})
expect(normalisationsOf('if (a) b(); else c();', 'if (a) { b(); } else { c(); }')).toEqual({
...NONE,
bracedBodies: 2
})
expect(normalisationsOf('do b(); while (a);', 'do { b(); } while (a);')).toEqual({
...NONE,
bracedBodies: 1
})
})
it('refuses a changed literal', () => {
expect(normalisationsOf('var a = 1;', 'var a = 2')).toBe(
'token 3: expected num 1, generated num 2'
mobile/src/terminal/document/terminal-document-tokens.test-support.ts
+94
View File
@@ -0,0 +1,94 @@
import { tokenizer } from 'acorn'
import { transformSync } from 'esbuild'
/**
* Reading a version of the in-WebView document script as a token stream.
*
* Kept apart from the comparison that consumes it: this side answers what the script says, and
* says nothing about which differences between two of them are allowed.
*/
/** One token as this comparison reads it: what kind it is, and the text it carried. */
export type DocumentToken = { readonly label: string; readonly text: string }
/**
* Acorn's `Token` class declares `type`, `start` and `end` and not `value`, which it does carry,
* so the field is read through a narrowing check rather than asserted onto the declared type.
*/
function readDocumentToken(token: unknown): DocumentToken | null {
if (typeof token !== 'object' || token === null || !('type' in token) || !('value' in token)) {
return null
}
const type: unknown = token.type
if (typeof type !== 'object' || type === null || !('label' in type)) {
return null
}
const label: unknown = type.label
if (typeof label !== 'string') {
return null
}
const value: unknown = token.value
return { label, text: value === undefined || value === null ? '' : String(value) }
}
/** The directive prepended to both sides, and checked to have survived printing. */
const STRICT_DIRECTIVE = 'use strict'
/**
* Both sides are printed by the generator's own printer before being read.
*
* Otherwise every choice the printer makes — semicolons, property shorthand, quote style — reads as
* a difference in the program, when it is a difference in who typed it. Printing both sides with
* one printer removes that whole class by construction rather than by a rule per symptom, and
* leaves only what the eight counted classes cover.
*/
function significantTokens(source: string): DocumentToken[] {
// Read strict on both sides. A loose script has to defend Annex B's block-scoped function
// declarations, and the printer does that by hoisting a `var` and renaming the function; a module
// does not, so one side would carry a rename the other cannot. Neither name escapes its block, so
// the two readings agree on behaviour and only the strict one can be compared.
const printed = transformSync(`'${STRICT_DIRECTIVE}';\n${source}`, {
loader: 'js',
target: 'chrome74',
minify: false
}).code
const kept: DocumentToken[] = []
for (const raw of tokenizer(printed, { ecmaVersion: 2020 })) {
const token = readDocumentToken(raw)
if (token === null) {
throw new Error('acorn produced a token this comparison cannot read')
}
if (token.label === ';' || token.label === 'eof') {
continue
}
kept.push(token)
}
if (kept[0]?.text !== STRICT_DIRECTIVE) {
throw new Error('the strict directive this comparison prepends did not survive printing')
}
return kept.slice(1)
}
/**
* The tokens of one side, or the reason it could not be read.
*
* A script that does not parse is a refusal with the printer's own message rather than an
* exception out of the comparison: a generator that emitted something broken should say so where
* the other differences are reported.
*/
export function readScriptTokens(
source: string,
side: string
): { ok: true; tokens: DocumentToken[] } | { ok: false; reason: string } {
try {
return { ok: true, tokens: significantTokens(source) }
} catch (error) {
return {
ok: false,
reason: `${side} does not parse: ${error instanceof Error ? error.message.split('\n')[0] : String(error)}`
}
}
}
export function describeToken(token: DocumentToken | undefined): string {
return token === undefined ? '(end of script)' : `${token.label} ${token.text}`.trim()
}