Skip to content

Parse parameter decorators#3015

Open
PaperPrototype wants to merge 19 commits intoAssemblyScript:mainfrom
PaperPrototype:parse-parameter-decorators
Open

Parse parameter decorators#3015
PaperPrototype wants to merge 19 commits intoAssemblyScript:mainfrom
PaperPrototype:parse-parameter-decorators

Conversation

@PaperPrototype
Copy link
Copy Markdown

@PaperPrototype PaperPrototype commented Apr 15, 2026

I noticed Josh Tenner, did a final attempt to fix his PR... I am not sure if it was actually correct or not but I am making a new PR based on that one. Mainly wanting to try and see if I can do my own review on these changes and introduce myself into contributing to the AS repository for the coming years. The final goal of all this is for me to write an AST transform for outputting WGSL code which @decorators on parameters will be useful for.

Note

I am not requesting a PR review or expecting this to be merged any time soon (unless the maintainer's want to do that). Making this PR is helpful for me to keep up to date with changes on main branch and seeing the live diff.

Changes proposed in this pull request:

Changes I would like to eventually arrive at in this pull request:

Allow checking for @decorators on parameters and return types using a helper (with least modifications to source code of AssemblyScript). So that a transform plugin can access them and output WGSL code with the same @decorators.

Something akin to this:

(AssemblyScript)

function addNumbers(a: f32, b: f32): f32 {
    return a + b;
}

// Vertex Shader
@wgsl.vertex
function vertexShader(@wgsl.builtin("vertex_index") vid: u32): @wgsl.builtin("position") vec4f {
    let pos: StaticArray<vec2f> = [
      vec2f( 0.0,  0.5),
      vec2f(-0.5, -0.5),
      vec2f( 0.5, -0.5)
    ];

    return vec4f(pos[vid], 0.0, 1.0);
}

// Fragment Shader
@wgsl.fragment
function fragmentShader(): @wgsl.location(0) vec4f {
    return vec4f(0.0, 1.0, 0.0, 1.0); // Green
}

(WGSL)

fn addNumbers(a: f32, b: f32) -> f32 {
    return a + b;
}

// Vertex Shader
@vertex
fn vertex_shader(@builtin(vertex_index) vid: u32) -> @builtin(position) vec4f {
    let pos = array(
        vec2f( 0.0,  0.5),
        vec2f(-0.5, -0.5),
        vec2f( 0.5, -0.5)
    );
    return vec4f(pos[vid], 0.0, 1.0);
}

// Fragment Shader
@fragment
fn fragment_shader() -> @location(0) vec4f {
    return vec4f(0.0, 1.0, 0.0, 1.0); // Green
}

Afaik this code allows reading @decorators from parameters using a helper function. I'll be doing my own research into this and trying to acquaint myself with AssemblyScript internals. I mainly wanted the PR so I can keep it up to date with main and see a live diff of what is different from main.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

NOTE: I don't use Codex or ClaudeCode, nor am I interested in using them (I don't even have them installed). I do make use of Claude chat (claude.ai/new) quite often though for research and smaller pieces of code of which I often heavily modify, as well as occasional Copilot inline editing provided by VS Code.

Related Issues:
#2497
#20

jtenner and others added 19 commits April 12, 2026 09:22
Extend parameter AST nodes to retain decorators, including explicit-this decorators on function signatures, without forcing a broad constructor API churn.

Teach both parameter parsers to accept stacked decorators on regular, rest, explicit-this, constructor-property, function-type, and parenthesized-arrow parameters.

Update the AST builder to serialize parameter decorators inline so parser fixtures can round-trip the new syntax faithfully.

Add a focused parser fixture covering the preserved syntax surface before deferred validation is introduced.
Add a Program-owned validation pass that walks the final AST and reports TS1206 once per decorated parameter, using the full decorator-list span for the diagnostic range.

Invoke that validation from compile after transforms have had their afterInitialize window, so transformers can still remove parameter decorators before any diagnostics are emitted.

Add a dedicated compiler rejection fixture covering regular, rest, explicit-this, constructor-property, function-type, function-expression, and arrow-parameter cases.
Add a dedicated transform input containing the same invalid parameter-decorator forms exercised by the compiler rejection fixture.

Introduce ESM and CommonJS afterInitialize transforms that walk the AST and strip parameter decorators, including explicit-this decorators on function signatures.

Extend the transform test scripts to compile that input with the stripping transforms, proving no TS1206 diagnostics are emitted once transforms remove the decorators in time.
Replace the bespoke parameter-decorator validation pass in Program with a small validator built on top of a shared AST walker.

The previous implementation worked, but it reimplemented a near-complete AST traversal inside program.ts for one feature. That made Program responsible for AST topology, duplicated traversal logic, and created exactly the kind of ad hoc pass review feedback called out: every AST shape change would have to remember to update this validator as well.

This change separates three concerns more cleanly:

- parser.ts records which source-level statements actually preserved parameter decorators while parsing
- ast.ts owns subtree traversal through a generic NodeWalker utility
- program.ts only performs the parameter-specific check after afterInitialize

In other words, the parser decides what roots are relevant, the walker decides how to descend the tree, and the validator decides what to report.

This is better because it keeps AST traversal knowledge in one place, removes a large one-off recursive pass from Program, and makes future AST validation/analysis passes much smaller if they need to inspect subtrees later.

The validator still runs after afterInitialize so transforms can inspect or remove preserved parameter decorators first, and the tests continue to cover parser round-tripping, compiler rejection, and transform-time removal.

Also add a namespace regression case to ensure preserved parameter decorators nested under a tracked source-level statement are still validated and can still be stripped by transforms.
Drop AST imports from src/program.ts that became unused after moving subtree traversal into NodeWalker. This fixes the lint warnings reported in CI and keeps the parameter decorator refactor clean.
Follow the existing AST convention that Range is the last constructor field and the last factory/helper parameter. This updates ParameterNode and Node.createParameter accordingly, and adjusts all call sites.
Account for explicit-this parameter decorators in the same way as parameter decorators: wire explicitThisDecorators through FunctionTypeNode and Node.createFunctionType, keep it adjacent to explicitThisType, and leave range as the last constructor/helper parameter. Update all call sites accordingly.
Remove the duplicate parameter-decorator serializer in ASTBuilder and fold the inline formatting case into serializeDecorator with an optional flag. This keeps decorator serialization in one place while preserving the spacing differences between declaration decorators and parameter decorators.
Rename the parser helper that consumes and reports decorators found after a parameter has already started from reportInvalidParameterDecorators to tryParseParameterDecorators, matching its behavior more closely and aligning with the review suggestion.
Comment thread src/ast.ts
}

/** Generic AST walker. */
export abstract class NodeWalker {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a whole new AST node walker...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you decided to pick up the baton. Not sure why NodeWalker is needed here. Feels like this could’ve been done without it.

If you want to keep working on this, the first thing to do is try to understand the internal architecture of at least the AssemblyScript parser. Claude can probably explain it better. Then try giving it a task to review the implementation in this PR and rewrite everything (except the tests), sticking to the existing code style and paradigms, with as little “creative liberty” as possible, basically minimize the diff as much as you can.

Also, I recently came across an interesting technical prompt for Claude by Andrej Karpathy I haven’t tried it yet. LLMs are pretty much useless for the kind of tasks I’m dealing with right now, but it seems like a decent starting point. Of course, ideally, you’d want to figure everything out yourself.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the NodeWalker here - if needed at all - can be delegated into the transform itself, with only the bits and pieces added to the AS parser and compiler that are strictly necessary to support parameter decorators. By default, if no transform consumes these decorators, the compiler would reject them as unsupported as it does with other decorators. That's not the feature though. The feature is that now a transform can consume them.

Comment thread src/ast.ts
export abstract class NodeWalker {

/** Indicates whether walking has been stopped. */
stopped: bool = false;
Copy link
Copy Markdown
Member

@MaxGraey MaxGraey Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this property is never set to true during the tree traversal. It is always false. No one outside is set it either. I just don’t understand what the point of all this is. The comments don’t offer any clear explanation either. It looks like some sort of AI artifact / hallucination. NodeWalker is just used as base Visitor-like class for ParameterDecoratorValidator which by itself looks like a highly questionable decision

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.

4 participants