fix(transformer/typescript): parameter property assignments before conditional super() - #20529
Conversation
bfda110 to
e2ca96f
Compare
|
|
||
| fn statement_contains_super_call(stmt: &Statement<'a>) -> bool { | ||
| match stmt { | ||
| Statement::ExpressionStatement(stmt) => stmt.expression.is_super_call_expression(), |
There was a problem hiding this comment.
| Statement::ExpressionStatement(stmt) => stmt.expression.is_super_call_expression(), | |
| Statement::ExpressionStatement(stmt) => match &stmt.expression { | |
| Expression::SequenceExpression(seq) => { | |
| seq.expressions.iter().any(Expression::is_super_call_expression) | |
| } | |
| expr => expr.is_super_call_expression(), | |
| }, |
Thanks for working on this! I hit a closely related case that this PR doesn't cover yet (super() inside a SequenceExpression).
@babel/plugin-proposal-decorators generates code like this:
export class A extends B {
constructor(private s: any) { super(), initExtra(this); }
x = init(this, 1);
}Output (both with current main and with this PR applied):
export class A extends B {
constructor(s) {
this.s = s;
this.x = init(this, 1);
super(), initExtra(this);
}
x;
}The ExpressionStatement branch of the new statement_contains_super_call only matches when the expression is the super call, so the sequence expression falls through and the assignments get inserted at position 0.
Here as test case:
class MyError3 extends Error {
constructor(
public code: string,
) {
super(code), init(this);
}
}
// to
class MyError3 extends Error {
constructor(code) {
super(code), init(this);
this.code = code;
}
}Would you mind including this in this PR?
There was a problem hiding this comment.
Fixed. super() inside a sequence expression is now detected, so parameter-property assignments are emitted after the complete expression statement. Added the suggested regression case; the focused conformance test passes.
camc314
left a comment
There was a problem hiding this comment.
can you run just coverage?
I would expect
oxc/tasks/transform_conformance/snapshots/babel.snap.md
Lines 1109 to 1110 in 76ec104
e2ca96f to
83da050
Compare
|
@camc314 Ran |
83da050 to
600474d
Compare
…nditional super() When `super()` is inside a top-level control flow statement (if/else, switch, try, labeled), parameter property assignments were inserted at the start of the constructor body — before `super()`. Fixes oxc-project#20527 Amp-Thread-ID: https://ampcode.com/threads/T-019f9fdf-7db3-700c-a022-472ecfe7119d
600474d to
4a253d0
Compare
|
@camc314 I initially misunderstood your point: you expected the fixture itself to be removed from the coverage failures, not just the assignment placement to be fixed. I added the standard Oxc override for the intentional |
Closes #20527
get_super_call_positiononly looked forsuper()as a direct top-levelExpressionStatement. Whensuper()was inside anif/elseorswitch, it was not found, so parameter property assignments (this.x = x) were inserted at position 0 — beforesuper(). This causes a runtimeReferenceError.The fix makes
get_super_call_positionwalk into top-level control flow statements (if,switch,try,labeled, blocks) to find the containing statement, and inserts assignments after it.