handle forward referenced in prefer-const rule (#1908)

src/language/utils.ts

@@ -144,6 +144,24 @@ export function isNodeFlagSet(node: ts.Node, flagToCheck: ts.NodeFlags): boolean
     /* tslint:enable:no-bitwise */
 }
 
+/**
+ * Bitwise check for combined node flags.
+ */
+export function isCombinedNodeFlagSet(node: ts.Node, flagToCheck: ts.NodeFlags): boolean {
+    /* tslint:disable:no-bitwise */
+    return (ts.getCombinedNodeFlags(node) & flagToCheck) !== 0;
+    /* tslint:enable:no-bitwise */
+}
+
+/**
+ * Bitwise check for combined modifier flags.
+ */
+export function isCombinedModifierFlagSet(node: ts.Node, flagToCheck: ts.ModifierFlags): boolean {
+    /* tslint:disable:no-bitwise */
+    return (ts.getCombinedModifierFlags(node) & flagToCheck) !== 0;
+    /* tslint:enable:no-bitwise */
+}
+
 /**
  * Bitwise check for type flags.
  */

src/language/walker/blockScopeAwareRuleWalker.ts

@@ -30,10 +30,10 @@ export abstract class BlockScopeAwareRuleWalker<T, U> extends ScopeAwareRuleWalk
         super(sourceFile, options);
 
         // initialize with global scope if file is not a module
-        this.blockScopeStack = this.fileIsModule ? [] : [this.createBlockScope()];
+        this.blockScopeStack = this.fileIsModule ? [] : [this.createBlockScope(sourceFile)];
     }
 
-    public abstract createBlockScope(): U;
+    public abstract createBlockScope(node: ts.Node): U;
 
     // get all block scopes available at this depth
     public getAllBlockScopes(): U[] {
@@ -62,7 +62,7 @@ export abstract class BlockScopeAwareRuleWalker<T, U> extends ScopeAwareRuleWalk
         const isNewBlockScope = this.isBlockScopeBoundary(node);
 
         if (isNewBlockScope) {
-            this.blockScopeStack.push(this.createBlockScope());
+            this.blockScopeStack.push(this.createBlockScope(node));
             this.onBlockScopeStart();
         }
 

src/rules/preferConstRule.ts

@@ -17,7 +17,7 @@
 
 import * as ts from "typescript";
 import * as Lint from "../index";
-import {isAssignment, isNodeFlagSet, unwrapParentheses} from "../language/utils";
+import {isAssignment, isCombinedModifierFlagSet, isCombinedNodeFlagSet, unwrapParentheses} from "../language/utils";
 
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
@@ -45,12 +45,70 @@ export class Rule extends Lint.Rules.AbstractRule {
 }
 
 class PreferConstWalker extends Lint.BlockScopeAwareRuleWalker<{}, ScopeInfo> {
+    private static collect(statements: ts.Statement[], scopeInfo: ScopeInfo) {
+        for (const s of statements) {
+            if (s.kind === ts.SyntaxKind.VariableStatement) {
+                PreferConstWalker.collectInVariableDeclarationList((s as ts.VariableStatement).declarationList, scopeInfo);
+            }
+        }
+    }
+
+    private static collectInVariableDeclarationList(node: ts.VariableDeclarationList, scopeInfo: ScopeInfo) {
+        if (isCombinedNodeFlagSet(node, ts.NodeFlags.Let) && !isCombinedModifierFlagSet(node, ts.ModifierFlags.Export)) {
+            for (const decl of node.declarations) {
+                PreferConstWalker.addDeclarationName(decl.name, node, scopeInfo);
+            }
+        }
+    }
+
+    private static addDeclarationName(node: ts.BindingName, container: ts.VariableDeclarationList, scopeInfo: ScopeInfo) {
+        if (node.kind === ts.SyntaxKind.Identifier) {
+            scopeInfo.addVariable(node as ts.Identifier, container);
+        } else {
+            for (const el of (node as ts.BindingPattern).elements) {
+                if (el.kind === ts.SyntaxKind.BindingElement) {
+                    PreferConstWalker.addDeclarationName(el.name, container, scopeInfo);
+                }
+            }
+        }
+    }
+
     public createScope() {
         return {};
     }
 
-    public createBlockScope() {
-        return new ScopeInfo();
+    public createBlockScope(node: ts.Node) {
+        const scopeInfo = new ScopeInfo();
+        switch (node.kind) {
+            case ts.SyntaxKind.SourceFile:
+                PreferConstWalker.collect((node as ts.SourceFile).statements, scopeInfo);
+                break;
+            case ts.SyntaxKind.Block:
+                PreferConstWalker.collect((node as ts.Block).statements, scopeInfo);
+                break;
+            case ts.SyntaxKind.ModuleDeclaration:
+                const body = (node as ts.ModuleDeclaration).body;
+                if (body && body.kind === ts.SyntaxKind.ModuleBlock) {
+                    PreferConstWalker.collect((body as ts.ModuleBlock).statements, scopeInfo);
+                }
+                break;
+            case ts.SyntaxKind.ForStatement:
+            case ts.SyntaxKind.ForOfStatement:
+            case ts.SyntaxKind.ForInStatement:
+                const initializer = (node as ts.ForInStatement | ts.ForOfStatement | ts.ForStatement).initializer;
+                if (initializer && initializer.kind === ts.SyntaxKind.VariableDeclarationList) {
+                    PreferConstWalker.collectInVariableDeclarationList(initializer as ts.VariableDeclarationList, scopeInfo);
+                }
+                break;
+            case ts.SyntaxKind.SwitchStatement:
+                for (const caseClause of (node as ts.SwitchStatement).caseBlock.clauses) {
+                    PreferConstWalker.collect(caseClause.statements, scopeInfo);
+                }
+                break;
+            default:
+                break;
+        }
+        return scopeInfo;
     }
 
     public onBlockScopeEnd() {
@@ -84,25 +142,6 @@ class PreferConstWalker extends Lint.BlockScopeAwareRuleWalker<{}, ScopeInfo> {
         super.visitPostfixUnaryExpression(node);
     }
 
-    protected visitVariableDeclaration(node: ts.VariableDeclaration) {
-        this.getCurrentBlockScope().currentVariableDeclaration = node;
-        super.visitVariableDeclaration(node);
-        this.getCurrentBlockScope().currentVariableDeclaration = null;
-    }
-
-    protected visitIdentifier(node: ts.Identifier) {
-        if (this.getCurrentBlockScope().currentVariableDeclaration != null) {
-            const declarationList = this.getCurrentBlockScope().currentVariableDeclaration.parent;
-            if (isNodeFlagSet(declarationList, ts.NodeFlags.Let)
-                && !Lint.hasModifier(declarationList.parent.modifiers, ts.SyntaxKind.ExportKeyword)) {
-                if (this.isVariableDeclaration(node)) {
-                    this.getCurrentBlockScope().addVariable(node, declarationList);
-                }
-            }
-        }
-        super.visitIdentifier(node);
-    }
-
     private handleLHSExpression(node: ts.Expression) {
         node = unwrapParentheses(node);
         if (node.kind === ts.SyntaxKind.Identifier) {
@@ -138,20 +177,6 @@ class PreferConstWalker extends Lint.BlockScopeAwareRuleWalker<{}, ScopeInfo> {
         }
     }
 
-    private isVariableDeclaration(node: ts.Identifier) {
-        if (this.getCurrentBlockScope().currentVariableDeclaration != null) {
-            // `isBindingElementDeclaration` differentiates between non-variable binding elements and variable binding elements
-            // for example in `let {a: {b}} = {a: {b: 1}}`, `a` is a non-variable and the 1st `b` is a variable
-            const isBindingElementDeclaration = node.parent.kind === ts.SyntaxKind.BindingElement
-                && node.parent.getText() === node.getText();
-            const isSimpleVariableDeclaration = node.parent.kind === ts.SyntaxKind.VariableDeclaration;
-            // differentiates between the left and right hand side of a declaration
-            const inVariableDeclaration = this.getCurrentBlockScope().currentVariableDeclaration.name.getEnd() >= node.getEnd();
-            return inVariableDeclaration && (isBindingElementDeclaration || isSimpleVariableDeclaration);
-        }
-        return false;
-    }
-
     private markAssignment(identifier: ts.Identifier) {
         const allBlockScopes = this.getAllBlockScopes();
         // look through block scopes from local -> global
@@ -171,8 +196,6 @@ interface IConstCandidate {
 }
 
 class ScopeInfo {
-    public currentVariableDeclaration: ts.VariableDeclaration;
-
     private identifierUsages: {
         [varName: string]: {
             letStatement: ts.VariableDeclarationList,

test/rules/prefer-const/test.ts.fix

@@ -122,3 +122,23 @@ const arr = [];
 arr.push(0);
 arr[1] = 1;
 arr.length = 1;
+
+// reassignment of the forward declaration
+function useOfForwardDecl() {
+    forwardDecl = 1;
+}
+
+let forwardDecl: number;
+
+module E {
+    const x = 1;
+}
+
+switch (1) {
+    case 1:
+        const x = 1;
+        break;
+    case 2:
+        const y = 1;
+        break;
+}

test/rules/prefer-const/test.ts.lint

@@ -144,3 +144,26 @@ let arr = [];
 arr.push(0);
 arr[1] = 1;
 arr.length = 1;
+
+// reassignment of the forward declaration
+function useOfForwardDecl() {
+    forwardDecl = 1;
+}
+
+let forwardDecl: number;
+
+module E {
+    let x = 1;
+        ~      [Identifier 'x' is never reassigned; use 'const' instead of 'let'.]
+}
+
+switch (1) {
+    case 1:
+        let x = 1;
+            ~      [Identifier 'x' is never reassigned; use 'const' instead of 'let'.]
+        break;
+    case 2:
+        let y = 1;
+            ~      [Identifier 'y' is never reassigned; use 'const' instead of 'let'.]
+        break;
+}