Skip to content

Parse parameter decorators#3015

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

Parse parameter decorators#3015
PaperPrototype wants to merge 34 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

Testing commands:

npm run build
npm run test:parser -- parameter-decorators
npm run test:compiler -- parameter-decorators --noDiff
npm run test:transform

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 Outdated
}

/** 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.

Copy link
Copy Markdown
Author

@PaperPrototype PaperPrototype Apr 16, 2026

Choose a reason for hiding this comment

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

This is definitely going to take me some time (especially because of how new I am). Also do you recommend I install claude code, or just paste the relevant files into a claude chat and analyze from there feeding it more context as it needs? I am somewhat unfamiliar with claude code and i've heard that it will modify a lot of stuff you may not want it to.

EDIT: this command worked well for getting the entire diff and pasting it into Claude git diff main...HEAD | pbcopy, it also copies the diff to your clipboard on MacOS

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.

Main goal right now is to just understand everything.

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.

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.

It would be ideal, at least in my mind, if your code could remain portable such that a math function written with @wgsl decorators could still be imported and used by AS.

Comment thread src/ast.ts Outdated
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

@PaperPrototype
Copy link
Copy Markdown
Author

PaperPrototype commented Apr 16, 2026

Result of running command npm run test:parser -- parameter-decorators is:

> assemblyscript@0.0.0 test:parser
> node --enable-source-maps tests/parser parameter-decorators

Testing parser/parameter-decorators.ts
diff OK

[ SUCCESS ]

Result of running command npm run test:compiler -- parameter-decorators --noDiff is:

> assemblyscript@0.0.0 test:compiler
> node --enable-source-maps --no-warnings tests/compiler parameter-decorators --noDiff

# compiler/parameter-decorators

- compile debug
  ERROR TS1206: Decorators are not valid here.
     :
   1 │ function regular(@first value: i32): void {}
     │                  ~~~~~~
     └─ in parameter-decorators.ts(1,18)

  ERROR TS1206: Decorators are not valid here.
     :
   2 │ function rest(@rest ...values: i32[]): void {}
     │               ~~~~~
     └─ in parameter-decorators.ts(2,15)

  ERROR TS1206: Decorators are not valid here.
     :
   3 │ function withthis(@self this: i32, value: i32): i32 { return this; }
     │                   ~~~~~
     └─ in parameter-decorators.ts(3,19)

  ERROR TS1206: Decorators are not valid here.
     :
   6 │ constructor(@field public value: i32) {}
     │             ~~~~~~
     └─ in parameter-decorators.ts(6,15)

  ERROR TS1206: Decorators are not valid here.
     :
   7 │ method(@arg value: i32): void {}
     │        ~~~~
     └─ in parameter-decorators.ts(7,10)

  ERROR TS1206: Decorators are not valid here.
      :
   10 │ type Callback = (@arg value: i32) => void;~~~~
      └─ in parameter-decorators.ts(10,18)

  ERROR TS1206: Decorators are not valid here.
      :
   11 │ const expression = function(@arg value: i32): void {};~~~~
      └─ in parameter-decorators.ts(11,29)

  ERROR TS1206: Decorators are not valid here.
      :
   12 │ const arrow = (@arg value: i32): void => {};~~~~
      └─ in parameter-decorators.ts(12,16)

  ERROR TS1206: Decorators are not valid here.
      :
   14 │ export function nested(@arg value: i32): void {}
      │                        ~~~~
      └─ in parameter-decorators.ts(14,26)

  ERROR AS102: User-defined: "EOF"
      :
   17 │ ERROR("EOF");~~~~~~~~~~~~
      └─ in parameter-decorators.ts(17,1)

  FAILURE 10 compile error(s)
  ---
  10 compile error(s)
  ---
  SKIPPED (104.258 ms)

- compare stderr
  SUCCESS (0.195 ms)

Time: 115 ms

[ SUCCESS ]

Result of running npm run test:transform is:

> assemblyscript@0.0.0 test:transform
> npm run test:transform:esm && npm run test:transform:cjs


> assemblyscript@0.0.0 test:transform:esm
> node bin/asc tests/compiler/empty --transform ./tests/transform/index.js --noEmit && node bin/asc tests/compiler/empty --transform ./tests/transform/simple.js --noEmit && node bin/asc tests/transform/parameter-decorators.ts --transform ./tests/transform/remove-parameter-decorators.js --noEmit

Transform loaded
- constructor
- afterParse
  defer
  resolve
  defer
  resolve
  defer
  resolve
  complete
- afterInitialize
  defer
  resolve
- afterCompile
Simple transform loaded
- afterParse
- afterInitialize
- afterCompile
Parameter decorator removal transform loaded
- afterInitialize strip parameter decorators

> assemblyscript@0.0.0 test:transform:cjs
> node bin/asc tests/compiler/empty --transform ./tests/transform/cjs/index.js --noEmit && node bin/asc tests/compiler/empty --transform ./tests/transform/cjs/simple.js --noEmit && node bin/asc tests/transform/parameter-decorators.ts --transform ./tests/transform/cjs/remove-parameter-decorators.js --noEmit

CommonJS transform loaded
- constructor
- afterParse
- afterInitialize
- afterCompile
Simple CommonJS transform loaded
- afterParse
- afterInitialize
- afterCompile
CommonJS parameter decorator removal transform loaded
- afterInitialize strip parameter decorators

@PaperPrototype
Copy link
Copy Markdown
Author

PaperPrototype commented Apr 16, 2026

Okay so after messing about I think I understand what needs done.

Default behaviour of decorators in AS is to ignore them if they are invalid. For example I can do:

@blabla
@interesting("")
@wgsl.vertex("main")
function initBoard(): void { }

... and there is no error.

The problem is that doing the following:

@blabla
@interesting("")
@wgsl.vertex("main")
function initBoard(@first hello: i32): @location(0) i32 {
  return 0;
}

...will throw compile errors:

ERROR TS1003: Identifier expected.
    :
 45 │ function initBoard(@first hello: i32): @location(0) i32 {
    │                   ~
    └─ in sketch.ts(45,19)

FAILURE 1 parse error(s)
ERROR TS1110: Type expected.
    :
 45 │ function initBoard(hello: i32): @location(0) i32 {
    │                                 ~
    └─ in sketch.ts(45,33)

FAILURE 1 parse error(s)

So we only need (as dcode put it) "the bits and pieces added to the AS parser and compiler that are strictly necessary to support parameter decorators." Yet somehow this PR seems to be doing a LOT more than that.

@PaperPrototype
Copy link
Copy Markdown
Author

PaperPrototype commented Apr 16, 2026

  1. Do we want an error to be thrown when a transform does not consume the parameter decorators? eg. function f(@a x: i32): void {} -> TS1206: Decorators are not valid here. but still parse successfully so transforms can remove them. Or...

  2. Can parameter decorators simply be ignored by the compiler like other decorators?

I prefer 2, the latter case. @MaxGraey @dcodeIO

Also, after research it seems the only changes needed are in ast.ts and parser.ts (and possibly src/extra/ast.ts). Does this sound correct?

@MaxGraey
Copy link
Copy Markdown
Member

MaxGraey commented Apr 16, 2026

ERROR TSXXXX errors relate to TypeScript's compiler and semantics. We can parse parameter decorators as we like and still error our specific ERROR ASXXX errors for invalid decorators which not pass our requirements.

Also, we can add suppression flag for compiler to completely ignore decorator errors. But in this case transformer should validate it by itself

@PaperPrototype
Copy link
Copy Markdown
Author

PaperPrototype commented May 1, 2026

@MaxGraey I was hoping for some clarification on something you mentioned

ERROR TSXXXX errors relate to TypeScript's compiler and semantics. We can parse parameter decorators as we like and still error our specific ERROR ASXXX errors for invalid decorators which not pass our requirements.

Current behavour I have implemented outputs these types of errors:

  ERROR TS1206: Decorators are not valid here.
     :
   1  function regular(@first value: i32): void {}
                       ~~~~~~
     └─ in parameter-decorators.ts(1,18)

  ERROR TS1206: Decorators are not valid here.
     :
   2  function rest(@rest ...values: i32[]): void {}
                    ~~~~~
     └─ in parameter-decorators.ts(2,15)

Should I implement a custom ASXXX error code or continue to use TS1206? If we want custom error codes then how should I go about doing that?

Here is the test commands if you want to check the output for yourself:

npm run test:parser -- parameter-decorators
npm run test:compiler -- parameter-decorators --noDiff
npm run test:transform
npm run build

@MaxGraey
Copy link
Copy Markdown
Member

MaxGraey commented May 1, 2026

If there's a suitable error in TS, it's best to use it but TS transpiler is fairly conservative and generally only throws errors that are still in tc39/proposals. And I don't recall what the status is there for parameter decorators.

Comment thread src/compiler.ts Outdated
Comment on lines +10631 to +10659
private validateParameterDecorators(): void {
let program = this.program;
if (program.parameterDecoratorsValidated) return;
program.parameterDecoratorsValidated = true;
for (let source of program.sources) {
let fts = source.decoratedFunctionTypes;
if (!fts) continue;
for (let i = 0, k = fts.length; i < k; ++i) {
let ft = fts[i];
let thisDecorators = ft.explicitThisDecorators;
if (thisDecorators && thisDecorators.length > 0) {
this.error(
DiagnosticCode.Decorators_are_not_valid_here,
Range.join(thisDecorators[0].range, thisDecorators[thisDecorators.length - 1].range)
);
}
let params = ft.parameters;
for (let j = 0, m = params.length; j < m; ++j) {
let decorators = params[j].decorators;
if (decorators && decorators.length > 0) {
this.error(
DiagnosticCode.Decorators_are_not_valid_here,
Range.join(decorators[0].range, decorators[decorators.length - 1].range)
);
}
}
}
}
}
Copy link
Copy Markdown
Member

@MaxGraey MaxGraey May 1, 2026

Choose a reason for hiding this comment

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

I don’t like that transformer validation happens in the compiler. If it’s not valid for upstream AS semantics, it’s better to move it to transform utils or add it to their pipeline automatically.

TS will not merge decorators soon, and even if it does, parameters are still unclear:
https://github.com/tc39/proposal-decorators/blob/master/EXTENSIONS.md#parameter-decorators-and-annotations

If param decorator semantics are not standardized there either, it feels wrong to validate this inside our compiler.

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.

Actually, I’d just pass the bare minimum of what needs to be fed into the transformer, and let the transformer itself parse, canon and validate the decorators via the AST. Especially considering that this feature is pretty niche in terms of general use.

Copy link
Copy Markdown
Author

@PaperPrototype PaperPrototype May 1, 2026

Choose a reason for hiding this comment

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

To be honest, I have no idea what I am doing 😅

Good to know about the decorators proposal for TS, I wasn't aware of it before 👍

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.

What do you mean by "add it to their pipeline automatically"?

Copy link
Copy Markdown
Member

@MaxGraey MaxGraey May 1, 2026

Choose a reason for hiding this comment

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

I mean extend transform's API by adding new validation entry point here between listFiles and afterParse and add validation hook to this entry point instead validate by itself. This would allow users to validate the code in the transformer themselves before compilation but after parsing

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.

Actually, I’d just pass the bare minimum of what needs to be fed into the transformer, and let the transformer itself parse, canon and validate the decorators via the AST. Especially considering that this feature is pretty niche in terms of general use.

Sorry, I needed to reload the page before I saw this. When you say "Transformer" what are you referring to? I'm assuming you mean the implementors who will want to make use of these decorators? Eg. the example transformers in /tests/transform? Still very new to all the verbiage being thrown around here, sorry about the extreme noobiness

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.

Oh unless you are referring to the AssemblyScript AST transformer...

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 needed to reload the page before I saw this. When you say "Transformer" what are you referring to? I'm assuming you mean the implementors who will want to make use of these decorators?

yes

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.

Or don't do this and just use afterParse hook only.

Simply put, in this case we have ambiguity. Should we fail because we can’t handle such decorators, or not fail and hope that the Transformer will take on this role? An additional validator interface's method for Transformer API would answer this question clearly.

I think I finally get what you meant. I made some notes:

  • AST can be extended without breaking changes, this is perfectly fine and anything that is added will just be ignored by the compiler but at least transformer plugins can then make use of it. No need to validate and throw errors unless it is an AST parse error.
    • Need to add a transformer hook so transformers can add their own validation. This way AssemblyScript can delegate validation to transformers instead of having to handle it internally, which doesn't make sense because AS shouldn't be in charge of worring about that in the first place.

Is this correct? Just a "👍" is fine for me, unless I am completely wrong 🤣

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.

Yeah that's pretty accurate

Comment on lines +35 to +53
Void: 48,
While: 49,
ClassDeclaration: 51,
EnumDeclaration: 52,
FieldDeclaration: 54,
FunctionDeclaration: 55,
InterfaceDeclaration: 57,
MethodDeclaration: 58,
NamespaceDeclaration: 59,
TypeDeclaration: 60,
VariableDeclaration: 61
};

const LiteralKind = {
Template: 3,
Array: 5,
Object: 6
};

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'd use the TS version of the transformer to handle all of this

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.

TS version of the transformer?

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.

Oh I see, write in in typescript so i can import these from assemblyscript directly, yep

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.

yes

…ipt directly (Done with Claude Code)

I still don't think this is entirely correct because decorators are still not properly implement in the AST but at least it removes all the duplicate types.

Not sure why it changed the root package.json other than because we changed from js to ts. This also might be incorrect but I'll leave it alone for now.

According to claude, here's what changed:

tests/transform/remove-parameter-decorators.ts (new, 18 lines) — replaces the deleted 330-line JS file. Uses import type { Program } from assemblyscript for proper typing, and reads directly from source.decoratedFunctionTypes instead of walking the entire AST. No hardcoded NodeKind numbers at all.

tests/transform/cjs/remove-parameter-decorators.js (15 lines, was 330) — same simplification: drops the full AST walk, uses source.decoratedFunctionTypes directly.

package.json:88 — ESM test now runs with --experimental-strip-types --no-warnings and points to the .ts file.

The reason both transforms shrank so dramatically: the old versions walked the entire AST because they had no other way to find parameters with decorators. Now that the parser tracks source.decoratedFunctionTypes directly, a transform can just iterate that list.
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