Make prefer-for-of block scope aware
#1926
Conversation
| } | ||
|
|
||
| public createBlockScope() { | ||
| // a map of incrementors and whether or not they are only used to index into an array reference in the for loop |
There was a problem hiding this comment.
this comment should go on the IncrementorMap definition
| private incrementorMap: { [name: string]: IIncrementorState }; | ||
|
|
||
| type IncrementorMap = { [name: string]: IIncrementorState }; | ||
| class PreferForOfWalker extends Lint.BlockScopeAwareRuleWalker<{}, IncrementorMap> { | ||
| constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { |
There was a problem hiding this comment.
you can omit this constructor now, it's not doing anything
| private incrementorMap: { [name: string]: IIncrementorState }; | ||
|
|
||
| type IncrementorMap = { [name: string]: IIncrementorState }; | ||
| class PreferForOfWalker extends Lint.BlockScopeAwareRuleWalker<{}, IncrementorMap> { |
| return this.blockScopeStack[i]; | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
don't return null explicitly; return undefined instead
| @@ -58,6 +58,16 @@ export abstract class BlockScopeAwareRuleWalker<T, U> extends ScopeAwareRuleWalk | |||
| return; | |||
| } | |||
|
|
|||
| public findBlockScope(predicate: (scope: U) => boolean) { | |||
There was a problem hiding this comment.
why do we need this? shouldn't it just be this.getAllBlockScopes().find(predicate)?
There was a problem hiding this comment.
this goes in reverse order from local to global
There was a problem hiding this comment.
Then you could call reverse() on the array; I think that'd be more readable
There was a problem hiding this comment.
that creates a copy of the array. I think I made this change based on a discussion about not unnecessarily copying arrays.
There was a problem hiding this comment.
alright, that sounds reasonable. in the future, please leave a code comment
|
@adidahiya I'm pretty sure this fixes the issue. The only way the call stack could have occurred is if the same iterator name was reused (added twice, then deleted when exiting the "scope"). The choice seems to be to close the ticket with a likely fix (it can be reopened if it's not fixed), or leaving the ticket permanently open if the submitter doesn't update the ticket. |
# Conflicts: # src/rules/preferForOfRule.ts
fixes #1884