Breaking: upgrade espree and support new class features (refs #14343)

[mysticatea]

WIP.

Remaining steps:

• update id-denylist
• update id-length
• update id-match
• check if other rules need updates or not.
• check rules again after rest.
• release espree and eslint-scope.
• update package.json with the new versions of espree and eslint-scope.

---

Refs #14343
Fixes #14632

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[X] Changes an existing rule
[X] Other, please explain: The espree update contains breaking changes.

What changes did you make? (Give an overview)

• Upgrade espree. This contains a big breaking change about acornjs/acorn@85f4150. As a result of this AST structure change, some rules (camelcase, id-denylist, id-length, and id-match) are broken. This PR contains the fix of those rules as well. It will fix [camelcase] false positive when parsing object pattern with a default value, and using a non-default parser. #13021.
• Support public class fields, private class fields, private class methods/accessors, and static of them. You need parserOptions: { ecmaVersion: 2022 } to use the new class features. This PR also change a ton of rules to support the new class features.

Commit	Rule	Description
150fd19	camelcase	This is a large refactoring. Now the rule understands code semantics better by using variables and references of eslint-scope.
d8a912b	class-methods-use-this	It perceives functions at class field initializers.
e50a84c	computed-property-spacing	It perceives the computed keys of class fields.
e7e860f	func-names	Functions at field initializers have inferred names. The rule with "as-needed" option recognizes that.
-	id-denylist	(not yet)
-	id-length	(not yet)
-	id-match	(not yet)
e5cff29	indent	It perceives class fields and indent class fields.
553aac0	keyword-spacing	It's related in three contextual keywords: get, set, static. Tweaks spacing between those and private identifier. And it additinally checks class fields.
7da3ed4	no-dupe-class-members	It perceives public class fields. Duplication of private members is syntax error.
4404054, 9d1700d	no-eval	It perceives that this.eval() at class field initializers is not eval. The ast-utils change in 7ccd811 and 8009406 is related.
29275bd	no-extra-semi	It perceives empty class elements preceded by class fields.
7ccd811, 8009406	no-invalid-this	It perceives that this in class field initializers is valid.
160013d	no-multi-assign	It perceives assignments at class field initializers.
06c676c	no-self-assign	It perceives private member accesses.
9a494a2	no-undef-init	It perceives unnecessary undefined at class field initializers.
4098d55	no-underscore-dangle	It perceives underscore in private identifiers.
40492d3	no-unreachable	It perceives that class fields are unreachable in rare cases. See also this comment.
ef87576	no-useless-computed-key	It perceives the computed keys of class fields.
b320bbd	operator-linebreak	It perceives = of class field initializers.
6fffdc6	prefer-exponentiation-operator	It perceives private identifier and doesn't confused with that. This upgrades eslint-utils.
803b06a	quotes	It perceives class fields.
7b89143	semi-spacing	It perceives ; of class fields.
b50db18	semi-style	It perceives ; of class fields.
560e0cf	semi	It perceives ; of class fields. Some cases are affected by beforeStatementContinuationChars option.
04e1ace	space-infix-ops	It perceives = of class field initializers.

And this PR adds some tests to a ton of rules.

Is there anything you'd like reviewers to focus on?

• Please check if there are more rules that need to be fixed for class fields.
• More tests?
• Are there wrong fixes by misunderstandings of the spec?
• There are rules that generate new errors for class fields. Is that good?
• A large refactoring exists for camelcase, id-denylist, id-length, and id-match. Is this good or not?

[eslint-github-bot]

Hi @mysticatea!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

[protoEvangelion]

Keep up the good work @mysticatea very excited for this 🙌

[mdjermanovic]

@mysticatea do we also plan to update code path analysis, or shall we leave it as is?

For example:

a = 1;

class A {
  foo = a++;
}

b = a;

a++ is in the same code path (and same code segment) as a = 1 and b = a, but it isn't really executed between them.

Non-static PropertyDefinition (or at least PropertyDefinition#value) is logically a separate code path?

[mdjermanovic]

Rules that use scope1.variableScope === scope2.variableScope logic:

/* eslint no-use-before-define: ["error", { "variables": false }] */

class A {
    foo = x; // false positive?
}

const x = 1;

/* eslint require-atomic-updates: error */

async () => {
    let i;

    foo(class { x = i++ });

    i = i + await bar(); // false negative?
}

Less sure about this one, since formally it isn't a function so maybe we could add an option later:

/* eslint no-loop-func: error */

for (var i = 0; i < 10; i++) {
    foo[i] = class {
        x = i; // false negative?
    }
}

[mdjermanovic]

semi false positive:

/* eslint semi: ["error", "never"] */

class A {
    a = b; // syntax error without `;`
    *c(){}
}

[mdjermanovic]

dot-notation false positive:

/* eslint dot-notation: ["error", { allowKeywords: false }] */

class C {
    #in;
    foo() {
        this.#in; // autofixed to `this["in"]`
    }
}

[mdjermanovic]

Should we put this behind an option, since this isn't a class method by definition?

[mdjermanovic]

I think PropertyDefinition should be behind an option (or a separate rule?), the docs for this rule says that it checks variable declarations.

[mdjermanovic]

We should add something like || node.type === "PropertyDefinition" on line 64:

/* eslint no-useless-computed-key: ["error", { enforceForClassMembers: true }] */

class C {
    ["constructor"]; // false positive
    static ["prototype"]; // false positive
    ["__proto__"]; // false negative
}

(also, lodash has been removed in the meantime)

[mdjermanovic]

static ["constructor"] is also false positive (produces parsing error instead of a runtime error in a maybe-unused code).

For some reason, "constructor" is allowed as the name of a static method, but not as the name of a static field.

ClassElement : static MethodDefinition

• It is a Syntax Error if PropName of MethodDefinition is "prototype".

ClassElement : static FieldDefinition ;

• It is a Syntax Error if PropName of FieldDefinition is "prototype" or "constructor".

[mdjermanovic]

In class-methods-use-this, "exceptMethods": ["a"] allows both a and #a:

/* eslint class-methods-use-this: ["error", { exceptMethods: ["a"] }] */

class C {
    a() {} // ok
    #a() {} // ok
}

Does it make sense, or should #a() be allowed only by "#a"?

[mdjermanovic]

no-extra-parens:

/* eslint no-extra-parens: ["error"] */

class C {
  [(a)]; // false negative
  b = (c + d); // false negative
}

[mdjermanovic]

no-dupe-class-members reports instance field and prototype method with the same name as duplicates, but that's a bit different from all other combinations where one definition basically overwrites the other. In this case, both are applied, and someone could write something like this:

class C {
    a = this.a.bind(this);
    a() {
        // uses `this`
    }
}

Thoughts?

[mysticatea]

I'm sorry for my inactive while a time. I'm rebooting...

[mysticatea]

@mysticatea do we also plan to update code path analysis, or shall we leave it as is?

For example:

a = 1;

class A {
  foo = a++;
}

b = a;

a++ is in the same code path (and same code segment) as a = 1 and b = a, but it isn't really executed between them.

Non-static PropertyDefinition (or at least PropertyDefinition#value) is logically a separate code path?

Oh. I had forgotten about code path analysis.
Yes, by the spec, class field initializers are implicit functions. The initializers define implicit functions and it's called from class instance instantiation process (by the way, the implicit functions define arguments local variable, but the use of arguments in class field initializers is a syntax error). Therefore, it should separate code paths and create scope.

[mdjermanovic]

A remaining task is to update code path analysis to create separate code paths for class field initializers (#14591 (comment) and #14591 (comment)). This is a change in the core that affects rules (built-in and custom), so it might be good to implement it before beta release?

Aside from that, I left comments for about 15-20 rules. Some are obvious bugs, some need discussion. I can open issues after we merge this PR, so that they could be worked on in parallel.

[nzakas]

I think code path analysis may not make it. @mysticatea is the only one who really understands it. I can take a swing at it but unsure how far I can actually get.

Yes, let’s deal with other rule changes separately. There is always a long tail of rule changes to support new syntax, and that’s part of what the beta period is for.