prefer-const: for-of loop with tuple assignment

[WilcoBakker]

Bug Report

• TSLint version: 5.3.2
• TypeScript version: 2.3.3
• Running TSLint via: Atom

TypeScript code being linted

for (let [key, value] of dataMap) {
	value = "Something else";
}

with tslint.json configuration:

{
  "extends": "tslint:recommended",
  "rules": {
    "quotemark": "double",
    "no-console": false,
    "jsdoc-format": false,
    "interface-name": false,
    "object-literal-sort-keys": false,
    "no-bitwise": false
  }
}

Actual behavior

It claims that the variable key is never reassigned even if value is.

Expected behavior

In some cases I need to overwrite one of the tuple variables so I must use let in such case.
So I expect tslint to not complain about some tuple variables not being reassigned if at least 2 variables are used within the tuple and at least one variable is being reassigned.

[ajafff]

https://palantir.github.io/tslint/rules/prefer-const/
{"destructuring": "all"} does that

[WilcoBakker]

Then why is it set to any by default? I don't see how in this case that makes sense.

[adidahiya]

any is a stricter option, that's why it's the default. usually we want to recommend code like this:

// bad
let { foo, bar } = this.props;

// good
const { foo } = this.props;
let { bar } = this.props;

I agree that in the case of a for-of loop, this kind of construct isn't really possible. So I suppose the rule could be more lenient here... but i'm not sure it's really worth the extra implementation complexity with this edge case. you can easily refactor your code to not mutate the value var.

I'm inclined to say this is "by design", but I'll leave this open for more feedback

[WilcoBakker]

@adidahiya So basically. what you suggest is copying the values I really do need to mutate?
I don't see how that would be desired, I would really appreciate a fix for this but if you claim it is not worth it to fix because not many others would face this problem, I would respect your point of view.

My counter argument however would be that if you look at the for of page of mozilla's developers guide (https://developer.mozilla.org/nl/docs/Web/JavaScript/Reference/Statements/for...of), they really do use the same example for iterating through a Map object. (Of course without altering any variable)
So it may not be as edgy as you claim but yet again, when is something really edgy? In this case that is not up to me to decide.

[adidahiya]

Well, yeah, they use the same syntax in the MDN example, but it's a super simple contrived example that you wouldn't use in a production app that has linting enabled:

for (let [key, value] of iterable) {
  console.log(value);
}

Obviously the lint rule should complain about the let here.

[ajafff]

It wouldn't be that hard to implement. I think it makes sense to handle destructuring in for-of-loop initializers in a more reasonable way.

In fact we already have special handling for VariableDeclarationLists inside ForStatements when one variable is reassigned.
For example:

for (let i = 0, len = arr.length; i < len; ++i) {} // outside of for-loops that would be a lint error

[WilcoBakker]

@adidahiya Yeah, indeed it should complain there, that's why I mentioned it didn't alter any variable.

[adidahiya]

ah, good point about the ForStatement special case.