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

Refactor variable-name rule #2355

Merged
merged 2 commits into from
Mar 24, 2017
Merged

Refactor variable-name rule #2355

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df436d1
Refactor 'variable-name'
andy-hanson Mar 16, 2017
0e0ae9c
Respond to PR comments
andy-hanson Mar 17, 2017
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
193 changes: 100 additions & 93 deletions src/rules/variableNameRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
* limitations under the License.
*/

// tslint:disable object-literal-sort-keys

import * as ts from "typescript";

import * as Lint from "../index";

const BANNED_KEYWORDS = ["any", "Number", "number", "String", "string", "Boolean", "boolean", "Undefined", "undefined"];
const bannedKeywordsSet = new Set(BANNED_KEYWORDS);
const bannedKeywordsStr = BANNED_KEYWORDS.map((kw) => `\`${kw}\``).join(", ");

const OPTION_LEADING_UNDERSCORE = "allow-leading-underscore";
const OPTION_TRAILING_UNDERSCORE = "allow-trailing-underscore";
Expand All @@ -28,7 +32,6 @@ const OPTION_CHECK_FORMAT = "check-format";
const OPTION_ALLOW_PASCAL_CASE = "allow-pascal-case";

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "variable-name",
description: "Checks variable names for various errors.",
Expand All @@ -39,8 +42,8 @@ export class Rule extends Lint.Rules.AbstractRule {
* \`"${OPTION_LEADING_UNDERSCORE}"\` allows underscores at the beginning (only has an effect if "check-format" specified)
* \`"${OPTION_TRAILING_UNDERSCORE}"\` allows underscores at the end. (only has an effect if "check-format" specified)
* \`"${OPTION_ALLOW_PASCAL_CASE}"\` allows PascalCase in addition to camelCase.
* \`"${OPTION_BAN_KEYWORDS}"\`: disallows the use of certain TypeScript keywords (\`any\`, \`Number\`, \`number\`, \`String\`,
\`string\`, \`Boolean\`, \`boolean\`, \`undefined\`) as variable or parameter names.`,
* \`"${OPTION_BAN_KEYWORDS}"\`: disallows the use of certain TypeScript keywords as variable or parameter names.
* These are: ${bannedKeywordsStr}`,
options: {
type: "array",
items: {
Expand All @@ -60,130 +63,134 @@ export class Rule extends Lint.Rules.AbstractRule {
type: "style",
typescriptOnly: false,
};
/* tslint:enable:object-literal-sort-keys */

public static FORMAT_FAILURE = "variable name must be in camelcase or uppercase";
public static KEYWORD_FAILURE = "variable name clashes with keyword/type";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const variableNameWalker = new VariableNameWalker(sourceFile, this.getOptions());
return this.applyWithWalker(variableNameWalker);
return this.applyWithFunction<Options>(sourceFile, walk, parseOptions(this.ruleArguments));
}
}

class VariableNameWalker extends Lint.RuleWalker {
private shouldBanKeywords: boolean;
private shouldCheckFormat: boolean;

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);

this.shouldBanKeywords = this.hasOption(OPTION_BAN_KEYWORDS);
interface Options {
banKeywords: boolean;
checkFormat: boolean;
leadingUnderscore: boolean;
trailingUnderscore: boolean;
allowPascalCase: boolean;
}
function parseOptions(ruleArguments: string[]): Options {
const banKeywords = hasOption(OPTION_BAN_KEYWORDS);
return {
banKeywords,
// check variable name formatting by default if no options are specified
this.shouldCheckFormat = !this.shouldBanKeywords || this.hasOption(OPTION_CHECK_FORMAT);
checkFormat: !banKeywords || hasOption(OPTION_CHECK_FORMAT),
leadingUnderscore: hasOption(OPTION_LEADING_UNDERSCORE),
trailingUnderscore: hasOption(OPTION_TRAILING_UNDERSCORE),
allowPascalCase: hasOption(OPTION_ALLOW_PASCAL_CASE),
};

function hasOption(name: string): boolean {
return ruleArguments.indexOf(name) !== -1;
}
}

public visitBindingElement(node: ts.BindingElement) {
if (node.name.kind === ts.SyntaxKind.Identifier) {
const identifier = node.name as ts.Identifier;
this.handleVariableNameKeyword(identifier);
// A destructuring pattern that does not rebind an expression is always an alias, e.g. `var {Foo} = ...;`.
// Only check if the name is rebound (`var {Foo: bar} = ...;`).
if (node.parent !== undefined && node.parent.kind !== ts.SyntaxKind.ObjectBindingPattern || node.propertyName) {
this.handleVariableNameFormat(identifier, node.initializer);
function walk(ctx: Lint.WalkContext<Options>): void {
const { options, sourceFile } = ctx;
return ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
switch (node.kind) {
case ts.SyntaxKind.BindingElement: {
const { initializer, name, propertyName } = node as ts.BindingElement;
if (name.kind === ts.SyntaxKind.Identifier) {
handleVariableNameKeyword(name);
// A destructuring pattern that does not rebind an expression is always an alias, e.g. `var {Foo} = ...;`.
// Only check if the name is rebound (`var {Foo: bar} = ...;`).
if (node.parent!.kind !== ts.SyntaxKind.ObjectBindingPattern || propertyName) {
handleVariableNameFormat(name, initializer);
}
}
break;
}
}
super.visitBindingElement(node);
}

public visitParameterDeclaration(node: ts.ParameterDeclaration) {
if (node.name.kind === ts.SyntaxKind.Identifier) {
const identifier = node.name as ts.Identifier;
this.handleVariableNameFormat(identifier, undefined /* parameters may not alias */);
this.handleVariableNameKeyword(identifier);
case ts.SyntaxKind.VariableStatement:
// skip 'declare' keywords
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.DeclareKeyword)) {
return;
}
break;

case ts.SyntaxKind.Parameter:
case ts.SyntaxKind.PropertyDeclaration:
case ts.SyntaxKind.VariableDeclaration: {
const { name, initializer } = node as ts.ParameterDeclaration | ts.PropertyDeclaration | ts.VariableDeclaration;
if (name.kind === ts.SyntaxKind.Identifier) {
handleVariableNameFormat(name, initializer);
// do not check property declarations for keywords, they are allowed to be keywords
if (node.kind !== ts.SyntaxKind.PropertyDeclaration) {
handleVariableNameKeyword(name);
}
}
break;
}
}
super.visitParameterDeclaration(node);
}

public visitPropertyDeclaration(node: ts.PropertyDeclaration) {
if (node.name != null && node.name.kind === ts.SyntaxKind.Identifier) {
const identifier = node.name as ts.Identifier;
this.handleVariableNameFormat(identifier, node.initializer);
// do not check property declarations for keywords, they are allowed to be keywords
}
super.visitPropertyDeclaration(node);
}
return ts.forEachChild(node, cb);
});

public visitVariableDeclaration(node: ts.VariableDeclaration) {
if (node.name.kind === ts.SyntaxKind.Identifier) {
const identifier = node.name as ts.Identifier;
this.handleVariableNameFormat(identifier, node.initializer);
this.handleVariableNameKeyword(identifier);
function handleVariableNameFormat(name: ts.Identifier, initializer?: ts.Expression): void {
if (!options.checkFormat) {
return;
}
super.visitVariableDeclaration(node);
}

public visitVariableStatement(node: ts.VariableStatement) {
// skip 'declare' keywords
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.DeclareKeyword)) {
super.visitVariableStatement(node);
const { text } = name;
if (initializer && isAlias(text, initializer)) {
return;
}
}

private isAlias(name: ts.Identifier, initializer: ts.Expression): boolean {
if (initializer.kind === ts.SyntaxKind.PropertyAccessExpression) {
return (initializer as ts.PropertyAccessExpression).name.text === name.text;
} else if (initializer.kind === ts.SyntaxKind.Identifier) {
return (initializer as ts.Identifier).text === name.text;
if (!isCamelCase(text, options) && !isUpperCase(text)) {
ctx.addFailureAtNode(name, Rule.FORMAT_FAILURE);
}
return false;
}

private handleVariableNameFormat(name: ts.Identifier, initializer?: ts.Expression) {
const variableName = name.text;

if (initializer && this.isAlias(name, initializer)) {
return;
}

if (this.shouldCheckFormat && !this.isCamelCase(variableName) && !isUpperCase(variableName)) {
this.addFailureAtNode(name, Rule.FORMAT_FAILURE);
function handleVariableNameKeyword(name: ts.Identifier): void {
if (options.banKeywords && bannedKeywordsSet.has(name.text)) {
ctx.addFailureAtNode(name, Rule.KEYWORD_FAILURE);
}
}
}

private handleVariableNameKeyword(name: ts.Identifier) {
const variableName = name.text;

if (this.shouldBanKeywords && BANNED_KEYWORDS.indexOf(variableName) !== -1) {
this.addFailureAtNode(name, Rule.KEYWORD_FAILURE);
}
function isAlias(name: string, initializer: ts.Expression): boolean {
switch (initializer.kind) {
case ts.SyntaxKind.PropertyAccessExpression:
return (initializer as ts.PropertyAccessExpression).name.text === name;
case ts.SyntaxKind.Identifier:
return (initializer as ts.Identifier).text === name;
default:
return false;
}
}

private isCamelCase(name: string) {
const firstCharacter = name.charAt(0);
const lastCharacter = name.charAt(name.length - 1);
const middle = name.substr(1, name.length - 2);
function isCamelCase(name: string, options: Options): boolean {
const firstCharacter = name[0];
const lastCharacter = name[name.length - 1];
const middle = name.slice(1, -1);

if (name.length <= 0) {
return true;
}
if (!this.hasOption(OPTION_LEADING_UNDERSCORE) && firstCharacter === "_") {
return false;
}
if (!this.hasOption(OPTION_TRAILING_UNDERSCORE) && lastCharacter === "_") {
return false;
}
if (!this.hasOption(OPTION_ALLOW_PASCAL_CASE) && !isLowerCase(firstCharacter)) {
return false;
}
return middle.indexOf("_") === -1;
if (!options.leadingUnderscore && firstCharacter === "_") {
return false;
}
if (!options.trailingUnderscore && lastCharacter === "_") {
return false;
}
if (!options.allowPascalCase && !isLowerCase(firstCharacter)) {
return false;
}
return middle.indexOf("_") === -1;
}

function isLowerCase(name: string) {
function isLowerCase(name: string): boolean {
return name === name.toLowerCase();
}

function isUpperCase(name: string) {
function isUpperCase(name: string): boolean {
return name === name.toUpperCase();
}