Fix: no-unused-vars false positive in loop (fixes #6646)

[mysticatea]

Fixes #6646.

no-unused-vars comes to mark (as used) self-modification inside of a loop because the reference inside of a loop can be used in the next iteration.

[mention-bot]

@mysticatea, thanks for your PR! By analyzing the annotation information on this pull request, we identified @vitorbal, @nzakas and @markelog to be potential reviewers

[eslintbot]

LGTM

[markelog]

@private is missing

[mysticatea]

Oh, thank you!

[eslintbot]

LGTM

[markelog]

Seems this kinda thing is needed among the rules, like -

eslint/lib/rules/indent.js

Line 616 in b837c92

"IfStatement", "WhileStatement", "ForStatement", "ForInStatement", "ForOfStatement", "DoWhileStatement", "ClassDeclaration"

eslint/lib/rules/no-unmodified-loop-condition.js

Line 22 in b837c92

var LOOP_PATTERN = /^(?:DoWhile|For|While)Statement$/;

and so on

Can we move it in one place?

[mysticatea]

Right.

maybe isLoop(node) and isFunction(node) of lib/ast-utils.js.

[markelog]

Could work, but probably more appropriate to do this in the separate pr

[eslintbot]

LGTM

[markelog]

Probably worth to add tests for such methods?

[mysticatea]

oops, I will add tomorrow (JST)

2016年7月11日(月) 22:05 Oleg Gaidarenko notifications@github.com:

In lib/ast-utils.js
#6649 (comment):

• /**

• \* Checks whether a given node is a loop node or not.
  

• \* The following types are loop nodes:
  

• *
  

• \* - DoWhileStatement
  

• \* - ForInStatement
  

• \* - ForOfStatement
  

• \* - ForStatement
  

• \* - WhileStatement
  

• *
  

• \* @param {ASTNode|null} node - A node to check.
  

• \* @returns {boolean} `true` if the node is a loop node.
  

• */
  

• isLoopNode: function(node) {

•    return Boolean(node && anyLoopPattern.test(node.type));
  

Probably worth to add tests for such methods?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/eslint/eslint/pull/6649/files/534b4a63da91a7ee7bb3bced229e269c1a447558..9df950521809d8b4aa8042852fc10edee155554b#r70252919,
or mute the thread
https://github.com/notifications/unsubscribe/AB2Rz2_t-vQyN44LhCdM7boewfLk09C7ks5qUj-2gaJpZM4JJSFj
.

[mysticatea]

I added it.

[nzakas]

Maybe just isLoop? (Not sure "node" is necessary)

[eslintbot]

LGTM

[nzakas]

Maybe just isFunction?

[eslintbot]

LGTM

[mysticatea]

I renamed those 📝 :

• isFunctionNode → isFunction
• isLoopNode → isLoop

[markelog]

I would do this in two steps - one for #6646 and other for ast-utils module, but if this is acceptable practise then LGTM

[ilyavolodin]

@markelog In this case, since those function are used in this rule, I think it's perfectly fine to do both in a single PR.

LGTM.