Add "variables" option to no-use-before-define rule

[feross]

• ESLint Version: 3.5.0

I want no-use-before-define to check for classes or functions that are used before they are defined. But I do not want to check for variables that are used before they are defined.

Right now there exist two options, which modify whether functions and classes are checked. For example, I can disable these two checks:

"no-use-before-define": [2, { "functions": false, "classes": false }]

But I cannot disable checking if variables are used before they are defined. I propose adding a "variables" option to address this:

"no-use-before-define": [2, { "variables": false }]

There is already a precedent for an option like this, in the "blocks" option to the padded-blocks rule. It's used like this:

"padded-blocks": [2, { "blocks": "never", "switches": "never", "classes": "never" }]

[feross]

To make this clearer, here are some examples.

Examples of incorrect code for the { "variables": false } option:

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

console.log(foo);
var foo = 5;

Examples of correct code for the { "variables": false } option:

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

function printFoo() {
    console.log(foo);
}
var foo = 5;

The way this works would be 100% analogous to the "classes" option, but for variables. The existing "classes" option is documented here: https://github.com/eslint/eslint/blob/master/docs/rules/no-use-before-define.md#classes

[platinumazure]

Thanks for filling out the template and adding examples, we really appreciate it.

I'm not opposed, but I'd love to see a real-world case where this is a problem (especially if it's something that "standard" recommends but cannot enforce, or wants to recommend but cannot, due to this limitation).

That said, since we already have classes and function exceptions, it does seem like we should just add a variable option. So one could make an argument that the rule is incomplete without this change.

Let's see what the rest of the team thinks.

[feross]

I'm not opposed, but I'd love to see a real-world case where this is a problem (especially if it's something that "standard" recommends but cannot enforce, or wants to recommend but cannot, due to this limitation).

In standard, we would like to prevent clearly incorrect use-before-define cases like the following:

console.log(foo);
var foo = 5;

While still allowing cases that may be correct if the program executes in the right order:

function printFoo() {
     // Variable "foo" is not used before it is defined, even though it
     // appears in the source before foo is defined.
    console.log(foo);
}
var foo = 5;
printFoo();

There are just too many false positives for us when the latter example is disallowed.

[feross]

More specifically, here is the concrete code example that prompted this issue. We would like to disallow the following:

var variable1 = variable1
let variable2 = variable2
const variable3 = variable3

[not-an-aardvark]

This seems like a reasonable enhancement. I'll champion this proposal.

@eslint/eslint-team Any support for this proposal? I think this comment explains the use-case well. We already have a classes option that does the same thing for classes.

[platinumazure]

Endorsed. Apologies to @feross for losing track of this and thank you @not-an-aardvark for picking this back up.

[feross]

No apology needed, @platinumazure! Thanks for the work both of you do on ESLint!

[not-an-aardvark]

@eslint/eslint-team Any support for this proposal? We need 2 more 👍s to accept it. This would allow users to avoid temporal-deadzone variable references without having to adhere to the stylistic requirements of no-use-before-define.

[dcousens]

@not-an-aardvark I added a +1.

[dcousens]

ping @fanatid @jprichardson @afk11 @Flet @timoxley @yoshuawuyts @LinusU @pinn3 @kilianc

[not-an-aardvark]

@dcousens Thanks, I appreciate it. However, I was mainly referring to 👍s from the ESLint team (from our maintainer guide, we accept enhancements after they have a 👍 from three team members).

[dcousens]

@not-an-aardvark ah, sorry for the call out then everyone.

[fanatid]

@feross as I understand next code with { "variables": false } will be incorrect?

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

x()
function x () { console.log(42) }

[not-an-aardvark]

@fanatid That's correct (the code there is already incorrect with this rule, and the variables option would only affect variable declarations, not function declarations).

[feross]

@not-an-aardvark Are you sure? @fanatid's example is a function so it shouldn't not affected by { "variables": false }. Also, with { "functions": false } the code is indeed valid.

This is not an error:

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

x()
function x () { console.log(42) }

[not-an-aardvark]

@feross I'm not sure I understood what you're saying, but I think we're in agreement.

This should report an error:

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

x()
function x () { console.log(42) }

This should not report an error:

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

x()
function x () { console.log(42) }

[feross]

Yes, sorry for the confusing wording. We're in agreement.

[Jessidhia]

This rule, as it is, without a variables: false option, also has false positives with binding exports:

export { group as default }
const group = new Set()

Note that if it was export default group, the error would be correct; but export { group } (or export { group as default } should not error.

[not-an-aardvark]

@Kovensky, may you please create another issue for that? If there is a false positive with the rule then we certainly want to address it, but it seems unrelated to this proposal. Thanks!

[Jessidhia]

@not-an-aardvark hm, true, it is kinda related but not exactly.

At any rate, have there been any updates w.r.t. deciding on whether to implement the potential variables option in code? It'd be nice to have a static TDZ checker without enforcing style changes. I could try to write a PR if the proposal is accepted.

Just noticed this after intentionally putting a let in TDZ for most of a file to make its initialization point clearer...

[Jessidhia]

Opened #7858

[feross]

What's the status of this issue? Are we still waiting for 👍's from additional ESLint team members? Thanks.

[vitorbal]

I'll support this. @eslint/eslint-team looks like all we need is one more +1 to accept this issue (and we already have a champion). Any takers?

[mikesherov]

👍

[not-an-aardvark]

PR up at #7948