refactor(traverse): separate #[scope] attr (#3901)

Separate out attributes which communicate info to codegen related to scopes into `#[scope]` attr.

Before:

```rs
#[visited_node(scope(ScopeFlags::empty()))]
pub struct BlockStatement<'a> { /* ... */ }
```

After:

```rs
#[visited_node]
#[scope]
pub struct BlockStatement<'a> { /* ... */ }
```

I think this is clearer.

crates/oxc_ast/src/ast/js.rs

@@ -1,3 +1,6 @@
+// NB: `#[visited_node]` and `#[scope]` attributes on AST nodes do not do anything to the code in this file.
+// They are purely markers for codegen used in `oxc_traverse`. See docs in that crate.
+
 // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
 #![allow(non_snake_case)]
 
@@ -23,8 +26,9 @@ use serde::Serialize;
 #[cfg(feature = "serialize")]
 use tsify::Tsify;
 
-#[visited_node(
+#[visited_node]
-    scope(ScopeFlags::Top),
+#[scope(
+    flags(ScopeFlags::Top),
     strict_if(self.source_type.is_strict() || self.directives.iter().any(Directive::is_use_strict)),
 )]
 #[derive(Debug)]
@@ -943,7 +947,8 @@ pub struct Hashbang<'a> {
 }
 
 /// Block Statement
-#[visited_node(scope(ScopeFlags::empty()))]
+#[visited_node]
+#[scope]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1099,10 +1104,8 @@ pub struct WhileStatement<'a> {
 }
 
 /// For Statement
-#[visited_node(
+#[visited_node]
-    scope(ScopeFlags::empty()),
+#[scope(if(self.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration)))]
-    scope_if(self.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration)),
-)]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1136,7 +1139,8 @@ pub enum ForStatementInit<'a> {
 }
 
 /// For-In Statement
-#[visited_node(scope(ScopeFlags::empty()), scope_if(self.left.is_lexical_declaration()))]
+#[visited_node]
+#[scope(if(self.left.is_lexical_declaration()))]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1168,7 +1172,8 @@ pub enum ForStatementLeft<'a> {
 }
 }
 /// For-Of Statement
-#[visited_node(scope(ScopeFlags::empty()), scope_if(self.left.is_lexical_declaration()))]
+#[visited_node]
+#[scope(if(self.left.is_lexical_declaration()))]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1228,7 +1233,8 @@ pub struct WithStatement<'a> {
 }
 
 /// Switch Statement
-#[visited_node(scope(ScopeFlags::empty()))]
+#[visited_node]
+#[scope]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1288,7 +1294,8 @@ pub struct TryStatement<'a> {
     pub finalizer: Option<Box<'a, BlockStatement<'a>>>,
 }
 
-#[visited_node(scope(ScopeFlags::empty()), scope_if(self.param.is_some()))]
+#[visited_node]
+#[scope(if(self.param.is_some()))]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -1422,9 +1429,10 @@ pub struct BindingRestElement<'a> {
 }
 
 /// Function Definitions
-#[visited_node(
+#[visited_node]
+#[scope(
     // TODO: `ScopeFlags::Function` is not correct if this is a `MethodDefinition`
-    scope(ScopeFlags::Function),
+    flags(ScopeFlags::Function),
     strict_if(self.body.as_ref().is_some_and(|body| body.has_use_strict_directive())),
 )]
 #[derive(Debug)]
@@ -1530,8 +1538,9 @@ pub struct FunctionBody<'a> {
 }
 
 /// Arrow Function Definitions
-#[visited_node(
+#[visited_node]
-    scope(ScopeFlags::Function | ScopeFlags::Arrow),
+#[scope(
+    flags(ScopeFlags::Function | ScopeFlags::Arrow),
     strict_if(self.body.has_use_strict_directive()),
 )]
 #[derive(Debug)]
@@ -1565,7 +1574,8 @@ pub struct YieldExpression<'a> {
 }
 
 /// Class Definitions
-#[visited_node(scope(ScopeFlags::StrictMode))]
+#[visited_node]
+#[scope(flags(ScopeFlags::StrictMode))]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(rename_all = "camelCase"))]
@@ -1690,7 +1700,8 @@ pub struct PrivateIdentifier<'a> {
     pub name: Atom<'a>,
 }
 
-#[visited_node(scope(ScopeFlags::ClassStaticBlock))]
+#[visited_node]
+#[scope(flags(ScopeFlags::ClassStaticBlock))]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]

crates/oxc_ast/src/ast/jsx.rs

@@ -1,7 +1,7 @@
 //! [JSX](https://facebook.github.io/jsx)
 
-// NB: `#[visited_node]` attribute on AST nodes does not do anything to the code in this file.
+// NB: `#[visited_node]` and `#[scope]` attributes on AST nodes do not do anything to the code in this file.
-// It is purely a marker for codegen used in `oxc_traverse`. See docs in that crate.
+// They are purely markers for codegen used in `oxc_traverse`. See docs in that crate.
 
 // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
 #![allow(non_snake_case)]

crates/oxc_ast/src/ast/literal.rs

@@ -1,7 +1,7 @@
 //! Literals
 
-// NB: `#[visited_node]` attribute on AST nodes does not do anything to the code in this file.
+// NB: `#[visited_node]` and `#[scope]` attributes on AST nodes do not do anything to the code in this file.
-// It is purely a marker for codegen used in `oxc_traverse`. See docs in that crate.
+// They are purely markers for codegen used in `oxc_traverse`. See docs in that crate.
 
 // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
 #![allow(non_snake_case)]

crates/oxc_ast/src/ast/ts.rs

@@ -3,8 +3,8 @@
 //! [AST Spec](https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/ast-spec)
 //! [Archived TypeScript spec](https://github.com/microsoft/TypeScript/blob/3c99d50da5a579d9fa92d02664b1b66d4ff55944/doc/spec-ARCHIVED.md)
 
-// NB: `#[visited_node]` attribute on AST nodes does not do anything to the code in this file.
+// NB: `#[visited_node]` and `#[scope]` attributes on AST nodes do not do anything to the code in this file.
-// It is purely a marker for codegen used in `oxc_traverse`. See docs in that crate.
+// They are purely markers for codegen used in `oxc_traverse`. See docs in that crate.
 
 // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
 #![allow(non_snake_case)]
@@ -46,7 +46,8 @@ pub struct TSThisParameter<'a> {
 /// Enum Declaration
 ///
 /// `const_opt` enum `BindingIdentifier` { `EnumBody_opt` }
-#[visited_node(scope(ScopeFlags::empty()))]
+#[visited_node]
+#[scope]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type"))]
@@ -562,7 +563,8 @@ pub struct TSTypeParameterInstantiation<'a> {
     pub params: Vec<'a, TSType<'a>>,
 }
 
-#[visited_node(scope(ScopeFlags::empty()))]
+#[visited_node]
+#[scope]
 #[derive(Debug)]
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(tag = "type", rename_all = "camelCase"))]
@@ -783,8 +785,9 @@ pub enum TSTypePredicateName<'a> {
     This(TSThisType),
 }
 
-#[visited_node(
+#[visited_node]
-    scope(ScopeFlags::TsModuleBlock),
+#[scope(
+    flags(ScopeFlags::TsModuleBlock),
     strict_if(self.body.as_ref().is_some_and(|body| body.is_strict())),
 )]
 #[derive(Debug)]

crates/oxc_traverse/scripts/lib/parse.mjs

@@ -1,8 +1,7 @@
 import {readFile} from 'fs/promises';
 import {join as pathJoin} from 'path';
 import {fileURLToPath} from 'url';
-import assert from 'assert';
+import {typeAndWrappers} from './utils.mjs';
-import {typeAndWrappers, snakeToCamel} from './utils.mjs';
 
 const FILENAMES = ['js.rs', 'jsx.rs', 'literal.rs', 'ts.rs'];
 
@@ -20,70 +19,113 @@ export default async function getTypesFromCode() {
     return types;
 }
 
+class Position {
+    constructor(filename, index) {
+        this.filename = filename;
+        this.index = index;
+    }
+
+    assert(condition, message) {
+        if (!condition) this.throw(message);
+    }
+    throw(message) {
+        throw new Error(`${message || 'Unknown error'} (at ${this.filename}:${this.index + 1})`);
+    }
+}
+
+class Lines {
+    constructor(lines, filename, offset = 0) {
+        this.lines = lines;
+        this.filename = filename;
+        this.offset = offset;
+        this.index = 0;
+    }
+
+    static fromCode(code, filename) {
+        const lines = code.split(/\r?\n/)
+            .map(line => line.replace(/\s+/g, ' ').replace(/ ?\/\/.*$/, '').replace(/ $/, ''));
+        return new Lines(lines, filename, 0);
+    }
+
+    child() {
+        return new Lines([], this.filename, this.index);
+    }
+
+    current() {
+        return this.lines[this.index];
+    }
+    next() {
+        return this.lines[this.index++];
+    }
+    isEnd() {
+        return this.index === this.lines.length;
+    }
+
+    position() {
+        return new Position(this.filename, this.index + this.offset);
+    }
+    positionPrevious() {
+        return new Position(this.filename, this.index + this.offset - 1);
+    }
+}
+
 function parseFile(code, filename, types) {
-    const lines = code.split(/\r?\n/).map(
+    const lines = Lines.fromCode(code, filename);
-        line => line.replace(/\s+/g, ' ').replace(/ ?\/\/.*$/, '')
+    while (!lines.isEnd()) {
-    );
+        if (lines.current() !== '#[visited_node]') {
-    for (let lineIndex = 0; lineIndex < lines.length; lineIndex++) {
+            lines.next();
-        const lineMatch = lines[lineIndex].match(/^#\[visited_node ?([\]\(])/);
+            continue;
-        if (!lineMatch) continue;
-
-        let scopeArgs = null;
-        if (lineMatch[1] === '(') {
-            let line = lines[lineIndex].slice(lineMatch[0].length),
-                scopeArgsStr = '';
-            while (!line.endsWith(')]')) {
-                scopeArgsStr += ` ${line}`;
-                line = lines[++lineIndex];
-            }
-            scopeArgsStr += ` ${line.slice(0, -2)}`;
-            scopeArgsStr = scopeArgsStr.trim().replace(/  +/g, ' ').replace(/,$/, '');
-
-            scopeArgs = parseScopeArgs(scopeArgsStr, filename, lineIndex);
         }
 
-        let match;
+        // Consume attrs and comments, parse `#[scope]` attr
-        while (true) {
+        let match, scopeArgs = null;
-            match = lines[++lineIndex].match(/^pub (enum|struct) ((.+?)(?:<'a>)?) \{/);
+        while (!lines.isEnd()) {
+            if (/^#\[scope[(\]]/.test(lines.current())) {
+                scopeArgs = parseScopeArgs(lines, scopeArgs);
+                continue;
+            }
+            match = lines.next().match(/^pub (enum|struct) ((.+?)(?:<'a>)?) \{/);
             if (match) break;
         }
-        const [, kind, rawName, name] = match,
+        lines.position().assert(match, `Could not find enum or struct after #[visited_node]`);
-            startLineIndex = lineIndex;
+        const [, kind, rawName, name] = match;
 
-        const itemLines = [];
+        // Find end of struct / enum
-        while (true) {
+        const itemLines = lines.child();
-            const line = lines[++lineIndex].trim();
+        while (!lines.isEnd()) {
+            const line = lines.next();
             if (line === '}') break;
-            if (line !== '') itemLines.push(line);
+            itemLines.lines.push(line.trim());
         }
 
         if (kind === 'struct') {
-            types[name] = parseStruct(name, rawName, itemLines, scopeArgs, filename, startLineIndex);
+            types[name] = parseStruct(name, rawName, itemLines, scopeArgs);
         } else {
-            types[name] = parseEnum(name, rawName, itemLines, filename, startLineIndex);
+            types[name] = parseEnum(name, rawName, itemLines);
         }
     }
 }
 
-function parseStruct(name, rawName, lines, scopeArgs, filename, startLineIndex) {
+function parseStruct(name, rawName, lines, scopeArgs) {
     const fields = [];
-    for (let i = 0; i < lines.length; i++) {
+    while (!lines.isEnd()) {
-        let line = lines[i];
+        let isScopeEntry = false, line;
-        const isScopeEntry = line === '#[scope(enter_before)]';
+        while (!lines.isEnd()) {
-        if (isScopeEntry) {
+            line = lines.next();
-            line = lines[++i];
+            if (line === '') continue;
-        } else if (line.startsWith('#[')) {
+            if (line === '#[scope(enter_before)]') {
-            while (!lines[i].endsWith(']')) {
+                isScopeEntry = true;
-                i++;
+            } else if (line.startsWith('#[')) {
+                while (!line.endsWith(']')) {
+                    line = lines.next();
+                }
+            } else {
+                break;
             }
-            continue;
         }
 
         const match = line.match(/^pub ((?:r#)?([a-z_]+)): (.+),$/);
-        assert(
+        lines.positionPrevious().assert(match, `Cannot parse line as struct field: '${line}'`);
-            match,
-            `Cannot parse line ${startLineIndex + i} in '${filename}' as struct field: '${line}'`
-        );
         const [, rawName, name, rawTypeName] = match,
             typeName = rawTypeName.replace(/<'a>/g, '').replace(/<'a, ?/g, '<'),
             {name: innerTypeName, wrappers} = typeAndWrappers(typeName);
@@ -95,10 +137,19 @@ function parseStruct(name, rawName, lines, scopeArgs, filename, startLineIndex)
     return {kind: 'struct', name, rawName, fields, scopeArgs};
 }
 
-function parseEnum(name, rawName, lines, filename, startLineIndex) {
+function parseEnum(name, rawName, lines) {
     const variants = [],
         inherits = [];
-    for (const [lineIndex, line] of lines.entries()) {
+    while (!lines.isEnd()) {
+        let line = lines.next();
+        if (line === '') continue;
+        if (line.startsWith('#[')) {
+            while (!line.endsWith(']')) {
+                line = lines.next();
+            }
+            continue;
+        }
+
         const match = line.match(/^(.+?)\((.+?)\)(?: ?= ?(\d+))?,$/);
         if (match) {
             const [, name, rawTypeName, discriminantStr] = match,
@@ -108,34 +159,50 @@ function parseEnum(name, rawName, lines, filename, startLineIndex) {
             variants.push({name, typeName, rawTypeName, innerTypeName, wrappers, discriminant});
         } else {
             const match2 = line.match(/^@inherit ([A-Za-z]+)$/);
-            assert(
+            lines.positionPrevious().assert(match2, `Cannot parse line as enum variant: '${line}'`);
-                match2,
-                `Cannot parse line ${startLineIndex + lineIndex} in '${filename}' as enum variant: '${line}'`
-            );
             inherits.push(match2[1]);
         }
     }
     return {kind: 'enum', name, rawName, variants, inherits};
 }
 
-function parseScopeArgs(argsStr, filename, lineIndex) {
+function parseScopeArgs(lines, scopeArgs) {
-    if (!argsStr) return null;
+    const position = lines.position();
+
+    // Get whole of `#[scope]` attr text as a single line string
+    let scopeArgsStr = '';
+    let line = lines.next();
+    if (line !== '#[scope]') {
+        line = line.slice('#[scope('.length);
+        while (!line.endsWith(')]')) {
+            scopeArgsStr += ` ${line}`;
+            line = lines.next();
+        }
+        scopeArgsStr += ` ${line.slice(0, -2)}`;
+        scopeArgsStr = scopeArgsStr.trim().replace(/  +/g, ' ').replace(/,$/, '');
+    }
+
+    // Parse attr
+    return parseScopeArgsStr(scopeArgsStr, scopeArgs, position);
+}
+
+function parseScopeArgsStr(argsStr, args, position) {
+    if (!args) args = {flags: 'ScopeFlags::empty()', if: null, strictIf: null};
+
+    if (!argsStr) return args;
 
     const matchAndConsume = (regex) => {
         const match = argsStr.match(regex);
-        assert(match);
+        position.assert(match);
         argsStr = argsStr.slice(match[0].length);
         return match.slice(1);
     };
 
-    const args = {};
     try {
         while (true) {
-            const [key] = matchAndConsume(/^([a-z_]+)\(/);
+            let [key] = matchAndConsume(/^([a-z_]+)\(/);
-            assert(
+            key = key.replace(/_([a-z])/g, (_, c) => c.toUpperCase());
-                ['scope', 'scope_if', 'strict_if'].includes(key),
+            position.assert(Object.hasOwn(args, key), `Unexpected scope macro arg: ${key}`);
-                `Unexpected visited_node macro arg: ${key}`
-            );
 
             let bracketCount = 1,
                 index = 0;
@@ -148,21 +215,16 @@ function parseScopeArgs(argsStr, filename, lineIndex) {
                     if (bracketCount === 0) break;
                 }
             }
-            assert(bracketCount === 0);
+            position.assert(bracketCount === 0);
 
-            const camelKey = key.replace(/_([a-z])/g, (_, c) => c.toUpperCase());
+            args[key] = argsStr.slice(0, index).trim();
-            args[camelKey] = argsStr.slice(0, index).trim();
             argsStr = argsStr.slice(index + 1);
             if (argsStr === '') break;
 
             matchAndConsume(/^ ?, ?/);
         }
-
-        assert(args.scope, 'Missing key `scope`');
     } catch (err) {
-        throw new Error(
+        position.throw(`Cannot parse scope args: '${argsStr}': ${err?.message || 'Unknown error'}`);
-            `Cannot parse visited_node args: ${argsStr} in ${filename}:${lineIndex}\n${err?.message}`
-        );
     }
 
     return args;