prefer-const object destructuring false positive

[dwelle]

Tell us about your environment

• ESLint Version: 4.4.0

What parser (default, Babel-ESLint, etc.) are you using? Babel-ESLint

Please show your full configuration:

/* eslint prefer-const: [ "warn", { "destructuring": "all", "ignoreReadBeforeAssign": true } ] */

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

/* eslint prefer-const: [ "warn", { "destructuring": "all", "ignoreReadBeforeAssign": true } ] */
/* global someFunc */
(function ({ a }) {
    let b;
    ({ a, b } = someFunc({ a }));
    return { a, b };
})()

What did you expect to happen?

Should pass since neither let nor const can be specified at the start of the variable assignment statement.

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

5:11 - 'b' is never reassigned. Use 'const' instead. (prefer-const)

[not-an-aardvark]

Thanks for the report. I can reproduce this issue.

[Jamesernator]

It also happens in array destructuring e.g.:

// This isn't global where it's used but the error still happens
let currentScanner = this
const result = []
for (let i = 0 ; i < value ; i++) {
    let char
    ;[currentScanner, char] = currentScanner.consumeOne()
    result.push(char)
}

[Orlandster]

I will make it in the next few days.

[Orlandster]

@not-an-aardvark I'm just going into to it, but there is something I'm curious about. Is the case described above really a reassignment?

Or is it just a reassignment if I explicit assign a value to b:

/* global someFunc */
(function ({ a }) {
  let b = 12;
  ({ a, b } = someFunc({ a }));
  return { a, b };
})()

Because the case below throws the same message:

function foo() {
  let b;
  b = 10;
}

So this is probably also a bug right?

[dwelle]

Strictly speaking, I'd say that in both cases, it is a reassignment (in 2nd case, the let b implicitly assigns undefined, but for the purpose of eslint, it's a good heuristic to say it's not been assigned anything).

Either way, the first case (I reported) is definitely explicit re-assignment.

But.. the nomenclature isn't really important here. The case I reported is a bug because a user wouldn't be able to use destructuring without setting eslint-disable-line

[not-an-aardvark]

Is the case described above really a reassignment?

Which case? I don't understand the question.

This is a reassignment:

let b = 10;
b = 20;

This is not a reassignment, since b could just be declared in a different place:

let b;
b = 20;

So this is probably also a bug right?

I'm not sure what case you're referring to. Could you clarify:

• What code does this issue happen with?
• What are you expecting the rule to do for that code?
• What does the rule actually do?

[jrpool]

I was thinking of working on this, but I am happy to yield to @Orlandster1998. I have a question about this issue. It is labeled as a rule-bug issue, but does it not imply that there is also a test-coverage issue? If I were working on this, I would guess that the proper course of action is (1) to write a new test that fails at present, (2) change the rule to correct the bug, (3) show that all the tests now pass. Is that incorrect?

[platinumazure]

@jrpool Yes, you got it. We always ask for tests in pull requests (exception: when code is being refactored and behavior isn't expected to change), so it's also fair to note that a rule bug usually means we missed a case in testing.

[ericuldall]

Any update on this? I'm currently affected by...

  const organizationId = organization.get('_id');
  ctx.body = { success, organizationId };

I'm happy to work on a PR if this has fallen through the cracks.

[jrpool]

@Orlandster1998 said he was going to do it. Is that happening?

[ericuldall]

Actually... just realized my issue was due to a typo. x-/

[platinumazure]

@Orlandster1998 Friendly ping: Are you still working on this? (If not, totally okay, just let us know if you intend to finish this or if someone else should start looking at this.) Thanks!