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

no-use-before-define should have an option for ES6 classes #3944

Closed
lucsky opened this Issue Sep 25, 2015 · 15 comments

Comments

Projects
None yet
6 participants
@lucsky
Copy link

lucsky commented Sep 25, 2015

The "no-use-before-define" rule has a "nofunc" option which allows to specify that using a function before it has been declared is ok. It should be possible (with a "noclass" option for example) to also do that with ES6 classes where it would allow a class name to be used before the class is declared later in the file.

Ex, this triggers the "Bar was used before it was defined" error:

class Foo {
    constructor() {
        this.bar = new Bar();
    }
}

class Bar {}
@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Sep 25, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Sep 25, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

gyandeeps commented Sep 25, 2015

Almost Related technically: #3941

@gyandeeps gyandeeps closed this Sep 25, 2015

@gyandeeps gyandeeps reopened this Sep 25, 2015

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Sep 25, 2015

I guess this new option should apply to anything that hoists with TDZ. Sounds good, though definitely dangerous.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Sep 25, 2015

Won't that code cause a runtime error?

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Sep 25, 2015

No, TDZ errors occur when the reference is evaluated before the declaration. So there's no way that this program would have a problem, regardless of what follows it.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Sep 25, 2015

Oh right, because the reference doesn't occur until new is called. Got it.

@nzakas nzakas added enhancement rule accepted and removed triage labels Sep 25, 2015

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Sep 25, 2015

I think "noclass" is confusable.
The following pattern is a runtime error different from functions.

let a = new A();
class A { }

So I favor of "allow-outer-scope" or something like.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Sep 26, 2015

Good point, though I don't think allow-outer-scope is clear enough. Is it even possible to accurately flag one but not the other in the error?

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Nov 28, 2015

I'm sorry.

Currently, no-use-before-define has an string option "nofunc".
We will add new option, so I think a use of an object is fine.

My proposal:

{
    "no-use-before-define": [2, {"functions": true, "outerClasses": true}]
}
  • "functions" (boolean) - If this option is false, FunctionDeclarations are allowed to use before declarations. Default is true.
  • "outerClasses" (boolean) - If this option is false, ClassDeclarations are allowed to use before declarations only if the declaration is in the scope of an outer function (or global). Default is true.
module.exports.schema = [
    {
        "oneOf": [
            {
                "enum": ["nofunc"]
            },
            {
                "type": "object",
                "properties": {
                    "functions": {"type": "boolean"},
                    "outerClasses": {"type": "boolean"}
                },
                "additionalProperties": false
            }
        ]
    }
];
@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 28, 2015

Why outer classes only?

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Nov 28, 2015

@nzakas Class declarations are not hoisted, so the following code raises a runtime error.

let a = new A();
class A { }

The following code is OK.

module.exports = function foo() {
    let a = new A();
};

class A { }
@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 30, 2015

Yes, that part I understand, I just don't understand the term "outer classes". What are you calling an "outer class"?

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Dec 1, 2015

I imaged "class declarations in upper function scopes or global scopes", and I saw a Ja-En dictionary.
I want to get advice.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 1, 2015

I think we can just use "classes" and have it work the way you describe. We just need to explain in the documentation.

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Dec 2, 2015

OK.
I will work on this.

@mysticatea mysticatea self-assigned this Dec 2, 2015

mysticatea added a commit to mysticatea/eslint that referenced this issue Dec 2, 2015

mysticatea added a commit to mysticatea/eslint that referenced this issue Dec 3, 2015

@nzakas nzakas closed this in #4597 Dec 8, 2015

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.