Check untyped destructuring sources in JavaScript files before reading keys (#2577)

packages/compiler/src/frontend/lowering/lower-stmts.ts

@@ -2646,16 +2646,18 @@ export function lowerDestructuringDecl(
             type: DYN,
             loc,
           }
-        : spelling !== null
-          ? {
-              kind: "dynDestrCheck",
-              value: unitValue(),
-              spelling,
-              ...firstPatternProp(lowerer, decl.name as ts.ObjectBindingPattern),
-              type: DYN,
-              loc,
-            }
-          : null;
+        : firstReadsWithDefault(decl.name as ts.ObjectBindingPattern)
+          ? firstKeyRead(lowerer, decl.name as ts.ObjectBindingPattern, unitValue, loc)
+          : spelling !== null
+            ? {
+                kind: "dynDestrCheck",
+                value: unitValue(),
+                spelling,
+                ...firstPatternProp(lowerer, decl.name as ts.ObjectBindingPattern),
+                type: DYN,
+                loc,
+              }
+            : null;
       out.push({
         kind: "if",
         cond: {
@@ -2696,22 +2698,25 @@ export function lowerDestructuringDecl(
   // value: RequireObjectCoercible with V8's own TypeError ("Cannot
   // destructure property 'a' of 'f(...)' as it is null.") for the source
   // spellings the assignment form reproduces; other sources are refused,
-  // as in the assignment form. JavaScript files keep their existing
-  // per-read errors, except that a runtime computed first key checks the
-  // source first: Node throws before it evaluates any key, and V8's bare
-  // message names the source.
-  const jsRuntimeKeyCheck =
+  // as in the assignment form. A JavaScript object pattern over a dyn
+  // source checks the source first in the same way when V8 can spell it,
+  // so a null or undefined source throws Node's message before any key
+  // is read or evaluated; other sources keep their per-read errors. V8
+  // skips the check when the first element has a default and a static
+  // key: the read of that key throws ("Cannot read properties of null
+  // (reading 'a')"), and so does the lowering's.
+  const jsSourceCheck =
     init.type.kind === "dyn" &&
     ts.isObjectBindingPattern(decl.name) &&
     isJsSourceFile(ts.getSourceFileOfNode(decl)) &&
-    (decl.name.elements.nodes.length === 0 || startsWithRuntimeKey(lowerer, decl.name)) &&
     destrSpellingOf(decl.initializer) !== null;
   if (
-    jsRuntimeKeyCheck ||
-    (init.type.kind === "dyn" &&
-      ts.isObjectBindingPattern(decl.name) &&
-      !isJsSourceFile(ts.getSourceFileOfNode(decl)) &&
-      lowerer.anyOrigin(decl.initializer))
+    (jsSourceCheck ||
+      (init.type.kind === "dyn" &&
+        ts.isObjectBindingPattern(decl.name) &&
+        !isJsSourceFile(ts.getSourceFileOfNode(decl)) &&
+        lowerer.anyOrigin(decl.initializer))) &&
+    !firstReadsWithDefault(decl.name as ts.ObjectBindingPattern)
   ) {
     const spelling = destrSpellingOf(decl.initializer);
     if (spelling === null) {
@@ -9694,6 +9699,39 @@ export function lowerDestructuringAssign(
   return { kind: "block", body: parts.stmts, loc };
 }
 
+/** An object pattern whose first element has a default and a static key
+ * (`{ a = 1 }`, `{ a: b = 1 }`): V8 reads that key without checking the
+ * source first, so a null or undefined source throws the read's TypeError. */
+function firstReadsWithDefault(pattern: ts.ObjectBindingPattern): boolean {
+  const first = pattern.elements.nodes[0];
+  return (
+    first !== undefined &&
+    first.dotDotDotToken === undefined &&
+    first.initializer !== undefined &&
+    !(first.propertyName !== undefined && ts.isComputedPropertyName(first.propertyName))
+  );
+}
+
+/** The read of the first key of a pattern that `firstReadsWithDefault`
+ * accepts, over a dyn source: on null or undefined it throws the read's
+ * TypeError, as V8 does. Null when the key has no static name. */
+function firstKeyRead(
+  lowerer: Lowerer,
+  pattern: ts.ObjectBindingPattern,
+  source: () => IrExpr,
+  loc: SrcLoc,
+): IrExpr | null {
+  const { firstProp } = firstPatternProp(lowerer, pattern);
+  if (firstProp === undefined) return null;
+  return {
+    kind: "dynKeyGet",
+    key: { kind: "strLit", value: firstProp, type: STRING, loc },
+    value: source(),
+    type: DYN,
+    loc,
+  };
+}
+
 /** The first property V8 names in an object pattern's destructuring
  * TypeError ("Cannot destructure property 'a' of ..."): an identifier,
  * string or numeric first key. A computed first key, a rest element or

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/destructure-untyped-source-null.js.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/destructure-untyped-source-null.js"
+ ],
+ "diags": []
+}

tests/corpus/destructure-nullable-union-source.js

@@ -48,6 +48,16 @@ show("object-undefined", () => {
   const { name } = items.find((item) => item.size > 5);
   console.log("never", name);
 });
+// A default on the first element: V8 reads that key without checking
+// the source first, so the read's TypeError is the one thrown.
+show("object-first-default", () => {
+  const { name = "none", size } = items.find((item) => item.size > 5);
+  console.log("never", name, size);
+});
+show("object-later-default", () => {
+  const { name, size = 0 } = items.find((item) => item.size > 5);
+  console.log("never", name, size);
+});
 function firstMatch(text) {
   return text.match(/q/);
 }

tests/corpus/destructure-untyped-source-null.js

@@ -0,0 +1,67 @@
+// A destructuring declaration over an untyped source (the result of an
+// untyped function, a parameter, a property of an untyped object) checks
+// the source first: a null or undefined source throws Node's TypeError,
+// which names the source and the first property, before any key is read.
+
+function show(label, fn) {
+  try {
+    fn();
+  } catch (error) {
+    console.log(label, error instanceof TypeError, error.message);
+  }
+}
+
+const table = { size: 3, entry: null, nested: { inner: undefined } };
+function lookup(key) {
+  return table[key];
+}
+function read(source) {
+  const { size, name } = source;
+  return [size, name];
+}
+
+show("call-null", () => {
+  const { size } = lookup("entry");
+  console.log("never", size);
+});
+show("call-undefined", () => {
+  const { size, label } = lookup("missing");
+  console.log("never", size, label);
+});
+show("parameter", () => {
+  console.log("never", read(null));
+});
+show("property-chain", () => {
+  const { value } = table.nested.inner;
+  console.log("never", value);
+});
+show("renamed-and-default", () => {
+  const { size: count = 0, rest } = lookup("entry");
+  console.log("never", count, rest);
+});
+show("first-default", () => {
+  const { size = 0 } = lookup("missing");
+  console.log("never", size);
+});
+show("later-default", () => {
+  const { size, name = "none" } = lookup("entry");
+  console.log("never", size, name);
+});
+show("computed-key-default", () => {
+  const key = "size";
+  const { [key]: value = 0 } = lookup("entry");
+  console.log("never", value);
+});
+show("string-key", () => {
+  const { "two words": both } = lookup("missing");
+  console.log("never", both);
+});
+show("rest-first", () => {
+  const { ...all } = lookup("entry");
+  console.log("never", all);
+});
+
+// Sources that hold a value bind as before, including primitives.
+const { size } = lookup("size") === 3 ? table : null;
+const { length } = lookup("missing") ?? "text";
+console.log("bound", size, length, read({ size: 1, name: "one" }).join(","));

tests/harness/corpus-costs/destructure-untyped-source-null.js.json

@@ -0,0 +1 @@
+{ "plain": 3000, "san": 4500 }