Skip to content

Commit

Permalink
semicolon: split walker implementation, make "never" spec compliant (p…
Browse files Browse the repository at this point in the history 
…alantir#2655)

Separate the implementations of "never" and "always", because "never" became a little more complex.
[enhancement] `semicolon`: option `"never"` is now spec compliant
[bugfix] `semicolon`: don't warn about unnecesary semicolon when it is actually needed, e.g. when followed by type assertion or template string
Ref: microsoft/TypeScript#15444
  • Loading branch information
@ajafff @adidahiya
ajafff authored and adidahiya committed May 24, 2017
1 parent dcb400b commit d52d587
Show file tree
Hide file tree
Showing 5 changed files with 360 additions and 110 deletions.
312 changes: 202 additions & 110 deletions src/rules/semicolonRule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const OPTION_IGNORE_BOUND_CLASS_METHODS = "ignore-bound-class-methods";
const OPTION_IGNORE_INTERFACES = "ignore-interfaces";

interface Options {
always: boolean;
boundClassMethods: boolean;
interfaces: boolean;
}
Expand Down Expand Up @@ -78,100 +77,89 @@ export class Rule extends Lint.Rules.AbstractRule {

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const options: Options = {
always: this.ruleArguments.indexOf(OPTION_NEVER) === -1,
boundClassMethods: this.ruleArguments.indexOf(OPTION_IGNORE_BOUND_CLASS_METHODS) === -1,
interfaces: this.ruleArguments.indexOf(OPTION_IGNORE_INTERFACES) === -1,
};
return this.applyWithWalker(new SemicolonWalker(sourceFile, this.ruleName, options));
const Walker = this.ruleArguments.indexOf(OPTION_NEVER) === -1 ? SemicolonAlwaysWalker : SemicolonNeverWalker;
return this.applyWithWalker(new Walker(sourceFile, this.ruleName, options));
}
}

class SemicolonWalker extends Lint.AbstractWalker<Options> {
private scanner?: ts.Scanner = undefined;
abstract class SemicolonWalker extends Lint.AbstractWalker<Options> {
public walk(sourceFile: ts.SourceFile) {
const cb = (node: ts.Node): void => {
switch (node.kind) {
case ts.SyntaxKind.VariableStatement:
case ts.SyntaxKind.ExpressionStatement:
case ts.SyntaxKind.ReturnStatement:
case ts.SyntaxKind.BreakStatement:
case ts.SyntaxKind.ContinueStatement:
case ts.SyntaxKind.ThrowStatement:
case ts.SyntaxKind.ImportEqualsDeclaration:
case ts.SyntaxKind.DoStatement:
case ts.SyntaxKind.ExportAssignment:
this.checkSemicolonAt(node as ts.Statement);
break;
case ts.SyntaxKind.TypeAliasDeclaration:
case ts.SyntaxKind.ImportDeclaration:
case ts.SyntaxKind.ExportDeclaration:
case ts.SyntaxKind.DebuggerStatement:
this.checkSemicolonOrLineBreak(node);
break;
case ts.SyntaxKind.ModuleDeclaration:
// shorthand module declaration
if ((node as ts.ModuleDeclaration).body === undefined) {
this.checkSemicolonOrLineBreak(node);
}
break;
case ts.SyntaxKind.PropertyDeclaration:
this.visitPropertyDeclaration(node as ts.PropertyDeclaration);
break;
case ts.SyntaxKind.MethodDeclaration:
case ts.SyntaxKind.FunctionDeclaration:
if ((node as ts.FunctionLikeDeclaration).body === undefined) {
this.checkSemicolonOrLineBreak(node);
}
break;
case ts.SyntaxKind.InterfaceDeclaration:
if (this.options.interfaces) {
this.checkInterface(node as ts.InterfaceDeclaration);
}
break;
case ts.SyntaxKind.SemicolonClassElement:
return this.reportUnnecessary(node.end - 1);
case ts.SyntaxKind.EmptyStatement:
return this.checkEmptyStatement(node);
default:
}
this.visitNode(node);
return ts.forEachChild(node, cb);
};
return ts.forEachChild(sourceFile, cb);
}

private visitPropertyDeclaration(node: ts.PropertyDeclaration) {
// check if this is a multi-line arrow function
if (node.initializer !== undefined &&
node.initializer.kind === ts.SyntaxKind.ArrowFunction &&
!utils.isSameLine(this.sourceFile, node.getStart(this.sourceFile), node.end)) {
if (this.options.boundClassMethods) {
if (this.sourceFile.text[node.end - 1] === ";" &&
this.isFollowedByLineBreak(node.end)) {
this.reportUnnecessary(node.end - 1);
}
}
} else {
this.checkSemicolonOrLineBreak(node);
protected visitNode(node: ts.Node) {
switch (node.kind) {
case ts.SyntaxKind.SemicolonClassElement:
return this.reportUnnecessary(node.end);
case ts.SyntaxKind.EmptyStatement:
return this.checkEmptyStatement(node);
case ts.SyntaxKind.PropertyDeclaration:
return this.visitPropertyDeclaration(node as ts.PropertyDeclaration);
}
}

private isFollowedByLineBreak(pos: number) {
const scanner = this.scanner !== undefined ? this.scanner :
(this.scanner = ts.createScanner(this.sourceFile.languageVersion, true, this.sourceFile.languageVariant, this.sourceFile.text));
scanner.setTextPos(pos);
return scanner.scan() === ts.SyntaxKind.EndOfFileToken || scanner.hasPrecedingLineBreak();
protected reportUnnecessary(pos: number, noFix = false) {
this.addFailure(pos - 1, pos, Rule.FAILURE_STRING_UNNECESSARY, noFix ? undefined : Lint.Replacement.deleteText(pos - 1, 1));
}

private checkSemicolonOrLineBreak(node: ts.Node) {
const hasSemicolon = this.sourceFile.text[node.end - 1] === ";";
if (this.options.always && !hasSemicolon) {
this.reportMissing(node.end);
} else if (!this.options.always && hasSemicolon && this.isFollowedByLineBreak(node.end)) {
// semicolon can be removed if followed by line break;
this.reportUnnecessary(node.end - 1);
protected checkSemicolonOrLineBreak(node: ts.Node) {
if (this.sourceFile.text[node.end - 1] !== ";") {
return;
}
const nextToken = utils.getNextToken(node, this.sourceFile)!;
switch (nextToken.kind) {
case ts.SyntaxKind.EndOfFileToken:
case ts.SyntaxKind.CloseBraceToken:
return this.reportUnnecessary(node.end);
default:
if (!utils.isSameLine(this.sourceFile, node.end, nextToken.end)) {
this.reportUnnecessary(node.end);
}
}
}

protected checkUnnecessary(node: ts.Node) {
if (this.sourceFile.text[node.end - 1] !== ";") {
return;
}
const lastToken = utils.getPreviousToken(node.getLastToken(this.sourceFile)!, this.sourceFile)!;
// yield does not continue on the next line if there is no yielded expression
if (lastToken.kind === ts.SyntaxKind.YieldKeyword && lastToken.parent!.kind === ts.SyntaxKind.YieldExpression ||
// arrow functions with block as body don't continue on the next line
lastToken.kind === ts.SyntaxKind.CloseBraceToken && lastToken.parent!.kind === ts.SyntaxKind.Block &&
lastToken.parent!.parent!.kind === ts.SyntaxKind.ArrowFunction) {
return this.checkSemicolonOrLineBreak(node);
}
const nextToken = utils.getNextToken(node, this.sourceFile)!;
switch (nextToken.kind) {
case ts.SyntaxKind.OpenParenToken:
case ts.SyntaxKind.OpenBracketToken:
case ts.SyntaxKind.PlusToken:
case ts.SyntaxKind.MinusToken:
case ts.SyntaxKind.RegularExpressionLiteral:
case ts.SyntaxKind.LessThanToken:
case ts.SyntaxKind.NoSubstitutionTemplateLiteral:
case ts.SyntaxKind.TemplateHead:
break;
case ts.SyntaxKind.CloseBraceToken:
case ts.SyntaxKind.EndOfFileToken:
return this.reportUnnecessary(node.end);
default:
if (!utils.isSameLine(this.sourceFile, node.end, nextToken.end)) {
this.reportUnnecessary(node.end);
}
}
}

protected abstract checkPropertyDeclaration(node: ts.PropertyDeclaration): void;

private checkEmptyStatement(node: ts.Node) {
// An empty statement is only ever useful when it is the only statement inside a loop
if (!utils.isIterationStatement(node.parent!)) {
Expand All @@ -181,61 +169,165 @@ class SemicolonWalker extends Lint.AbstractWalker<Options> {
const noFix = parentKind === ts.SyntaxKind.IfStatement ||
parentKind === ts.SyntaxKind.LabeledStatement ||
parentKind === ts.SyntaxKind.WithStatement;
this.reportUnnecessary(node.end - 1, noFix);
this.reportUnnecessary(node.end, noFix);
}
}

private checkInterface(node: ts.InterfaceDeclaration) {
for (const member of node.members) {
const lastChar = this.sourceFile.text[member.end - 1];
const hasSemicolon = lastChar === ";";
if (this.options.always && !hasSemicolon) {
if (lastChar === ",") {
this.addFailureAt(member.end - 1, 1, Rule.FAILURE_STRING_COMMA, new Lint.Replacement(member.end - 1, 1, ";"));
} else {
this.reportMissing(member.end);
}
} else if (!this.options.always && hasSemicolon &&
(member === node.members[node.members.length - 1] || this.isFollowedByLineBreak(member.end))) {
this.reportUnnecessary(member.end - 1);
private visitPropertyDeclaration(node: ts.PropertyDeclaration) {
// check if this is a multi-line arrow function
if (node.initializer !== undefined &&
node.initializer.kind === ts.SyntaxKind.ArrowFunction &&
!utils.isSameLine(this.sourceFile, node.getStart(this.sourceFile), node.end)) {
if (this.options.boundClassMethods) {
this.checkUnnecessary(node);
}
} else {
this.checkPropertyDeclaration(node);
}
}
}

private reportMissing(pos: number) {
this.addFailureAt(pos, 0, Rule.FAILURE_STRING_MISSING, Lint.Replacement.appendText(pos, ";"));
class SemicolonAlwaysWalker extends SemicolonWalker {
protected visitNode(node: ts.Node) {
switch (node.kind) {
case ts.SyntaxKind.VariableStatement:
case ts.SyntaxKind.ExpressionStatement:
case ts.SyntaxKind.ReturnStatement:
case ts.SyntaxKind.BreakStatement:
case ts.SyntaxKind.ContinueStatement:
case ts.SyntaxKind.ThrowStatement:
case ts.SyntaxKind.ImportEqualsDeclaration:
case ts.SyntaxKind.DoStatement:
case ts.SyntaxKind.ExportAssignment:
case ts.SyntaxKind.TypeAliasDeclaration:
case ts.SyntaxKind.ImportDeclaration:
case ts.SyntaxKind.ExportDeclaration:
case ts.SyntaxKind.DebuggerStatement:
return this.checkMissing(node);
case ts.SyntaxKind.ModuleDeclaration:
case ts.SyntaxKind.MethodDeclaration:
case ts.SyntaxKind.FunctionDeclaration:
// check shorthand module declarations and method / function signatures
if ((node as ts.FunctionLikeDeclaration | ts.ModuleDeclaration).body === undefined) {
this.checkMissing(node);
}
break;
case ts.SyntaxKind.InterfaceDeclaration:
if (this.options.interfaces) {
this.checkInterface(node as ts.InterfaceDeclaration);
}
break;
default:
return super.visitNode(node);
}
}

private reportUnnecessary(pos: number, noFix?: boolean) {
this.addFailureAt(pos, 1, Rule.FAILURE_STRING_UNNECESSARY, noFix === true ? undefined : Lint.Replacement.deleteText(pos, 1));
protected checkPropertyDeclaration(node: ts.PropertyDeclaration) {
return this.checkMissing(node);
}

private checkSemicolonAt(node: ts.Statement) {
const hasSemicolon = this.sourceFile.text[node.end - 1] === ";";

if (this.options.always && !hasSemicolon) {
private checkMissing(node: ts.Node) {
if (this.sourceFile.text[node.end - 1] !== ";") {
this.reportMissing(node.end);
} else if (!this.options.always && hasSemicolon) {
switch (utils.getNextToken(node, this.sourceFile)!.kind) {
case ts.SyntaxKind.OpenParenToken:
case ts.SyntaxKind.OpenBracketToken:
case ts.SyntaxKind.PlusToken:
case ts.SyntaxKind.MinusToken:
case ts.SyntaxKind.RegularExpressionLiteral:
}
}

private reportMissing(pos: number) {
this.addFailureAt(pos, 0, Rule.FAILURE_STRING_MISSING, Lint.Replacement.appendText(pos, ";"));
}

private checkInterface(node: ts.InterfaceDeclaration) {
for (const member of node.members) {
switch (this.sourceFile.text[member.end - 1]) {
case ";": break;
case ",":
this.addFailureAt(member.end - 1, 1, Rule.FAILURE_STRING_COMMA, new Lint.Replacement(member.end - 1, 1, ";"));
break;
default:
if (!this.isFollowedByStatement(node)) {
this.reportUnnecessary(node.end - 1);
}
this.reportMissing(member.end);
}
}
}
}

private isFollowedByStatement(node: ts.Statement): boolean {
class SemicolonNeverWalker extends SemicolonWalker {
protected visitNode(node: ts.Node) {
switch (node.kind) {
case ts.SyntaxKind.ExpressionStatement:
case ts.SyntaxKind.ThrowStatement:
case ts.SyntaxKind.ExportAssignment:
return this.checkUnnecessary(node as ts.Statement);
case ts.SyntaxKind.VariableStatement:
return this.checkVariableStatement(node as ts.VariableStatement);
case ts.SyntaxKind.ReturnStatement:
if ((node as ts.ReturnStatement).expression === undefined) {
// return does not continue on the next line if the is no returned expression
return this.checkSemicolonOrLineBreak(node);
}
return this.checkUnnecessary(node);
case ts.SyntaxKind.TypeAliasDeclaration:
case ts.SyntaxKind.ImportEqualsDeclaration:
case ts.SyntaxKind.ImportDeclaration:
case ts.SyntaxKind.ExportDeclaration:
case ts.SyntaxKind.DebuggerStatement:
case ts.SyntaxKind.BreakStatement:
case ts.SyntaxKind.ContinueStatement:
case ts.SyntaxKind.DoStatement:
return this.checkSemicolonOrLineBreak(node);
case ts.SyntaxKind.ModuleDeclaration:
// shorthand module declaration
if ((node as ts.ModuleDeclaration).body === undefined) {
this.checkShorthandModuleDeclaration(node as ts.ModuleDeclaration);
}
break;
case ts.SyntaxKind.MethodDeclaration:
// check method signature
if ((node as ts.MethodDeclaration).body === undefined) {
this.checkSemicolonOrLineBreak(node);
}
break;
case ts.SyntaxKind.FunctionDeclaration:
// check function signature
if ((node as ts.FunctionDeclaration).body === undefined) {
this.checkSemicolonOrLineBreak(node);
}
break;
case ts.SyntaxKind.InterfaceDeclaration:
if (this.options.interfaces) {
this.checkInterface(node as ts.InterfaceDeclaration);
}
break;
default:
return super.visitNode(node);
}
}

protected checkPropertyDeclaration(node: ts.PropertyDeclaration) {
if (node.initializer === undefined) {
return this.checkSemicolonOrLineBreak(node);
}
return this.checkUnnecessary(node);
}

private checkVariableStatement(node: ts.VariableStatement) {
const declarations = node.declarationList.declarations;
if (declarations[declarations.length - 1].initializer === undefined) {
// variable declaration does not continue on the next line if it has no initializer
return this.checkSemicolonOrLineBreak(node);
}
return this.checkUnnecessary(node);
}

private checkShorthandModuleDeclaration(node: ts.ModuleDeclaration) {
const nextStatement = utils.getNextStatement(node);
if (nextStatement === undefined) {
return false;
if (nextStatement === undefined || nextStatement.kind !== ts.SyntaxKind.Block) {
this.checkSemicolonOrLineBreak(node);
}
}

private checkInterface(node: ts.InterfaceDeclaration) {
for (const member of node.members) {
this.checkSemicolonOrLineBreak(member);
}
return utils.isSameLine(this.sourceFile, node.end, nextStatement.getStart(this.sourceFile));
}
}
2 changes: 2 additions & 0 deletions test/rules/semicolon/never/eof1.ts.lint
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var foo = bar;
~ [Unnecessary semicolon]
2 changes: 2 additions & 0 deletions test/rules/semicolon/never/eof2.ts.lint
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
return;
~ [Unnecessary semicolon]
Loading

0 comments on commit d52d587

Please sign in to comment.