Skip to content

fix: parenthesize non-directive string literals in a directive prologue#740

Open
spokodev wants to merge 1 commit into
davidbonnet:mainfrom
spokodev:fix/directive-prologue-parens
Open

fix: parenthesize non-directive string literals in a directive prologue#740
spokodev wants to merge 1 commit into
davidbonnet:mainfrom
spokodev:fix/directive-prologue-parens

Conversation

@spokodev

Copy link
Copy Markdown

Bug

A string-literal expression statement at the start of a program or function body that is not a directive — parenthesized in the source, or built by a transform that didn't set directive — is generated without parentheses. Re-parsing then promotes it to a directive, which silently changes semantics (e.g. enabling strict mode) and can produce code that no longer parses:

import { generate } from 'astring'
import { parse } from 'acorn'

generate(parse('("use strict"); with ({}) {}'))
// => '"use strict"; with ({}) {}'
//    `with` is now inside a (newly-introduced) strict-mode prologue → SyntaxError on reparse

The same happens inside function bodies, and for things like ("use strict"); var x = 010; (octal becomes a strict-mode error). generate(parse(x)) is no longer idempotent for these inputs.

Cause

ExpressionStatement only parenthesizes for object/function ambiguity and assignment-to-pattern. It doesn't guard a string literal that sits at a directive-prologue position, so a non-directive string is emitted bare and is reparsed as a directive.

Fix

Track the directive prologue while writing a Program / BlockStatement body (it stays open across leading string-literal expression statements), and in ExpressionStatement parenthesize a string literal there only when it is not a directive — ESTree marks directives via node.directive, which acorn sets.

  • Real directives (node.directive set) are emitted bare, exactly as before.
  • Mid-body string statements are unaffected (the prologue is already closed), so existing output like debugger;\n"…"; is unchanged.
  • Parenthesizing a string literal is always semantically safe, so the change can only ever add parentheses in the prologue case it targets.

Tests

Added src/tests/fixtures/syntax/directive.js (covered by the existing Syntax check exact round-trip test). It exercises a real directive (stays bare), a parenthesized prologue non-directive (kept parenthesized), a mid-body string (stays bare), and the same inside function bodies. On main this fixture fails (the parens are dropped); with this change it round-trips.

Note: I couldn't run npm test locally — [email protected] throws an uncaught exception at load under current Node (unrelated to this change; it fails the same way on a clean checkout). I verified the new fixture round-trips on the build, confirmed it fails on the published build (red→green), and diffed published-vs-patched output across every fixture plus ~43k random valid expressions: the only change is the intended prologue parenthesization, zero other differences.

A string-literal expression statement at the start of a program or
function body that is NOT a directive (parenthesized in the source, or
produced by a transform that didn't set `directive`) was generated
without parentheses, so re-parsing promoted it to a directive. This
silently changes semantics (e.g. enabling strict mode) and can even
produce code that no longer parses:

  generate(parse('("use strict"); with ({}) {}'))
  // => '"use strict"; with ({}) {}'   -> SyntaxError on reparse

Track the directive prologue while writing a Program/BlockStatement body
and parenthesize a string literal there only when it is not a directive
(ESTree marks directives via `node.directive`). Real directives and
mid-body string statements are unaffected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant