fix: Incorrect scope analysis of class name in extends expression

[lizhihao132]

fix this bug: Incorrect scope analysis of class name in extends expression, as the code follows, the C in probeHeritage function body should be the class C, not var C
more details see here: ref

var C = 'outside';

var cls = class C extends (
    probeHeritage = function() { return C; },
    setHeritage = function() {  }
  ) {
  method() {
    return C;
  }
};

[mdjermanovic]

I still don't see why Identifier would be a special case. Per the specification (15.7.14 Runtime Semantics: ClassDefinitionEvaluation), ClassHeritage is always evaluated in the class scope.

Here's an example that demonstrates this:

class C {}

(class C extends C {}); // ReferenceError: Cannot access 'C' before initialization

[lizhihao132]

The example you provided is indeed correct, and I understand your point. However, you might be referring to the scope during dynamic runtime: In this code, class C extends C throws an error because defining a class C that extends C is semantically contradictory, hence the error. However, eslint-scope is a concept of static scope. In class C extends C, the second C should obviously be the C in the scope where this code is located. That's why I proposed the solution above: when the target of 'extends' is an Identifier, we need to traverse this node in the current scope first, then create a class sub-scope to traverse other nodes.

So, keeping the Identifier as a special case, which is the solution provided in this pull request, allows all tests to be passed when running npm run test in the eslint-scope main directory. Otherwise, an existing case would fail: es6-class.js, specifically a few assertion tests related to references, such as:

expect(scope.references).to.have.length(2);
expect(scope.references[0].identifier.name).to.be.equal("Base");
expect(scope.references[1].identifier.name).to.be.equal("Derived");

the main code as follows:

class Derived extends Base {
                constructor() {
                }
            }
            new Derived();

other information

[mdjermanovic]

However, eslint-scope is a concept of static scope. In class C extends C, the second C should obviously be the C in the scope where this code is located.

eslint-scope aims to represent JavaScript scoping. If the second C is, per the specification, reference to the first C, it should be the same in eslint-scope. If the existing tests are wrong, we can fix them.

[bradzacher]

I was just looking because I was curious - @typescript-eslint/scope-manager works fine for this case.

The fix we implemented was simply to always visit the superclass after the class name is defined. Not conditionally - just always do it.

https://github.com/typescript-eslint/typescript-eslint/blob/6128a02cb15d500fe22fe265c83e4d7a73ae52c3/packages/scope-manager/src/referencer/ClassVisitor.ts#L51-L72

[mdjermanovic]

@lizhihao132 are you still working on this?

[mdjermanovic]

No response, so closing as we need to finish this in time for ESLint v9.0.0-beta.0