From fad93a8741ce84b0fa98401c6ca5241eee1f494e Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 22 Dec 2016 20:31:33 -0800 Subject: [PATCH] handle forward referenced in prefer-const rule (#1908) --- src/language/utils.ts | 18 ++++ .../walker/blockScopeAwareRuleWalker.ts | 6 +- src/rules/preferConstRule.ts | 99 ++++++++++++------- test/rules/prefer-const/test.ts.fix | 20 ++++ test/rules/prefer-const/test.ts.lint | 23 +++++ 5 files changed, 125 insertions(+), 41 deletions(-) diff --git a/src/language/utils.ts b/src/language/utils.ts index bdcb8bc3518..1c58f7fcf64 100644 --- a/src/language/utils.ts +++ b/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. */ diff --git a/src/language/walker/blockScopeAwareRuleWalker.ts b/src/language/walker/blockScopeAwareRuleWalker.ts index c61ca7473ac..4f962de6e95 100644 --- a/src/language/walker/blockScopeAwareRuleWalker.ts +++ b/src/language/walker/blockScopeAwareRuleWalker.ts @@ -30,10 +30,10 @@ export abstract class BlockScopeAwareRuleWalker 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 extends ScopeAwareRuleWalk const isNewBlockScope = this.isBlockScopeBoundary(node); if (isNewBlockScope) { - this.blockScopeStack.push(this.createBlockScope()); + this.blockScopeStack.push(this.createBlockScope(node)); this.onBlockScopeStart(); } diff --git a/src/rules/preferConstRule.ts b/src/rules/preferConstRule.ts index fc020eda59b..44ffe4db12c 100644 --- a/src/rules/preferConstRule.ts +++ b/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, diff --git a/test/rules/prefer-const/test.ts.fix b/test/rules/prefer-const/test.ts.fix index 8a744e6b773..edcd4da665b 100644 --- a/test/rules/prefer-const/test.ts.fix +++ b/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; +} \ No newline at end of file diff --git a/test/rules/prefer-const/test.ts.lint b/test/rules/prefer-const/test.ts.lint index dd95d37e1ba..80d8bb407bc 100644 --- a/test/rules/prefer-const/test.ts.lint +++ b/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; +} \ No newline at end of file