feat(ast): heritage array is now optional, not empty array

[JamesHenry]

I'll probably add the other heritage related alignment points before merging

[codecov[bot]]

Codecov Report

Merging #120 into master will decrease coverage by 0.01%. The diff coverage is 94.44%.

@@            Coverage Diff            @@
##           master    #120      +/-   ##
=========================================
- Coverage   93.22%   93.2%   -0.02%     
=========================================
  Files           8       8              
  Lines        1357    1368      +11     
  Branches      319     324       +5     
=========================================
+ Hits         1265    1275      +10     
  Misses         51      51              
- Partials       41      42       +1

Impacted Files	Coverage Δ
src/semantic-errors.ts	77.77% <ø> (ø)	:arrow_up:
src/ast-node-types.ts	100% <100%> (ø)	:arrow_up:
src/convert.ts	93.82% <94.11%> (-0.05%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f11e368...1436d61. Read the comment docs.

[JamesHenry]

@armano2 in https://github.com/JamesHenry/typescript-estree/pull/120/commits/e40b9d7c8f1a5718a5a05df15a626cf6a08ff35b I have moved the ignore for abstract classes to node patching.

My gut reaction is that aligning with babel here might not actually a step in the right direction. I feel like this is similar to other things we've had before where it makes the most sense for the construct to have its own unique node type...

What do you think? I could imagine this would be harder to deal with in a linting situation, potentially causing issues with existing rules...

[armano2]

i was thinking about this to, if we unify those nodes, there will be no easy way to query them in eslint.

i will just suggest renaming them, ClassImplements to TSClassImplements (this is no es node), and we don't want that flow rules are going to play with it.

i will follow name of property from babel:

• extends
• implements

instead of heritage

you can use something like this:

     TSExpressionWithTypeArguments(node: any, parent: any) {
        if (parent.type === 'TSInterfaceDeclaration') {
          node.type = 'TSInterfaceHeritage';
        } else if (
          parent.type === 'ClassExpression' ||
          parent.type === 'ClassDeclaration'
        ) {
          node.type = 'ClassImplements';
        }
      }

and we should log suggestion/request on babel repo for this

[JamesHenry]

One thing is that TSExpressionWithTypeArguments has expression not id.

To be honest though, I'm struggling to create valid TypeScript where it is anything other than an Identifier. Any idea what expression would be valid in extends ...?

cc @uniqueiniquity

[JamesHenry]

@armano2 I have made all the changes we discussed, please could you take a look?

[armano2]

typescript also parses code like this correctly:

interface foo extends foo.call {} // valid

interface foo extends foo.call() {} // TS2499

interface foo extends foo() {} // TS2499

interface foo extends ({ foo: bar }) {} // TS2499

[armano2]

One thing is that TSExpressionWithTypeArguments has expression not id.

To be honest though, I'm struggling to create valid TypeScript where it is anything other than an Identifier. Any idea what expression would be valid in extends ...?

cc @uniqueiniquity

i'm starting think that expression is better suited for this because it can contain Identifier and QualifiedName

[JamesHenry]

Pushing shortly, thanks for the updates, and good spot on the TSQualifiedName

[JamesHenry]

@armano2 let me know what you think of the last two commits

[JamesHenry]

Thanks! Let's stabilize for a bit now while we catch the ESLint ecosystem up and take stock

[armano2]

all of nodes are now aligned -> most of issues is due to range bugs in babel :)

i'm running my tests now

[JamesHenry]

:tada: This PR is included in version 18.0.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[JamesHenry]

Well apart from all the property and kind tweaks we have to make to nodes in ast-alignment/utils right? :)

[armano2]

yes, in my tests i'm doing same thing.