Misses class references

[nzakas]

The 2.0.5 version misses references to classes, such as:

class Shoe {
  constructor() {
    //Shoe.x = true;
  }
}
let s = new Shoe();

escope fails to identify new Shoe() as a reference.

[nzakas]

Looks like Shoe ends up in the global scope's through array, and ESLint is picking up on that.

[nzakas]

I tried to figure this out, but I'm still really unfamiliar with how variables are added to scopes and counted as references. :(

[michaelficarra]

Top-level classes should add their binding to the lexical contour because they create block bindings, not var bindings.

[Constellation]

I'll investigate it.

[Constellation]

@nzakas

Hm, I've created the test and it seems passed.

Looks like Shoe ends up in the global scope's through array, and ESLint is picking up on that.

Currently, escope conservatively annotate all global variable as through because escope doesn't know much about global scope.
You can use optimistic: true option.

[nzakas]

This is how ESLint determines if a global reference is unresolved:

1. Filter global scope's through so we get only references that are read (not write).
2. Compare each of those references to those in the global scope's variables array.
3. If there's anything in through that is not in variables, then flag it as unused.

You can see the code here: https://github.com/eslint/eslint/blob/master/lib/rules/no-unused-vars.js#L137

So, the problem is that Foo shows up through even though it's a resolved reference. Your test only validates that Foo is found in global scope's variables, but it should also validate that Foo is not in the global scope's through.

[michaelficarra]

@Constellation Top-level block bindings (let, class, etc.) should be added to a child scope of the global scope. This scope is not dynamic and should be able to statically resolve references.

[nzakas]

Adding to the global scope is also causing issues as it relates to module imports in ESLint.

[Constellation]

@michaelficarra:

Correct? According to the 14.5.16 http://people.mozilla.org/~jorendorff/es6-draft.html#sec-class-definitions-runtime-semantics-evaluation

Let status be the result of BindingClassDeclarationEvaluation of this ClassDeclaration.

And 14.5.15 http://people.mozilla.org/~jorendorff/es6-draft.html#sec-runtime-semantics-bindingclassdeclarationevaluation

Let status be InitializeBoundName(className, value, env).

And at that time, env is the environment record of global scope,

[michaelficarra]

@Constellation: Okay, I was thinking about Modules.

And at that time, env is the environment record of global scope

Only for Script evaluation. See steps 6 and 7 from 15.1.7:

6. Set the VariableEnvironment of scriptCxt to globalEnv.
7. Set the LexicalEnvironment of scriptCxt to globalEnv.

For modules, env will be module.[[Environment]], which was set up in a call to ModuleDeclarationInstantiation (section 15.2.1.16.4). See steps 11 and 12 of section 15.2.1.16.5:

11. Set the VariableEnvironment of moduleCxt to module.[[Environment]].
12. Set the LexicalEnvironment of moduleCxt to module.[[Environment]].

This is the "lexical contour" I was talking about.