From 9db2506d304898183e5e0907a334e7299bc3e089 Mon Sep 17 00:00:00 2001 From: Jakub Benes Date: Sat, 26 Jan 2019 16:11:24 +0100 Subject: [PATCH 1/2] Added fixer for newline-before-return rule --- src/rules/newlineBeforeReturnRule.ts | 19 ++- .../newline-before-return/default/test.ts.fix | 114 ++++++++++++++++++ .../default/test.tsx.fix | 17 +++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 test/rules/newline-before-return/default/test.ts.fix create mode 100644 test/rules/newline-before-return/default/test.tsx.fix diff --git a/src/rules/newlineBeforeReturnRule.ts b/src/rules/newlineBeforeReturnRule.ts index 7ccef2d5eb8..d0309229189 100644 --- a/src/rules/newlineBeforeReturnRule.ts +++ b/src/rules/newlineBeforeReturnRule.ts @@ -26,6 +26,7 @@ export class Rule extends Lint.Rules.AbstractRule { ruleName: "newline-before-return", description: "Enforces blank line before return when not the only line in the block.", rationale: "Helps maintain a readable style in your codebase.", + hasFix: true, optionsDescription: "Not configurable.", options: {}, optionExamples: [true], @@ -54,6 +55,12 @@ class NewlineBeforeReturnWalker extends Lint.AbstractWalker { return ts.forEachChild(sourceFile, cb); } + private getIndentation(node: ts.Node): string { + const text = this.sourceFile.text.substr(node.pos, node.getStart() - node.pos); + const matches = text.match(/([ \t]*)$/); + return matches !== null ? matches[1] : ""; + } + private visitReturnStatement(node: ts.ReturnStatement) { const prev = getPreviousStatement(node); if (prev === undefined) { @@ -78,10 +85,18 @@ class NewlineBeforeReturnWalker extends Lint.AbstractWalker { } } const prevLine = ts.getLineAndCharacterOfPosition(this.sourceFile, prev.end).line; - if (prevLine >= line - 1) { + const indentationCurrent = this.getIndentation(node); + const indentationPrev = this.getIndentation(prev); + + const fixer = Lint.Replacement.replaceFromTo( + start - indentationCurrent.length, + start, + line === prevLine ? `\n\n${indentationPrev}` : `\n${indentationCurrent}`, + ); + // Previous statement is on the same or previous line - this.addFailure(start, start, Rule.FAILURE_STRING); + this.addFailure(start, start, Rule.FAILURE_STRING, fixer); } } } diff --git a/test/rules/newline-before-return/default/test.ts.fix b/test/rules/newline-before-return/default/test.ts.fix new file mode 100644 index 00000000000..d256d583c85 --- /dev/null +++ b/test/rules/newline-before-return/default/test.ts.fix @@ -0,0 +1,114 @@ +function foo(bar) { + if (!bar) { + return; + } + + return bar; +} + +function foo(bar) { + if (!bar) { + var statement = ''; + + return statement; + } + + return bar; +} + +function foo(bar) { + if (!bar) { + return; + } + + /* multi-line + comment */ + return bar; +} + +var fn = () => null; +function foo() { + fn(); + + return; +} + +function foo(fn) { + fn(); + + return; +} + +function foo() { + return; +} + +function foo() { + + return; +} + +function foo(bar) { + if (!bar) return; +} + +function foo(bar) { + let someCall; + if (!bar) return; +} + +function foo(bar) { + if (!bar) { return }; +} + +function foo(bar) { + if (!bar) { + return; + } +} + +function foo(bar) { + if (!bar) { + return; + } + + return bar; +} + +function foo(bar) { + if (!bar) { + + return; + } +} + +function foo() { + + // comment + return; +} + +function test() { + console.log("Any statement"); + // Any comment + + return; +} + +function foo() { + fn(); + // comment + + // comment + return; +} + +function bar() { + "some statement"; + + //comment + //comment + //comment + return; +} + diff --git a/test/rules/newline-before-return/default/test.tsx.fix b/test/rules/newline-before-return/default/test.tsx.fix new file mode 100644 index 00000000000..e6b34d9b171 --- /dev/null +++ b/test/rules/newline-before-return/default/test.tsx.fix @@ -0,0 +1,17 @@ +import * as React from 'react'; + +
{ [].map((child: any) => { + let i = 0; + + return ; +}) }
+ +
{ [].map((child: any) => { + return ; +}) }
+ +
{ [].map((child: any) => + ; +) }
+ + From 37646c51a5e7940a7eaac9e5b71d278ddb92997b Mon Sep 17 00:00:00 2001 From: Jakub Benes Date: Sat, 26 Jan 2019 22:08:50 +0100 Subject: [PATCH 2/2] Added util and refactored both fixer for newline-before-return and curly rule --- src/rules/curlyRule.ts | 24 +++--------------- src/rules/newlineBeforeReturnRule.ts | 16 ++++-------- src/utils.ts | 25 +++++++++++++++++++ .../newline-before-return/default/test.ts.fix | 12 +++++++++ .../default/test.ts.lint | 10 ++++++++ 5 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/rules/curlyRule.ts b/src/rules/curlyRule.ts index 0e828e22edb..2b51c2b5df8 100644 --- a/src/rules/curlyRule.ts +++ b/src/rules/curlyRule.ts @@ -19,6 +19,7 @@ import { isBlock, isIfStatement, isIterationStatement, isSameLine } from "tsutil import * as ts from "typescript"; import * as Lint from "../index"; +import { newLineWithIndentation } from "../utils"; import { codeExamples } from "./code-examples/curly.examples"; @@ -172,29 +173,10 @@ class CurlyWalker extends Lint.AbstractWalker { Lint.Replacement.appendText(statement.end, " }"), ]; } else { - const match = /\n([\t ])/.exec(node.getFullText(this.sourceFile)); // determine which character to use (tab or space) - const indentation = - match === null - ? "" - : // indentation should match start of statement - match[1].repeat( - ts.getLineAndCharacterOfPosition( - this.sourceFile, - node.getStart(this.sourceFile), - ).character, - ); - - const maybeCarriageReturn = - this.sourceFile.text[this.sourceFile.getLineEndOfPosition(node.pos) - 1] === "\r" - ? "\r" - : ""; - + const newLine = newLineWithIndentation(node, this.sourceFile); return [ Lint.Replacement.appendText(statement.pos, " {"), - Lint.Replacement.appendText( - statement.end, - `${maybeCarriageReturn}\n${indentation}}`, - ), + Lint.Replacement.appendText(statement.end, `${newLine}}`), ]; } } diff --git a/src/rules/newlineBeforeReturnRule.ts b/src/rules/newlineBeforeReturnRule.ts index d0309229189..24bb1d5c626 100644 --- a/src/rules/newlineBeforeReturnRule.ts +++ b/src/rules/newlineBeforeReturnRule.ts @@ -19,6 +19,7 @@ import { getPreviousStatement } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +import { newLineWithIndentation } from "../utils"; export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ @@ -55,12 +56,6 @@ class NewlineBeforeReturnWalker extends Lint.AbstractWalker { return ts.forEachChild(sourceFile, cb); } - private getIndentation(node: ts.Node): string { - const text = this.sourceFile.text.substr(node.pos, node.getStart() - node.pos); - const matches = text.match(/([ \t]*)$/); - return matches !== null ? matches[1] : ""; - } - private visitReturnStatement(node: ts.ReturnStatement) { const prev = getPreviousStatement(node); if (prev === undefined) { @@ -86,13 +81,12 @@ class NewlineBeforeReturnWalker extends Lint.AbstractWalker { } const prevLine = ts.getLineAndCharacterOfPosition(this.sourceFile, prev.end).line; if (prevLine >= line - 1) { - const indentationCurrent = this.getIndentation(node); - const indentationPrev = this.getIndentation(prev); - const fixer = Lint.Replacement.replaceFromTo( - start - indentationCurrent.length, + line === prevLine ? node.pos : node.pos + 1, start, - line === prevLine ? `\n\n${indentationPrev}` : `\n${indentationCurrent}`, + line === prevLine + ? newLineWithIndentation(prev, this.sourceFile, 2) + : newLineWithIndentation(prev, this.sourceFile), ); // Previous statement is on the same or previous line diff --git a/src/utils.ts b/src/utils.ts index 15cb2354105..4ca158a47b1 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -294,6 +294,31 @@ function tryResolveSync(packageName: string, relativeTo?: string): string | unde } } +/** + * Gets the full indentation of the provided node + */ +export function getIndentation(node: ts.Node, sourceFile: ts.SourceFile): string { + const text = sourceFile.text.substr(node.pos, node.getStart() - node.pos); + const matches = text.match(/([ \t]*)$/); + return matches !== null ? matches[1] : ""; +} + +/** + * Creates x new lines with a proper indentation at the last one based on the provided node + */ +export function newLineWithIndentation( + node: ts.Node, + sourceFile: ts.SourceFile, + linesCount: number = 1, +) { + const maybeCarriageReturn = + sourceFile.text[sourceFile.getLineEndOfPosition(node.pos) - 1] === "\r" ? "\r" : ""; + + const indentation = getIndentation(node, sourceFile); + + return `${`${maybeCarriageReturn}\n`.repeat(linesCount)}${indentation}`; +} + /** * @deprecated Copied from tsutils 2.27.2. This will be removed once TSLint requires tsutils > 3.0. */ diff --git a/test/rules/newline-before-return/default/test.ts.fix b/test/rules/newline-before-return/default/test.ts.fix index d256d583c85..bfde13a5783 100644 --- a/test/rules/newline-before-return/default/test.ts.fix +++ b/test/rules/newline-before-return/default/test.ts.fix @@ -39,6 +39,18 @@ function foo(fn) { return; } +function foo(fn) { + fn(); + + return; +} + +function foo(fn) { + fn(); + + return; +} + function foo() { return; } diff --git a/test/rules/newline-before-return/default/test.ts.lint b/test/rules/newline-before-return/default/test.ts.lint index a684f764e2e..1ed6848f696 100644 --- a/test/rules/newline-before-return/default/test.ts.lint +++ b/test/rules/newline-before-return/default/test.ts.lint @@ -38,6 +38,16 @@ function foo(fn) { ~nil [0] } +function foo(fn) { + fn(); return; + ~nil [0] +} + +function foo(fn) { + fn(); return; + ~nil [0] +} + function foo() { return; }