Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: report class evaluation TDZ errors in no-use-before-define #15134

Merged
merged 1 commit into from Nov 5, 2021

Conversation

mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Oct 4, 2021

Prerequisites checklist

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

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

This bug fix produces only more warnings.

Tell us about your environment

  • ESLint Version: v8.0.0-rc.0
  • Node Version: v12.22.4
  • npm Version: v6.14.14

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?

default

Please show your full configuration:

Configuration
module.exports = {
    parserOptions: {
        ecmaVersion: 2022
    }
};

What did you do? Please include the actual source code causing the issue.

/* eslint no-use-before-define: error */

class A extends A {} // TDZ error

class B {
    [B]() {} // TDZ error
}

const C = class {
    static foo = C; // TDZ error
}
/* eslint no-use-before-define: ["error", { variables: false }] */

class D {
    static foo = x; // TDZ error
}

const x = 1;

What did you expect to happen?

no-use-before-define to report all references marked as "TDZ error" in the above examples.

What actually happened? Please include the actual, raw output from ESLint.

No errors.

What changes did you make? (Give an overview)

Fixed the no-use-before-define rule for ClassDefinitionEvaluation steps. Class heritage and computed properties are evaluated before initializing the class binding (A and B create classBinding in the class scope; they also create an outer variable but the references from the class definition are to the inner one, and the outer one is initialized even later). Static initializers are run after the class binding is initialized (if it exists), but still before initializing C in const C = class ... and before declarations that appear after the class (const x).

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

Examples that don't require new syntax are reproducible as false negatives in v7.32.0, too (demo link). This is a bug fix regarding extends and computed properties, and a sort of enhancement regarding static field initializers since they're implicit functions. This rule doesn't track function calls, but in this case we can know when they will be called.

@mdjermanovic mdjermanovic added bug ESLint is working incorrectly enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Oct 4, 2021
@mdjermanovic mdjermanovic added this to Feedback Needed in Triage Oct 19, 2021
@mdjermanovic mdjermanovic changed the title Update: report class evaluation TDZ errors in no-use-before-define feat: report class evaluation TDZ errors in no-use-before-define Oct 26, 2021
@eslint-github-bot
Copy link

eslint-github-bot bot commented Oct 26, 2021

Hi @mdjermanovic!, thanks for the Pull Request

The first commit message 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 commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
  • There should be a space following the initial tag and colon, for example 'feat: Message'.
  • The first letter of the tag should be in lowercase

Read more about contributing to ESLint here

@mdjermanovic mdjermanovic force-pushed the nousebeforedefine-classes-tdz branch from 3b8fda5 to b4d7f73 Compare Oct 26, 2021
nzakas
nzakas approved these changes Nov 5, 2021
Copy link
Member

@nzakas nzakas left a comment

This melted my brain but LGTM

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Nov 5, 2021
@mdjermanovic mdjermanovic merged commit c9fefd2 into main Nov 5, 2021
14 checks passed
Triage automation moved this from Feedback Needed to Complete Nov 5, 2021
@mdjermanovic mdjermanovic deleted the nousebeforedefine-classes-tdz branch Nov 5, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators May 5, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
Triage
Complete
Development

Successfully merging this pull request may close these issues.

None yet

2 participants