From 4e556277dadebd00f1f6f0663df99fdb5a86fa38 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Fri, 20 Oct 2017 05:46:30 +0200 Subject: [PATCH] one-line: refactor and add fixer (#3200) --- src/rules/oneLineRule.ts | 326 +++++------------- test/rules/one-line/all/test.ts.fix | 138 ++++++++ test/rules/one-line/all/test.ts.lint | 22 ++ test/rules/one-line/no-whitespace/test.ts.fix | 132 +++++++ .../rules/one-line/no-whitespace/test.ts.lint | 170 +++++++++ test/rules/one-line/no-whitespace/tslint.json | 5 + 6 files changed, 561 insertions(+), 232 deletions(-) create mode 100644 test/rules/one-line/all/test.ts.fix create mode 100644 test/rules/one-line/no-whitespace/test.ts.fix create mode 100644 test/rules/one-line/no-whitespace/test.ts.lint create mode 100644 test/rules/one-line/no-whitespace/tslint.json diff --git a/src/rules/oneLineRule.ts b/src/rules/oneLineRule.ts index 2b847ef9e16..7c311c35f14 100644 --- a/src/rules/oneLineRule.ts +++ b/src/rules/oneLineRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { getPreviousToken } from "tsutils"; +import { getChildOfKind, isBlockLike, isObjectLiteralExpression, isSameLine } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -26,6 +26,14 @@ const OPTION_ELSE = "check-else"; const OPTION_FINALLY = "check-finally"; const OPTION_WHITESPACE = "check-whitespace"; +interface Options { + brace: boolean; + catch: boolean; + else: boolean; + finally: boolean; + whitespace: boolean; +} + export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { @@ -51,249 +59,103 @@ export class Rule extends Lint.Rules.AbstractRule { optionExamples: [[true, OPTION_CATCH, OPTION_FINALLY, OPTION_ELSE]], type: "style", typescriptOnly: false, + hasFix: true, }; /* tslint:enable:object-literal-sort-keys */ - public static BRACE_FAILURE_STRING = "misplaced opening brace"; - public static CATCH_FAILURE_STRING = "misplaced 'catch'"; - public static ELSE_FAILURE_STRING = "misplaced 'else'"; - public static FINALLY_FAILURE_STRING = "misplaced 'finally'"; public static WHITESPACE_FAILURE_STRING = "missing whitespace"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const oneLineWalker = new OneLineWalker(sourceFile, this.getOptions()); - return this.applyWithWalker(oneLineWalker); + return this.applyWithWalker(new OneLineWalker(sourceFile, this.ruleName, { + brace: this.ruleArguments.indexOf(OPTION_BRACE) !== -1, + catch: this.ruleArguments.indexOf(OPTION_CATCH) !== -1, + else: this.ruleArguments.indexOf(OPTION_ELSE) !== -1, + finally: this.ruleArguments.indexOf(OPTION_FINALLY) !== -1, + whitespace: this.ruleArguments.indexOf(OPTION_WHITESPACE) !== -1, + })); } } -class OneLineWalker extends Lint.RuleWalker { - public visitIfStatement(node: ts.IfStatement) { - const thenStatement = node.thenStatement; - const thenIsBlock = thenStatement.kind === ts.SyntaxKind.Block; - if (thenIsBlock) { - const expressionCloseParen = node.getChildAt(3); - const thenOpeningBrace = thenStatement.getChildAt(0); - this.handleOpeningBrace(expressionCloseParen, thenOpeningBrace); - } - - const elseStatement = node.elseStatement; - if (elseStatement !== undefined) { - // find the else keyword - const elseKeyword = Lint.childOfKind(node, ts.SyntaxKind.ElseKeyword)!; - if (elseStatement.kind === ts.SyntaxKind.Block) { - const elseOpeningBrace = elseStatement.getChildAt(0); - this.handleOpeningBrace(elseKeyword, elseOpeningBrace); - } - if (thenIsBlock && this.hasOption(OPTION_ELSE)) { - const thenStatementEndLine = this.getLineAndCharacterOfPosition(thenStatement.getEnd()).line; - const elseKeywordLine = this.getLineAndCharacterOfPosition(elseKeyword.getStart()).line; - if (thenStatementEndLine !== elseKeywordLine) { - this.addFailureAtNode(elseKeyword, Rule.ELSE_FAILURE_STRING); +class OneLineWalker extends Lint.AbstractWalker { + public walk(sourceFile: ts.SourceFile) { + const cb = (node: ts.Node): void => { + switch (node.kind) { + case ts.SyntaxKind.Block: + if (!isBlockLike(node.parent!)) { + this.check({pos: node.pos, end: (node as ts.Block).statements.pos}); + } + break; + case ts.SyntaxKind.CaseBlock: + this.check({pos: node.pos, end: (node as ts.CaseBlock).clauses.pos}); + break; + case ts.SyntaxKind.ModuleBlock: + this.check({pos: node.pos, end: (node as ts.ModuleBlock).statements.pos}); + break; + case ts.SyntaxKind.EnumDeclaration: + this.check({pos: (node as ts.EnumDeclaration).name.end, end: (node as ts.EnumDeclaration).members.pos}); + break; + case ts.SyntaxKind.InterfaceDeclaration: + case ts.SyntaxKind.ClassDeclaration: + case ts.SyntaxKind.ClassExpression: { + this.check(getChildOfKind(node, ts.SyntaxKind.OpenBraceToken, sourceFile)!); + break; } - } - } - - super.visitIfStatement(node); - } - - public visitCatchClause(node: ts.CatchClause) { - const catchClosingParen = Lint.childOfKind(node, ts.SyntaxKind.CloseParenToken); - const catchOpeningBrace = node.block.getChildAt(0); - this.handleOpeningBrace(catchClosingParen, catchOpeningBrace); - super.visitCatchClause(node); - } - - public visitTryStatement(node: ts.TryStatement) { - const catchClause = node.catchClause; - const finallyBlock = node.finallyBlock; - const finallyKeyword = Lint.childOfKind(node, ts.SyntaxKind.FinallyKeyword); - - // "visit" try block - const tryKeyword = node.getChildAt(0); - const tryBlock = node.tryBlock; - const tryOpeningBrace = tryBlock.getChildAt(0); - this.handleOpeningBrace(tryKeyword, tryOpeningBrace); - - if (this.hasOption(OPTION_CATCH) && catchClause !== undefined) { - const tryClosingBrace = node.tryBlock.getChildAt(node.tryBlock.getChildCount() - 1); - const catchKeyword = catchClause.getChildAt(0); - const tryClosingBraceLine = this.getLineAndCharacterOfPosition(tryClosingBrace.getEnd()).line; - const catchKeywordLine = this.getLineAndCharacterOfPosition(catchKeyword.getStart()).line; - if (tryClosingBraceLine !== catchKeywordLine) { - this.addFailureAtNode(catchKeyword, Rule.CATCH_FAILURE_STRING); - } - } - - if (finallyBlock !== undefined && finallyKeyword !== undefined) { - const finallyOpeningBrace = finallyBlock.getChildAt(0); - this.handleOpeningBrace(finallyKeyword, finallyOpeningBrace); - - if (this.hasOption(OPTION_FINALLY)) { - const previousBlock = catchClause !== undefined ? catchClause.block : node.tryBlock; - const closingBrace = previousBlock.getChildAt(previousBlock.getChildCount() - 1); - const closingBraceLine = this.getLineAndCharacterOfPosition(closingBrace.getEnd()).line; - const finallyKeywordLine = this.getLineAndCharacterOfPosition(finallyKeyword.getStart()).line; - if (closingBraceLine !== finallyKeywordLine) { - this.addFailureAtNode(finallyKeyword, Rule.FINALLY_FAILURE_STRING); + case ts.SyntaxKind.IfStatement: { + const { thenStatement, elseStatement } = node as ts.IfStatement; + if (elseStatement !== undefined && thenStatement.kind === ts.SyntaxKind.Block) { + this.check({pos: thenStatement.end, end: elseStatement.pos}, "else"); + } + break; + } + case ts.SyntaxKind.TryStatement: { + const { finallyBlock, catchClause, tryBlock } = node as ts.TryStatement; + if (catchClause !== undefined) { + this.check(catchClause.getChildAt(0, sourceFile), "catch"); + if (finallyBlock !== undefined) { + this.check({pos: catchClause.end, end: finallyBlock.pos}, "finally"); + } + } else if (finallyBlock !== undefined) { + this.check({pos: tryBlock.end, end: finallyBlock.pos}, "finally"); + } + break; + } + case ts.SyntaxKind.BinaryExpression: { + const { operatorToken, right } = node as ts.BinaryExpression; + if (operatorToken.kind === ts.SyntaxKind.EqualsToken && isObjectLiteralExpression(right)) { + this.check({pos: right.pos, end: right.properties.pos}); + } + break; + } + case ts.SyntaxKind.VariableDeclaration: { + const { initializer } = node as ts.VariableDeclaration; + if (initializer !== undefined && isObjectLiteralExpression(initializer)) { + this.check({pos: initializer.pos, end: initializer.properties.pos}); + } + break; + } + case ts.SyntaxKind.TypeAliasDeclaration: { + const { type } = node as ts.TypeAliasDeclaration; + if (type.kind === ts.SyntaxKind.MappedType || type.kind === ts.SyntaxKind.TypeLiteral) { + this.check(type.getChildAt(0, sourceFile)); + } } } - } - - super.visitTryStatement(node); - } - - public visitForStatement(node: ts.ForStatement) { - this.handleIterationStatement(node); - super.visitForStatement(node); - } - - public visitForInStatement(node: ts.ForInStatement) { - this.handleIterationStatement(node); - super.visitForInStatement(node); - } - - public visitWhileStatement(node: ts.WhileStatement) { - this.handleIterationStatement(node); - super.visitWhileStatement(node); - } - - public visitBinaryExpression(node: ts.BinaryExpression) { - const rightkind = node.right.kind; - const opkind = node.operatorToken.kind; - - if (opkind === ts.SyntaxKind.EqualsToken && rightkind === ts.SyntaxKind.ObjectLiteralExpression) { - const equalsToken = node.getChildAt(1); - const openBraceToken = node.right.getChildAt(0); - this.handleOpeningBrace(equalsToken, openBraceToken); - } - - super.visitBinaryExpression(node); - } - - public visitVariableDeclaration(node: ts.VariableDeclaration) { - const initializer = node.initializer; - if (initializer !== undefined && initializer.kind === ts.SyntaxKind.ObjectLiteralExpression) { - const equalsToken = Lint.childOfKind(node, ts.SyntaxKind.EqualsToken); - const openBraceToken = initializer.getChildAt(0); - this.handleOpeningBrace(equalsToken, openBraceToken); - } - super.visitVariableDeclaration(node); - } - - public visitDoStatement(node: ts.DoStatement) { - const doKeyword = node.getChildAt(0); - const statement = node.statement; - if (statement.kind === ts.SyntaxKind.Block) { - const openBraceToken = statement.getChildAt(0); - this.handleOpeningBrace(doKeyword, openBraceToken); - } - super.visitDoStatement(node); - } - - public visitModuleDeclaration(node: ts.ModuleDeclaration) { - const nameNode = node.name; - const body = node.body; - if (body !== undefined && body.kind === ts.SyntaxKind.ModuleBlock) { - const openBraceToken = body.getChildAt(0); - this.handleOpeningBrace(nameNode, openBraceToken); - } - super.visitModuleDeclaration(node); - } - - public visitEnumDeclaration(node: ts.EnumDeclaration) { - const nameNode = node.name; - const openBraceToken = Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!; - this.handleOpeningBrace(nameNode, openBraceToken); - super.visitEnumDeclaration(node); - } - - public visitSwitchStatement(node: ts.SwitchStatement) { - const closeParenToken = node.getChildAt(3); - const openBraceToken = node.caseBlock.getChildAt(0); - this.handleOpeningBrace(closeParenToken, openBraceToken); - super.visitSwitchStatement(node); - } - - public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) { - this.handleClassLikeDeclaration(node); - super.visitInterfaceDeclaration(node); - } - - public visitClassDeclaration(node: ts.ClassDeclaration) { - this.handleClassLikeDeclaration(node); - super.visitClassDeclaration(node); - } - - public visitFunctionDeclaration(node: ts.FunctionDeclaration) { - this.handleFunctionLikeDeclaration(node); - super.visitFunctionDeclaration(node); - } - - public visitMethodDeclaration(node: ts.MethodDeclaration) { - this.handleFunctionLikeDeclaration(node); - super.visitMethodDeclaration(node); - } - - public visitConstructorDeclaration(node: ts.ConstructorDeclaration) { - this.handleFunctionLikeDeclaration(node); - super.visitConstructorDeclaration(node); - } - - public visitArrowFunction(node: ts.ArrowFunction) { - const body = node.body; - if (body !== undefined && body.kind === ts.SyntaxKind.Block) { - const arrowToken = Lint.childOfKind(node, ts.SyntaxKind.EqualsGreaterThanToken); - const openBraceToken = body.getChildAt(0); - this.handleOpeningBrace(arrowToken, openBraceToken); - } - super.visitArrowFunction(node); - } - - private handleFunctionLikeDeclaration(node: ts.FunctionLikeDeclaration) { - const body = node.body; - if (body !== undefined && body.kind === ts.SyntaxKind.Block) { - const openBraceToken = body.getChildAt(0); - if (node.type !== undefined) { - this.handleOpeningBrace(node.type, openBraceToken); - } else { - const closeParenToken = Lint.childOfKind(node, ts.SyntaxKind.CloseParenToken); - this.handleOpeningBrace(closeParenToken, openBraceToken); - } - } - } - - private handleClassLikeDeclaration(node: ts.ClassDeclaration | ts.InterfaceDeclaration) { - const openBraceToken = Lint.childOfKind(node, ts.SyntaxKind.OpenBraceToken)!; - this.handleOpeningBrace(getPreviousToken(openBraceToken), openBraceToken); - } - - private handleIterationStatement(node: ts.IterationStatement) { - // last child is the statement, second to last child is the close paren - const closeParenToken = node.getChildAt(node.getChildCount() - 2); - const statement = node.statement; - if (statement.kind === ts.SyntaxKind.Block) { - const openBraceToken = statement.getChildAt(0); - this.handleOpeningBrace(closeParenToken, openBraceToken); - } - } - - private handleOpeningBrace(previousNode: ts.Node | undefined, openBraceToken: ts.Node) { - if (previousNode === undefined || openBraceToken === undefined) { - return; - } - - const previousNodeLine = this.getLineAndCharacterOfPosition(previousNode.getEnd()).line; - const openBraceLine = this.getLineAndCharacterOfPosition(openBraceToken.getStart()).line; - let failure: string | undefined; - - if (this.hasOption(OPTION_BRACE) && previousNodeLine !== openBraceLine) { - failure = Rule.BRACE_FAILURE_STRING; - } else if (this.hasOption(OPTION_WHITESPACE) && previousNode.getEnd() === openBraceToken.getStart()) { - failure = Rule.WHITESPACE_FAILURE_STRING; - } - - if (failure !== undefined) { - this.addFailureAtNode(openBraceToken, failure); + return ts.forEachChild(node, cb); + }; + return ts.forEachChild(sourceFile, cb); + } + + private check(range: ts.TextRange, kind?: "catch" | "else" | "finally") { + const tokenStart = range.end - (kind === undefined ? 1 : kind.length); + if (this.options[kind === undefined ? "brace" : kind] && !isSameLine(this.sourceFile, range.pos, tokenStart)) { + this.addFailure( + tokenStart, + range.end, + `misplaced ${kind === undefined ? "opening brace" : `'${kind}'`}`, + Lint.Replacement.replaceFromTo(range.pos, tokenStart, this.options.whitespace ? " " : ""), + ); + } else if (this.options.whitespace && range.pos === tokenStart) { + this.addFailure(tokenStart, range.end, Rule.WHITESPACE_FAILURE_STRING, Lint.Replacement.appendText(range.pos, " ")); } } } diff --git a/test/rules/one-line/all/test.ts.fix b/test/rules/one-line/all/test.ts.fix new file mode 100644 index 00000000000..ec67c2b0cbd --- /dev/null +++ b/test/rules/one-line/all/test.ts.fix @@ -0,0 +1,138 @@ +module Module { + export enum Enumeration { + A, + B, + C, + D + } + + export function Call() { + if (x == 3) { + x = 4; + } else { + x = 5; + } + return "called"; + } +} + +interface Class { + variable: string; +} + +var object = { + a: 1, + b: 2 +}; + +for(var x= 0; x < 1; ++x) { + ++i; +} + +switch(y) { + case 0: + x--; + break; + default: + x++; + break; +} + +try { + throw new Error("hi"); +} catch (e) { + throw(e); +} finally { + // do something +} + +try { + foo(); +} finally { + bar(); +} + +try { + foo(); +} catch(e) { + bar(); +} finally { + baz(); +} + +while(x < 10) { + x++; +} + +function f(): + number { + + return 5; +} + +class BarBooBaz { + +} + +class FooBarBaz { +} + +// Valid multiline declarations +export class LongDescriptiveClassName, S> + extends SomeAbstractBaseClass implements IImportantInterface { +} + +export interface LongDescriptiveInterfaceName + extends AThirdInterface { +} + +function longFunctionNameWithLotsOfParams( + x: number, + y: number, + z: number, + a: T) { +} + +let geoConfig: { + maximumAge?: number; + timeout?: number; + enableHighAccuracy?: boolean; +} = { + maximumAge: 3000, + timeout: 5000, + enableHighAccuracy: false +}; + +declare module "*"; +declare module "someLibrary/*"; + +// No error if there are no braces +if (true) + true; +else + false; + +class Foo< + Bar, + Baz, + Bas +> { + // works for multiline type parameters +} + +type Foo = { + bar, +}; +type Bar = { bar }; +type Baz = { [K in T]: T }; +type Bas = "a"; + +{ + {}{}{} // just some random blocks +} + +if (true) { + true; +} else { + false; +} diff --git a/test/rules/one-line/all/test.ts.lint b/test/rules/one-line/all/test.ts.lint index f2d52d0e073..037fd008c79 100644 --- a/test/rules/one-line/all/test.ts.lint +++ b/test/rules/one-line/all/test.ts.lint @@ -157,3 +157,25 @@ class Foo< > { // works for multiline type parameters } + +type Foo = { + bar, +}; +type Bar = +{ bar }; +~ [misplaced opening brace] +type Baz = +{ [K in T]: T }; +~ [misplaced opening brace] +type Bas = "a"; + +{ + {}{}{} // just some random blocks +} + +if (true) { + true; +}else { + ~~~~ [missing whitespace] + false; +} diff --git a/test/rules/one-line/no-whitespace/test.ts.fix b/test/rules/one-line/no-whitespace/test.ts.fix new file mode 100644 index 00000000000..e4e1ceb0770 --- /dev/null +++ b/test/rules/one-line/no-whitespace/test.ts.fix @@ -0,0 +1,132 @@ +module Module{ + export enum Enumeration{ + A, + B, + C, + D + } + + export function Call(){ + if (x == 3){ + x = 4; + }else { + x = 5; + } + return "called"; + } +} + +interface Class{ + variable: string; +} + +var object ={ + a: 1, + b: 2 +}; + +for(var x= 0; x < 1; ++x){ + ++i; +} + +switch(y){ + case 0: + x--; + break; + default: + x++; + break; +} + +try{ + throw new Error("hi"); +}catch (e){ + throw(e); +}finally{ + // do something +} + +try { + foo(); +}finally{ + bar(); +} + +try{ + foo(); +} catch(e){ + bar(); +} finally{ + baz(); +} + +while(x < 10){ + x++; +} + +function f(): + number { + + return 5; +} + +class BarBooBaz{ + +} + +class FooBarBaz { +} + +// Valid multiline declarations +export class LongDescriptiveClassName, S> + extends SomeAbstractBaseClass implements IImportantInterface { +} + +export interface LongDescriptiveInterfaceName + extends AThirdInterface { +} + +function longFunctionNameWithLotsOfParams( + x: number, + y: number, + z: number, + a: T) { +} + +let geoConfig: { + maximumAge?: number; + timeout?: number; + enableHighAccuracy?: boolean; +} = { + maximumAge: 3000, + timeout: 5000, + enableHighAccuracy: false +}; + +declare module "*"; +declare module "someLibrary/*"; + +// No error if there are no braces +if (true) + true; +else + false; + +class Foo< + Bar, + Baz, + Bas +> { + // works for multiline type parameters +} + +type Foo = { + bar, +}; +type Bar ={ bar }; +type Baz ={ [K in T]: T }; +type Bas = "a"; + +{ + {}{}{} // just some random blocks +} diff --git a/test/rules/one-line/no-whitespace/test.ts.lint b/test/rules/one-line/no-whitespace/test.ts.lint new file mode 100644 index 00000000000..6ed6e4de9a2 --- /dev/null +++ b/test/rules/one-line/no-whitespace/test.ts.lint @@ -0,0 +1,170 @@ +module Module +{ +~ [misplaced opening brace] + export enum Enumeration + { + ~ [misplaced opening brace] + A, + B, + C, + D + } + + export function Call() + { + ~ [misplaced opening brace] + if (x == 3) + { + ~ [misplaced opening brace] + x = 4; + } + else { + ~~~~ [misplaced 'else'] + x = 5; + } + return "called"; + } +} + +interface Class +{ +~ [misplaced opening brace] + variable: string; +} + +var object = +{ +~ [misplaced opening brace] + a: 1, + b: 2 +}; + +for(var x= 0; x < 1; ++x) +{ +~ [misplaced opening brace] + ++i; +} + +switch(y) +{ +~ [misplaced opening brace] + case 0: + x--; + break; + default: + x++; + break; +} + +try +{ +~ [misplaced opening brace] + throw new Error("hi"); +} +catch (e) +~~~~~ [misplaced 'catch'] +{ +~ [misplaced opening brace] + throw(e); +} +finally +~~~~~~~ [misplaced 'finally'] +{ +~ [misplaced opening brace] + // do something +} + +try { + foo(); +} +finally +~~~~~~~ [misplaced 'finally'] +{ +~ [misplaced opening brace] + bar(); +} + +try{ + foo(); +} catch(e){ + bar(); +} finally{ + baz(); +} + +while(x < 10){ + x++; +} + +function f(): + number { + + return 5; +} + +class BarBooBaz +{ +~ [misplaced opening brace] + +} + +class FooBarBaz { +} + +// Valid multiline declarations +export class LongDescriptiveClassName, S> + extends SomeAbstractBaseClass implements IImportantInterface { +} + +export interface LongDescriptiveInterfaceName + extends AThirdInterface { +} + +function longFunctionNameWithLotsOfParams( + x: number, + y: number, + z: number, + a: T) { +} + +let geoConfig: { + maximumAge?: number; + timeout?: number; + enableHighAccuracy?: boolean; +} = { + maximumAge: 3000, + timeout: 5000, + enableHighAccuracy: false +}; + +declare module "*"; +declare module "someLibrary/*"; + +// No error if there are no braces +if (true) + true; +else + false; + +class Foo< + Bar, + Baz, + Bas +> { + // works for multiline type parameters +} + +type Foo = { + bar, +}; +type Bar = +{ bar }; +~ [misplaced opening brace] +type Baz = +{ [K in T]: T }; +~ [misplaced opening brace] +type Bas = "a"; + +{ + {}{}{} // just some random blocks +} diff --git a/test/rules/one-line/no-whitespace/tslint.json b/test/rules/one-line/no-whitespace/tslint.json new file mode 100644 index 00000000000..80fd92e4cd0 --- /dev/null +++ b/test/rules/one-line/no-whitespace/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "one-line": [true, "check-open-brace", "check-catch", "check-else", "check-finally"] + } +}