From 3991a49094d3fdc5a058d7383efe26dea5483b58 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 22 Oct 2026 20:40:56 -0500 Subject: [PATCH] Hold any later value in JavaScript var bindings A JavaScript `var` took the checker's type of its initializer, which no later write is checked against in a JavaScript file. js-yaml 4.1.1 failed on every timestamp with "a 'string' value is not representable in the target union": its `var fraction = 0` holds the fraction's digits as a string while it pads them (`fraction = match[7].slice(0, 3)`). Every YAML error also threw the "assignment to non-variables" fence instead of the YAMLException, because its error mark is a `var` object literal that gains a member (`mark.snippet = snippet(mark)`). - A `var` whose writes the declared type cannot hold is checked-dynamic, by the rules `let` already uses (jsBindingHasOpenWrites), and so is a `var` whose object literal gains members or carries methods (jsDeclarationExpandsObject, shared with `let` and `const`). - `+=` of a string onto a number or boolean binding (`width += "px"`) counts as such a write, for `var` and `let`. - A JavaScript function that returns such a binding returns a checked-dynamic value (an async one a promise of one) instead of the initializer's type. A string binding that is only appended to stays a string. --- .../src/frontend/lowering/function-returns.ts | 44 ++++++ .../src/frontend/lowering/lower-stmts.ts | 133 +++++++++++++----- .../7037-js-var-open-writes/main.ts.json | 6 + .../corpus/7038-js-var-later-writes.mjs.json | 6 + tests/corpus/7037-js-var-open-writes/main.ts | 18 +++ .../node_modules/scalar-marks/index.d.ts | 4 + .../node_modules/scalar-marks/index.js | 81 +++++++++++ .../node_modules/scalar-marks/package.json | 1 + tests/corpus/7038-js-var-later-writes.mjs | 59 ++++++++ .../7037-js-var-open-writes/main.ts.json | 1 + .../7038-js-var-later-writes.mjs.json | 1 + 11 files changed, 321 insertions(+), 33 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7037-js-var-open-writes/main.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7038-js-var-later-writes.mjs.json create mode 100644 tests/corpus/7037-js-var-open-writes/main.ts create mode 100644 tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.d.ts create mode 100644 tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.js create mode 100644 tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/package.json create mode 100644 tests/corpus/7038-js-var-later-writes.mjs create mode 100644 tests/harness/corpus-costs/7037-js-var-open-writes/main.ts.json create mode 100644 tests/harness/corpus-costs/7038-js-var-later-writes.mjs.json diff --git a/packages/compiler/src/frontend/lowering/function-returns.ts b/packages/compiler/src/frontend/lowering/function-returns.ts index 5ede3efe3..dde109ea7 100644 --- a/packages/compiler/src/frontend/lowering/function-returns.ts +++ b/packages/compiler/src/frontend/lowering/function-returns.ts @@ -15,6 +15,7 @@ import { import { isJsSourceFile } from "../program.js"; import { PoisonError, dynFallbackType } from "./lowerer.js"; import { functionCanReturnUndefined } from "../function-completion.js"; +import { jsBindingAppendsOnly, jsBindingHasOpenWrites } from "./lower-stmts.js"; import type { FnCtx } from "./function-context.js"; /** Keep the conservative completion rule shared by return construction @@ -124,6 +125,32 @@ export function producesConstructor(lowerer: Lowerer, type: ts.Type): boolean { return false; } +/** Whether a return of `decl` itself (not of a nested function) is a bare + * read of a JavaScript variable whose writes the checker does not + * constrain. A string that is only appended to stays a string. */ +function returnsOpenJsBinding(lowerer: Lowerer, decl: ts.SignatureDeclaration): boolean { + let open = false; + const opens = (value: ts.Expression | undefined): boolean => { + while (value && ts.isParenthesizedExpression(value)) value = value.expression; + if (!value || !ts.isIdentifier(value)) return false; + const binding = lowerer.resolveValueSymbol(value)?.valueDeclaration; + return ( + binding !== undefined && + ts.isVariableDeclaration(binding) && + jsBindingHasOpenWrites(lowerer, binding) && + !jsBindingAppendsOnly(lowerer, binding) + ); + }; + const scan = (node: ts.Node): void => { + if (open || (node !== decl && ts.isFunctionLike(node))) return; + if (ts.isReturnStatement(node) && opens(node.expression)) open = true; + ts.forEachChild(node, scan); + }; + if (ts.isArrowFunction(decl) && decl.body && !ts.isBlock(decl.body)) open = opens(decl.body); + else scan(decl); + return open; +} + /** Map a declaration's return contract while retaining JavaScript * inference fallbacks. Diagnostics preserve the caller's blame node. */ export function declaredReturnType( @@ -212,6 +239,23 @@ export function declaredReturnType( lowerer.unions.get(mappedReturn.unionId)?.arms.some((arm) => arm.kind === "func") ) return DYN; + // A JS function returning a local that holds whatever its writes store + // (jsBindingHasOpenWrites): the inferred result is the initializer's + // type, which the returned value need not have. + // An async function's promise fulfills with that value. + if ( + isJsSourceFile(ts.getSourceFileOfNode(decl)) && + !decl.type && + !decl.asteriskToken && + !hasExplicitJsDocReturn(decl) && + mappedReturn !== null && + mappedReturn.kind !== "dyn" && + !(mappedReturn.kind === "promise" && mappedReturn.inner.kind === "dyn") && + returnsOpenJsBinding(lowerer, decl) + ) + return decl.modifiers?.nodes.some((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword) + ? { kind: "promise", inner: DYN } + : DYN; // JS inference assigns Number to arithmetic over any, although the // runtime operands can both be BigInts. Preserve that native result. if ( diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index ea5ef72ac..62c2e3a0f 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -1220,6 +1220,43 @@ function lowerJsVarFromExisting( }; } +/** Whether a JavaScript declaration's object value changes shape after its + * initializer (jsOpenObjectType, jsObjectBindingExpands) or carries + * methods, so only the checked-dynamic representation holds it. + * `inferred` is the declaration's mapped checker type. */ +function jsDeclarationExpandsObject( + lowerer: Lowerer, + decl: ts.VariableDeclaration, + inferred: IrType | null, +): boolean { + const arms = + inferred?.kind === "union" + ? (lowerer.unions.get(inferred.unionId)?.arms ?? []) + : inferred + ? [inferred] + : []; + return ( + (!decl.type && + !hasJsTypeAnnotation(decl) && + inferred !== null && + jsOpenObjectType(decl, inferred, lowerer.shapes, lowerer.unions).kind === "dyn") || + arms.some((arm) => arm.kind === "record" && jsObjectBindingExpands(lowerer, decl, arm)) || + (isJsSourceFile(ts.getSourceFileOfNode(decl)) && + !decl.type && + !hasJsTypeAnnotation(decl) && + decl.initializer !== undefined && + ts.isObjectLiteralExpression(decl.initializer) && + decl.initializer.properties.nodes.some( + (property) => + ts.isMethodDeclaration(property) || + ts.isAccessor(property) || + (ts.isPropertyAssignment(property) && + (ts.isFunctionExpression(property.initializer) || + ts.isArrowFunction(property.initializer))), + )) + ); +} + /** The `var` symbol's function-scoped binding TYPE — the declared type at * the binding name, with the JS-source fallbacks every mutable binding * takes (an empty-object-literal type is checked-dynamic because tsc @@ -1254,6 +1291,19 @@ function varBindingType(lowerer: Lowerer, nameNode: ts.Identifier): IrType | nul type = staticImportNamespaceType(lowerer, nameNode.parent.initializer) ?? type; } if (!type || type.kind === "void") return null; + // A JavaScript `var` holds whatever its writes store, as a `let` does: + // the checker types it by its initializer and checks no later write + // (`var fraction = 0;`, later `fraction = match[7].slice(0, 3)`), and an + // object it holds may gain members (`mark.snippet = snippet(mark)`). + if ( + type.kind !== "dyn" && + ts.isVariableDeclaration(nameNode.parent) && + nameNode.parent.name === nameNode && + isJsSourceFile(ts.getSourceFileOfNode(nameNode)) && + (jsBindingHasOpenWrites(lowerer, nameNode.parent) || + jsDeclarationExpandsObject(lowerer, nameNode.parent, type)) + ) + return DYN; // A binding readable before its first store holds undefined until then. return lowerer.runtimeOptionalBindingType(nameNode, type); } @@ -5597,26 +5647,7 @@ export function lowerVarDecl( : inferredObjectType ? [inferredObjectType] : []; - const expandsObject = - (!decl.type && - !hasJsTypeAnnotation(decl) && - inferredObjectType !== null && - jsOpenObjectType(decl, inferredObjectType, lowerer.shapes, lowerer.unions).kind === "dyn") || - inferredObjectArms.some( - (arm) => arm.kind === "record" && jsObjectBindingExpands(lowerer, decl, arm), - ) || - (isJsSourceFile(ts.getSourceFileOfNode(decl)) && - !decl.type && - !hasJsTypeAnnotation(decl) && - ts.isObjectLiteralExpression(decl.initializer) && - decl.initializer.properties.nodes.some( - (property) => - ts.isMethodDeclaration(property) || - ts.isAccessor(property) || - (ts.isPropertyAssignment(property) && - (ts.isFunctionExpression(property.initializer) || - ts.isArrowFunction(property.initializer))), - )); + const expandsObject = jsDeclarationExpandsObject(lowerer, decl, inferredObjectType); let init: IrExpr; try { init = @@ -6327,18 +6358,15 @@ export function hasJsTypeAnnotation(decl: ts.VariableDeclaration): boolean { ); } -/** Inferred JS scalar initializers do not constrain later writes through - * an any-valued callback. Keep those bindings open, including captured writes. */ -export function jsBindingHasOpenWrites(lowerer: Lowerer, decl: ts.VariableDeclaration): boolean { - if ( - !isJsSourceFile(ts.getSourceFileOfNode(decl)) || - decl.type || - hasJsTypeAnnotation(decl) || - !ts.isIdentifier(decl.name) - ) - return false; - const symbol = lowerer.resolveValueSymbol(decl.name); - if (!symbol) return false; +/** The assignments to a declaration's binding in its function (or file), + * `=` and compound alike. Null when the name has no symbol. */ +function jsBindingWrites( + lowerer: Lowerer, + decl: ts.VariableDeclaration, + name: ts.Identifier, +): ts.BinaryExpression[] | null { + const symbol = lowerer.resolveValueSymbol(name); + if (!symbol) return null; let owner: ts.Node = decl; while (owner.parent && !ts.isFunctionLike(owner) && !ts.isSourceFile(owner)) owner = owner.parent; let writes = lowerer.jsBindingWritesByOwner.get(owner); @@ -6359,9 +6387,36 @@ export function jsBindingHasOpenWrites(lowerer: Lowerer, decl: ts.VariableDeclar writes = indexed; lowerer.jsBindingWritesByOwner.set(owner, writes); } - const candidates = (writes.get(decl.name.text) ?? []).filter( + return (writes.get(name.text) ?? []).filter( (node) => lowerer.resolveValueSymbol(node.left as ts.Identifier) === symbol, ); +} + +/** Whether a JavaScript binding always holds a string: the checker types it + * as one, and every write appends to it (`result += part`), which yields a + * string whatever the part is. */ +export function jsBindingAppendsOnly(lowerer: Lowerer, decl: ts.VariableDeclaration): boolean { + if (!ts.isIdentifier(decl.name)) return false; + if (lowerer.mapTypeOf(lowerer.typeOf(decl.name))?.kind !== "string") return false; + const writes = jsBindingWrites(lowerer, decl, decl.name); + return ( + writes !== null && + writes.every((node) => node.operatorToken.kind === ts.SyntaxKind.PlusEqualsToken) + ); +} + +/** Inferred JS scalar initializers do not constrain later writes through + * an any-valued callback. Keep those bindings open, including captured writes. */ +export function jsBindingHasOpenWrites(lowerer: Lowerer, decl: ts.VariableDeclaration): boolean { + if ( + !isJsSourceFile(ts.getSourceFileOfNode(decl)) || + decl.type || + hasJsTypeAnnotation(decl) || + !ts.isIdentifier(decl.name) + ) + return false; + const candidates = jsBindingWrites(lowerer, decl, decl.name); + if (candidates === null) return false; if ( candidates.some( (node) => @@ -6369,6 +6424,18 @@ export function jsBindingHasOpenWrites(lowerer: Lowerer, decl: ts.VariableDeclar ) ) return true; + // A `+=` of a string turns a number or boolean binding into a string + // (`n += "px"`); the checker keeps the declared type. + if ( + candidates.some( + (node) => + node.operatorToken.kind === ts.SyntaxKind.PlusEqualsToken && + (lowerer.typeOf(node.right).flags & ts.TypeFlags.StringLike) !== 0, + ) + ) { + const declared = lowerer.mapTypeOf(lowerer.typeOf(decl.name)); + if (declared?.kind === "f64" || declared?.kind === "bool") return true; + } // A write that can store null or undefined into a binding the checker // types as a primitive (`offset = match[3] || null`): JavaScript files // are checked without strict null checks, so the declared type does not diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7037-js-var-open-writes/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7037-js-var-open-writes/main.ts.json new file mode 100644 index 000000000..a1bdb91b8 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7037-js-var-open-writes/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7037-js-var-open-writes/main.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7038-js-var-later-writes.mjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7038-js-var-later-writes.mjs.json new file mode 100644 index 000000000..1e51a088f --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7038-js-var-later-writes.mjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7038-js-var-later-writes.mjs" + ], + "diags": [] +} diff --git a/tests/corpus/7037-js-var-open-writes/main.ts b/tests/corpus/7037-js-var-open-writes/main.ts new file mode 100644 index 000000000..cc0612974 --- /dev/null +++ b/tests/corpus/7037-js-var-open-writes/main.ts @@ -0,0 +1,18 @@ +// A JavaScript package whose `var` bindings start as one type and later +// hold another, as js-yaml's timestamp and error code does: a number that +// becomes a string and back (`fraction = match[7].slice(0, 3)`), an object +// literal that gains a member (`mark.snippet = snippet(mark)`), and `+=` of +// a string onto a number. Node runs every write; each binding holds it. +import { errorAt, millis, label, total } from "scalar-marks"; + +console.log(millis("2001-12-14t21:59:43.10"), millis("2001-12-14t21:59:43"), millis("1970-01-01t00:00:00.12345")); +for (const [input, at] of [["a: [1", 5], ["key: 'open", 10], ["a:\n [1", 7], ["ok", 1]] as const) { + try { + console.log(errorAt(input, at, "unexpected end")); + } catch (e) { + const err = e as { name: string; message: string; mark: { line: number; column: number; snippet: string } }; + console.log(err.name, "|", err.message, "|", err.mark.line, err.mark.column, JSON.stringify(err.mark.snippet)); + } +} +console.log(label(3), label(0)); +console.log(total([1, 2, 3]), total(["a", "b"]), total([1, "b", 2])); diff --git a/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.d.ts b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.d.ts new file mode 100644 index 000000000..0e0f8252c --- /dev/null +++ b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.d.ts @@ -0,0 +1,4 @@ +export declare function millis(data: string): number; +export declare function errorAt(input: string, position: number, message: string): string; +export declare function label(n: number): string; +export declare function total(values: (number | string)[]): number | string; diff --git a/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.js b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.js new file mode 100644 index 000000000..7d6b6116c --- /dev/null +++ b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/index.js @@ -0,0 +1,81 @@ +"use strict"; +var TIMESTAMP = new RegExp( + '^([0-9][0-9][0-9][0-9])-([0-9][0-9]?)-([0-9][0-9]?)' + + '[Tt]([0-9][0-9]?):([0-9][0-9]):([0-9][0-9])(?:\\.([0-9]*))?$'); + +// The milliseconds of a timestamp: the fraction starts as the number 0, +// holds its digits as a string while it is padded, and ends as a number. +function millis(data) { + var match, fraction = 0, delta = null; + match = TIMESTAMP.exec(data); + if (match === null) throw new Error('Date resolve error'); + if (match[7]) { + fraction = match[7].slice(0, 3); + while (fraction.length < 3) { + fraction += '0'; + } + fraction = +fraction; + } + if (delta) fraction -= delta; + return Date.UTC(+match[1], +match[2] - 1, +match[3], +match[4], +match[5], +match[6], fraction); +} + +function State(input) { + this.input = input; + this.position = 0; + this.line = 0; + this.lineStart = 0; +} + +function snippet(mark) { + var start = mark.position, text = mark.buffer; + return text.slice(0, start) + '<' + text.slice(start) + '>'; +} + +function MarkError(reason, mark) { + Error.call(this); + this.name = 'MarkError'; + this.reason = reason; + this.mark = mark; + this.message = reason + ' (' + (mark.line + 1) + ':' + (mark.column + 1) + ')'; +} +MarkError.prototype = Object.create(Error.prototype); +MarkError.prototype.constructor = MarkError; + +// The error's mark is an object literal that gains its snippet afterwards. +function generateError(state, message) { + var mark = { + buffer: state.input, + position: state.position, + line: state.line, + column: state.position - state.lineStart + }; + mark.snippet = snippet(mark); + return new MarkError(message, mark); +} + +function errorAt(input, position, message) { + var state = new State(input); + for (var i = 0; i < position && i < input.length; i++) { + state.position++; + if (input[i] === '\n') { state.line++; state.lineStart = state.position; } + } + if (state.position >= input.length) throw generateError(state, message); + return 'no error at ' + state.position; +} + +// A number binding that `+=` turns into a string. +function label(n) { + var width = 10; + width += 'px'; + return n + ' ' + width; +} + +// A sum that becomes a string concatenation once a string arrives. +function total(values) { + var sum = 0; + for (var i = 0; i < values.length; i++) sum += values[i]; + return sum; +} + +module.exports = { millis: millis, errorAt: errorAt, label: label, total: total }; diff --git a/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/package.json b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/package.json new file mode 100644 index 000000000..ade0a9891 --- /dev/null +++ b/tests/corpus/7037-js-var-open-writes/node_modules/scalar-marks/package.json @@ -0,0 +1 @@ +{ "name": "scalar-marks", "version": "1.0.0", "main": "./index.js", "types": "./index.d.ts" } diff --git a/tests/corpus/7038-js-var-later-writes.mjs b/tests/corpus/7038-js-var-later-writes.mjs new file mode 100644 index 000000000..48b85d77a --- /dev/null +++ b/tests/corpus/7038-js-var-later-writes.mjs @@ -0,0 +1,59 @@ +// JavaScript `var` and `let` bindings that later hold another type than +// their initializer's, which nothing checks in a JavaScript file: Node +// runs each write, and a function returning the binding returns what it +// holds. +function retyped(input) { + var value = 0; + value = input.slice(0, 2); + return value; +} + +function padded(digits) { + var fraction = 0; + if (digits) { + fraction = digits.slice(0, 3); + while (fraction.length < 3) fraction += "0"; + fraction = +fraction; + } + return fraction + 1; +} + +function suffixed(n) { + var width = n * 2; + width += "px"; + return width; +} + +function letRetyped(record) { + let current = 1; + current = record.name; + return current; +} + +function grown(line) { + var mark = { line: line, column: line - 1 }; + mark.text = "at " + (mark.line + 1); + return mark; +} + +async function awaitedTotal(values) { + var total = 0; + for (var i = 0; i < values.length; i++) total += await values[i]; + console.log("awaited", total, typeof total); + return total; +} + +function counted(values) { + var count = 0; + for (var i = 0; i < values.length; i++) count += values[i]; + return count; +} + +console.log(retyped("abc"), typeof retyped("abc")); +console.log(padded("5"), padded("12345"), padded("")); +console.log(suffixed(4), typeof suffixed(4)); +console.log(letRetyped({ name: "x" }), letRetyped({ name: 2 })); +console.log(JSON.stringify(grown(3)), Object.keys(grown(0)).join(",")); +console.log(counted([1, 2]), counted([1, "a", 2])); +await awaitedTotal([1, Promise.resolve(2)]); +await awaitedTotal([1, Promise.resolve("b")]); diff --git a/tests/harness/corpus-costs/7037-js-var-open-writes/main.ts.json b/tests/harness/corpus-costs/7037-js-var-open-writes/main.ts.json new file mode 100644 index 000000000..2e747487d --- /dev/null +++ b/tests/harness/corpus-costs/7037-js-var-open-writes/main.ts.json @@ -0,0 +1 @@ +{ "plain": 1970, "san": 2150 } diff --git a/tests/harness/corpus-costs/7038-js-var-later-writes.mjs.json b/tests/harness/corpus-costs/7038-js-var-later-writes.mjs.json new file mode 100644 index 000000000..29130736b --- /dev/null +++ b/tests/harness/corpus-costs/7038-js-var-later-writes.mjs.json @@ -0,0 +1 @@ +{ "plain": 1570, "san": 2390 }