feat(linter): add eslint/constructor-super

[Sysix]

closes #2280

This is still not completed but a good start.
Currently I am questioning myself if this make sense how I built it.

Still in nursery!

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[codspeed-hq]

CodSpeed Performance Report

Merging #7651 will not alter performance

_{Comparing Sysix:feat-linter-add-constructor-super (36f1f87) with main (3858221)}

Summary

✅ 29 untouched benchmarks

[Sysix]

@camc314 I am feeling stupid parsing all possible Statements for the right error. Here are some key points what this rule does:

• Reports when invalid super call (¹)
• Reports when no super call is found (²)
• Reports when multiple super calls found (³)

¹ When no super class is there or not a valid one like null, 100, ...

² Some does not get executed because of a return before.

³ Some are expressions like super() || super().
Some are in loops.

^{2 3} Some can be in switch cases with and without break.

Do you have a better idea before I try to be a runtime-parser :)?

[Sysix]

@Boshen do you have any idea? :)

[camc314]

apologies @Sysix , last few days have been very busy.

Without diving too deep into the eslint code, i could guess that using a visitor would be the best approach - something like:

struct ConstrucorSuperVisitor<'a,'b> {
    semanticL &'b Semantic<'a>,
    stack: Vec<AstKind<'a>>,
    super_found: bool
    invalid_super_calls: Vec<Span> // maybe vec of diagnostic actually
}

impl <'a> Visit<'a> for ConstrucorSuperVisitor {
}

then inside the run, you would create the visitor and call visit_function_body

Any time you found a super() call, you could check the stack to see what it's parent is.

i guess potential problems with this approach is that it would be tricky to check stuff like:

class A extends B {
    constructor() {
      try { a; } catch (err) { super(); }
  }
}

maybe for those it'd make sense to use the cfg?

[Sysix]

Okay that looks like almost what I am doing. I added the ToDos in the nursery-block.

i guess potential problems with this approach is that it would be tricky to check stuff like:

class A extends B {
    constructor() {
      try { a; } catch (err) { super(); }
  }
}

as far as I understood the rule, all super calls in try and catch are not valid. you need a finally block.
This will still be funny for try { super() } catch { /* nothing */ } finally { super() }

I would be happy if we can merge this current state :)

[camc314]

Okay that looks like almost what I am doing. I added the ToDos in the nursery-block.

i guess potential problems with this approach is that it would be tricky to check stuff like:

class A extends B {
    constructor() {
      try { a; } catch (err) { super(); }
  }
}

as far as I understood the rule, all super calls in try and catch are not valid. you need a finally block. This will still be funny for try { super() } catch { /* nothing */ } finally { super() }

🫠 i think it would depend. the following should be valid i think

 class A extends B {
     constructor() {
       try { super(); } catch (err) { throw err }
   }
 }

eslint playround

I would be happy if we can merge this current state :)

i can take a propert look tomorow, thanks for your work on this

[Sysix]

"class A extends B { constructor() { try { super(); } catch (err) {} } }",

This is one of the rule, which should fail 😆

[Boshen]

Thank you trying to tackle one of the last and hardest rules, because it requires the CFG :-(

https://github.com/eslint/eslint/blob/90c1db9a9676a7e2163158b37aef0b61a37a9820/lib/rules/constructor-super.js#L156-L367