From 5c1cda3c3214c7ca314913358b4502c73fdb4cd1 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 1 May 2017 11:44:55 +0200 Subject: [PATCH 1/3] semicolon: split walker, enhance option never --- src/rules/semicolonRule.ts | 317 +++++++++++++++--------- test/rules/semicolon/never/eof1.ts.lint | 2 + test/rules/semicolon/never/eof2.ts.lint | 2 + test/rules/semicolon/never/test.ts.fix | 70 ++++++ test/rules/semicolon/never/test.ts.lint | 79 ++++++ 5 files changed, 358 insertions(+), 112 deletions(-) create mode 100644 test/rules/semicolon/never/eof1.ts.lint create mode 100644 test/rules/semicolon/never/eof2.ts.lint diff --git a/src/rules/semicolonRule.ts b/src/rules/semicolonRule.ts index 2dc2a0d6c31..a8b1192b468 100644 --- a/src/rules/semicolonRule.ts +++ b/src/rules/semicolonRule.ts @@ -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; } @@ -75,101 +74,95 @@ 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 { - private scanner?: ts.Scanner = undefined; +abstract class SemicolonWalker extends Lint.AbstractWalker { 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 && - ts.getLineAndCharacterOfPosition(this.sourceFile, node.getStart(this.sourceFile)).line - !== ts.getLineAndCharacterOfPosition(this.sourceFile, node.getEnd()).line) { - 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?: boolean) { + 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); + if (nextToken === undefined) { + return this.reportUnnecessary(node.end); + } + 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); + if (nextToken === undefined) { + return this.reportUnnecessary(node.end); + } + 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!)) { @@ -179,62 +172,162 @@ class SemicolonWalker extends Lint.AbstractWalker { 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); } } } +} + +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) { + 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 isFollowedByStatement(node: ts.Statement): boolean { + 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 ts.getLineAndCharacterOfPosition(this.sourceFile, node.end).line - === ts.getLineAndCharacterOfPosition(this.sourceFile, nextStatement.getStart(this.sourceFile)).line; } } diff --git a/test/rules/semicolon/never/eof1.ts.lint b/test/rules/semicolon/never/eof1.ts.lint new file mode 100644 index 00000000000..1e1cb6b6940 --- /dev/null +++ b/test/rules/semicolon/never/eof1.ts.lint @@ -0,0 +1,2 @@ +var foo = bar; + ~ [Unnecessary semicolon] \ No newline at end of file diff --git a/test/rules/semicolon/never/eof2.ts.lint b/test/rules/semicolon/never/eof2.ts.lint new file mode 100644 index 00000000000..518b947922c --- /dev/null +++ b/test/rules/semicolon/never/eof2.ts.lint @@ -0,0 +1,2 @@ +return; + ~ [Unnecessary semicolon] \ No newline at end of file diff --git a/test/rules/semicolon/never/test.ts.fix b/test/rules/semicolon/never/test.ts.fix index ce670da5881..faf348af310 100644 --- a/test/rules/semicolon/never/test.ts.fix +++ b/test/rules/semicolon/never/test.ts.fix @@ -32,6 +32,9 @@ debugger import v = require("i") module M { export var x + [] + export var y = x; + [] export function f(s: string): string export function f(n: number): number export function f(x: any) { return x } @@ -144,3 +147,70 @@ switch(foo.bar) { const x = f(() => 0); const y = 2 + +a = b +++c + +foo(); +(bar).foo() + +foo(); +(bar as Bar).foo() + +foo(); +bar.foo() + +foo(); +() => {} + +foo(); +`bar` + +foo(); +`bar${baz}` + +a; +/b/g + +a; +/=b/i + +function bar(foo) { if (foo) return true; return false } +function bar(foo) { if (foo) {return true} return false} + +class Foo { + public bar = foo; + ['foo'] + public baz = () => true; + ['foobar'] + public baz2 = () => () => {return true} + ['foobarbaz'] + public bas = () => {return true} + ['barfoo'] + public moar = () => + true; + ['barbaz'] +} + +declare module "foo"; +{ /* don't remove semicolon, otherwise the shorthand module get's a body */ } + +function *gen() { + yield + [] + yield 1; + [] + if (foo) + return yield + [] + if (!foo) + return yield 1 || yield + [] + return yield 1; + [] +} + +var foo = () => {} +[] +var bar; var foo = function() {}; +[] diff --git a/test/rules/semicolon/never/test.ts.lint b/test/rules/semicolon/never/test.ts.lint index d45b8959b1c..2cf2a0ffe00 100644 --- a/test/rules/semicolon/never/test.ts.lint +++ b/test/rules/semicolon/never/test.ts.lint @@ -47,6 +47,9 @@ import v = require("i"); module M { export var x; ~ [0] + [] + export var y = x; + [] export function f(s: string): string; ~ [0] export function f(n: number): number @@ -187,4 +190,80 @@ switch(foo.bar) { const x = f(() => 0); const y = 2 +a = b; + ~ [0] +++c + +foo(); +(bar).foo() + +foo(); +(bar as Bar).foo() + +foo(); +bar.foo() + +foo(); +() => {} + +foo(); +`bar` + +foo(); +`bar${baz}` + +a; +/b/g + +a; +/=b/i + +function bar(foo) { if (foo) return true; return false } +function bar(foo) { if (foo) {return true;} return false;} + ~ [0] + ~ [0] + +class Foo { + public bar = foo; + ['foo'] + public baz = () => true; + ['foobar'] + public baz2 = () => () => {return true}; + ~ [0] + ['foobarbaz'] + public bas = () => {return true}; + ~ [0] + ['barfoo'] + public moar = () => + true; + ['barbaz'] +} + +declare module "foo"; +{ /* don't remove semicolon, otherwise the shorthand module get's a body */ } + +function *gen() { + yield; + ~ [0] + [] + yield 1; + [] + if (foo) + return yield; + ~ [0] + [] + if (!foo) + return yield 1 || yield; + ~ [0] + [] + return yield 1; + [] +} + +var foo = () => {}; + ~ [0] +[] +var bar; var foo = function() {}; +[] + [0]: Unnecessary semicolon \ No newline at end of file From 160fcce8a1010d380c7d62339d25f9cbded9e6d5 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Wed, 3 May 2017 17:36:06 +0200 Subject: [PATCH 2/3] handle uninitialized properties --- src/rules/semicolonRule.ts | 3 +++ test/rules/semicolon/never/test.ts.fix | 2 ++ test/rules/semicolon/never/test.ts.lint | 3 +++ 3 files changed, 8 insertions(+) diff --git a/src/rules/semicolonRule.ts b/src/rules/semicolonRule.ts index a8b1192b468..ef10585a2b9 100644 --- a/src/rules/semicolonRule.ts +++ b/src/rules/semicolonRule.ts @@ -306,6 +306,9 @@ class SemicolonNeverWalker extends SemicolonWalker { } protected checkPropertyDeclaration(node: ts.PropertyDeclaration) { + if (node.initializer === undefined) { + return this.checkSemicolonOrLineBreak(node); + } return this.checkUnnecessary(node); } diff --git a/test/rules/semicolon/never/test.ts.fix b/test/rules/semicolon/never/test.ts.fix index faf348af310..8a8aec617a4 100644 --- a/test/rules/semicolon/never/test.ts.fix +++ b/test/rules/semicolon/never/test.ts.fix @@ -179,6 +179,8 @@ function bar(foo) { if (foo) return true; return false } function bar(foo) { if (foo) {return true} return false} class Foo { + public [foo] + ['moo'] public bar = foo; ['foo'] public baz = () => true; diff --git a/test/rules/semicolon/never/test.ts.lint b/test/rules/semicolon/never/test.ts.lint index 2cf2a0ffe00..d64e82a3490 100644 --- a/test/rules/semicolon/never/test.ts.lint +++ b/test/rules/semicolon/never/test.ts.lint @@ -224,6 +224,9 @@ function bar(foo) { if (foo) {return true;} return false;} ~ [0] class Foo { + public [foo]; + ~ [0] + ['moo'] public bar = foo; ['foo'] public baz = () => true; From f8cd56f590c262b4b6e81fe7992450583d43225a Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 12 May 2017 16:14:42 +0200 Subject: [PATCH 3/3] fix lint --- src/rules/semicolonRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/semicolonRule.ts b/src/rules/semicolonRule.ts index ef10585a2b9..b28e32fb1a5 100644 --- a/src/rules/semicolonRule.ts +++ b/src/rules/semicolonRule.ts @@ -102,7 +102,7 @@ abstract class SemicolonWalker extends Lint.AbstractWalker { } } - protected reportUnnecessary(pos: number, noFix?: boolean) { + protected reportUnnecessary(pos: number, noFix = false) { this.addFailure(pos - 1, pos, Rule.FAILURE_STRING_UNNECESSARY, noFix ? undefined : Lint.Replacement.deleteText(pos - 1, 1)); }