Clarify doc and examples for enforceForRenamedProperties option of prefer-destructuring rule

[pvdlg]

Environment

• **ESLint Version: 4.17.0
• **Node Version: 9.5.0
• **npm Version: 5.6.0

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

default one

Please show your full configuration:

Configuration

{
  "rules": {
     "prefer-destructuring": ["error", {array: true, object: true}, {enforceForRenamedProperties: true}]
  }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

const array = ['a', 'b', 'c'];
const someIndex = 1;
const value = array[someIndex];

eslint **/*.js

What did you expect to happen?
To not return an error for the prefer-destructuring rules.

What actually happened? Please include the actual, raw output from ESLint.
ESLint return the error Use object destructuring. for the line const value = array[someIndex];.

If enforceForRenamedProperties is set to false, the error is not returned which is expected.

[not-an-aardvark]

Hi, thanks for the report. However, this is working as intended -- to fix the issue you should write the code like this:

const { [someIndex]: value } = array.

[pvdlg]

Thanks for the quick answer!
As this is a bit of convoluted way to use destructuring maybe it would be a good idea to add it to the example in the "correct" section of the rule documentation. Maybe adding to the existing example something like that:

Examples of correct code when enforceForRenamedProperties is enabled:

/* eslint prefer-destructuring: ["error", {array: true}, {enforceForRenamedProperties: true}] */

[bar] = array;

const { [someIndex]: value } = array

[not-an-aardvark]

Improving the documentation sounds like a good idea to me. Would you be willing to make a PR?

[pvdlg]

Yeah sure! Can you assign the issue to me?

[santanaG]

This issue should have been closed along wit #9989. Can it be done manually?

[platinumazure]

@santanaG We could close this manually, but should we? I'm not clear on if there is still an issue here (i.e., if there is something to be resolved in a different way than what #9989 tried to do).

[santanaG]

@platinumazure facepalm Apologies, I just realized that it actually closed and the changes were not accepted!

[OliverSieweke]

I was confused by this rule as well. I had a read through the discussion in #9989 and had a look at the source code.

It seems to me that the confusion mostly comes from the fact that the rule and the documentation use the terms "array" and "object", while what the code is actually checking is merely whether the property being accessed is a number or not (in the code: function isArrayIndexAccess(node) {return Number.isInteger(node.property.value);}). Hence the following counter-intuitive situations:

• Accessing an array element through a computed index will fall under the category "object".
• Accessing an object property whose key is an integer will fall under the category "array".

I'm happy to submit a PR to propose 2 sentences that could add some clarification to the documentation.

Ultimately it seems impossible to fully realize the intent of this rule through mere static code analysis...