Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

handle forward referenced in prefer-const rule #1908

Merged
merged 3 commits into from
Dec 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/language/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
6 changes: 3 additions & 3 deletions src/language/walker/blockScopeAwareRuleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down Expand Up @@ -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();
}

Expand Down
99 changes: 61 additions & 38 deletions src/rules/preferConstRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -171,8 +196,6 @@ interface IConstCandidate {
}

class ScopeInfo {
public currentVariableDeclaration: ts.VariableDeclaration;

private identifierUsages: {
[varName: string]: {
letStatement: ts.VariableDeclarationList,
Expand Down
20 changes: 20 additions & 0 deletions test/rules/prefer-const/test.ts.fix
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
23 changes: 23 additions & 0 deletions test/rules/prefer-const/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}