fix(compiler): enforce super constructor call in derived class constructors #5354
Conversation
Signed-off-by: monada-bot[bot] <[email protected]>
Signed-off-by: monada-bot[bot] <[email protected]>
monadabot
added
the
⚠️ pr/review-mutation
| @@ -98,7 +99,7 @@ class $Root extends $stdlib.std.Resource { | |||
| } | |||
| class Derived extends Base { | |||
| constructor($scope, $id, ) { | |||
| super($scope, $id); | |||
| super($scope, $id, ); | |||
There was a problem hiding this comment.
Can this be avoided? Not critical if it's a hassle
There was a problem hiding this comment.
I was wondering about his change, it was generated after I added super() to the inflight constructor and I ran e2e tests. I don't know if this is critical, but seems suspicious to me, so I pushed it to see if somebody knows whats going on 🙂
|
Hi, This PR has not seen activity in 20 days. Therefore, we are marking the PR as stale for now. It will be closed after 7 days. |
| @@ -480,6 +482,84 @@ impl<'s> Parser<'s> { | |||
| } | |||
| } | |||
|
|
|||
| fn report_missing_super_call(&self, node: &Node, scope: &Scope) { | |||
There was a problem hiding this comment.
I'd change this logic. Instead of re-scanning the entire tree I'd just modify build_class_statement. There you can look at the returned value from build_scope when creating the scope for the initializer. If the first statement (or actually any statement) is a StmtKind::SuperConstructor and we have a defined parent for the class then you're good to go.
|
Hi, This PR has not seen activity in 20 days. Therefore, we are marking the PR as stale for now. It will be closed after 7 days. |
Closes #4277
This PR adds an error to compilation if
super()was not called in a derived class constructor.One last point is questionable. If you take a look at
__snapshots__/invalid.ts.snapthe error is also thrown ifsuper()is not the first statement in a constructor. This is tricky to solve.Further I got a weird change in
test_corpus/valid/optionals.test.w_compile_tf-aws.mdafter I added the now requiredsuper()calls tooptionals.test.w. This is not related to my changes inparser.rsbut I am wondering though.Checklist
pr/e2e-fulllabel if this feature requires end-to-end testingBy submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.