refactor newlineBeforeReturn to support more variants

src/configs/all.ts

@@ -190,7 +190,8 @@ export const rules = {
     "jsdoc-format": true,
     "match-default-export-name": true,
     "new-parens": true,
-    "newline-before-return": true,
+    "newline": [true],
+    "newline-before-return": [true],
     "no-angle-bracket-type-assertion": true,
     "no-boolean-literal-compare": true,
     "no-consecutive-blank-lines": true,

src/rules/newlineBeforeReturnRule.ts

@@ -15,14 +15,15 @@
  * limitations under the License.
  */
 
-import { getPreviousStatement } from "tsutils";
+import { NewlineWalker, parseOptions } from "./newlineRule";
 import * as ts from "typescript";
 import * as Lint from "../index";
 
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "newline-before-return",
+        deprecationMessage: "Use newline instead of 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.",
         optionsDescription: "Not configurable.",
@@ -36,48 +37,6 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static FAILURE_STRING = "Missing blank line before return";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NewlineBeforeReturnWalker(sourceFile, this.ruleName, undefined));
-    }
-}
-
-class NewlineBeforeReturnWalker extends Lint.AbstractWalker<void> {
-    public walk(sourceFile: ts.SourceFile) {
-        const cb = (node: ts.Node): void => {
-            if (node.kind === ts.SyntaxKind.ReturnStatement) {
-                this.visitReturnStatement(node as ts.ReturnStatement);
-            }
-            return ts.forEachChild(node, cb);
-        };
-        return ts.forEachChild(sourceFile, cb);
-    }
-
-    private visitReturnStatement(node: ts.ReturnStatement) {
-        const prev = getPreviousStatement(node);
-        if (prev === undefined) {
-            // return is not within a block (e.g. the only child of an IfStatement) or the first statement of the block
-            // no need to check for preceding newline
-            return;
-        }
-
-        let start = node.getStart(this.sourceFile);
-        let line = ts.getLineAndCharacterOfPosition(this.sourceFile, start).line;
-        const comments = ts.getLeadingCommentRanges(this.sourceFile.text, node.pos);
-        if (comments !== undefined) {
-            // check for blank lines between comments
-            for (let i = comments.length - 1; i >= 0; --i) {
-                const endLine = ts.getLineAndCharacterOfPosition(this.sourceFile, comments[i].end).line;
-                if (endLine < line - 1) {
-                    return;
-                }
-                start = comments[i].pos;
-                line = ts.getLineAndCharacterOfPosition(this.sourceFile, start).line;
-            }
-        }
-        const prevLine = ts.getLineAndCharacterOfPosition(this.sourceFile, prev.end).line;
-
-        if (prevLine >= line - 1) {
-            // Previous statement is on the same or previous line
-            this.addFailure(start, start, Rule.FAILURE_STRING);
-        }
+        return this.applyWithWalker(new NewlineWalker(sourceFile, this.ruleName, parseOptions({return: "always"})));
     }
 }

src/rules/newlineRule.ts

@@ -0,0 +1,144 @@
+/**
+* @license
+* Copyright 2017 Palantir Technologies, Inc.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+
+import { getPreviousStatement, isBlock, isClassDeclaration, isFunctionWithBody } from "tsutils";
+import * as ts from "typescript";
+import * as Lint from "../index";
+
+export enum ViolationType {
+    needed = "needed",
+    needless = "needless",
+}
+
+const ALWAYS_IGNORE_OR_NEVER = {
+    enum: ["always", "never", "ignore"],
+    type: "string",
+};
+
+export type OptionName = "return" | "class" | "functionBlock" | "block";
+export type Option = "always" | "ignore" | "never";
+export type Options = Record<OptionName, Option>;
+
+export function parseOptions(json: Partial<Options> | undefined) {
+    const defaultValues: Options = {return: "ignore", class: "ignore", functionBlock: "ignore", block: "ignore"};
+    return {...defaultValues, ...json};
+}
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "newline",
+        description: "Requires or disallows blank lines before return, after class declaration, function block declaration or block.",
+        rationale: "Helps maintain a readable style in your codebase.",
+        optionsDescription: Lint.Utils.dedent`
+            Following arguments may be optionally provided:
+
+            * \`"return"\` checks for empty listane before return when not the only statement in the block.
+            * \`"class"\` checks for empty line at the start of a class body.
+            * \`"functionBlock"\` checks for empty line at the start of a function block.
+            * \`"block"\` checks for empty line after block declaration.`,
+        options: {
+            properties: {
+                return: ALWAYS_IGNORE_OR_NEVER,
+                class: ALWAYS_IGNORE_OR_NEVER,
+                functionBlock: ALWAYS_IGNORE_OR_NEVER,
+                block: ALWAYS_IGNORE_OR_NEVER,
+            },
+            type: "object",
+        },
+        optionExamples: [[true, {return: "always", class: "never"}]],
+        type: "style",
+        typescriptOnly: false,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING_FACTORY(kind: ViolationType, nodeType: string) {
+        const kindMsg = kind === ViolationType.needed ? 'Missing' : 'Unneeded';
+        return `${kindMsg} blank line before ${nodeType}`;
+    }
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new NewlineWalker(sourceFile, this.ruleName, parseOptions(this.ruleArguments[0] as Partial<Options>)));
+    }
+}
+
+export class NewlineWalker extends Lint.AbstractWalker<Options> {
+    public walk(sourceFile: ts.SourceFile) {
+        const cb = (node: ts.Node): void => {
+            if (node.kind === ts.SyntaxKind.ReturnStatement && this.options.return !== "ignore") {
+                const prev = getPreviousStatement(node as ts.Statement);
+                if (prev === undefined) {
+                    // return is not within a block (e.g. the only child of an IfStatement) or the first statement of the block
+                    // no need to check for preceding newline
+                    return;
+                }
+
+                this.checkForEmptyLine(prev.end, node, this.options.return === "always", 'return');
+            } else if (isClassDeclaration(node) && this.options.class !== "ignore" && node.members.length > 0) {
+                this.checkForEmptyLine(
+                    node.members[0].pos, node.members[0],
+                    this.options.class === "always", 'first member',
+                );
+            } else if (isBlock(node) && node.statements.length > 0) {
+                if (isFunctionWithBody(node.parent!) && this.options.functionBlock !== "ignore") {
+                    this.checkForEmptyLine(
+                        node.statements[0].pos, node.statements[0], this.options.functionBlock === "always", 'first statement',
+                    );
+                } else if (this.options.block !== "ignore") {
+                    this.checkForEmptyLine(
+                        node.statements[0].pos, node.statements[0], this.options.block === "always", 'first statement',
+                    );
+                }
+            }
+            return ts.forEachChild(node, cb);
+        };
+        return ts.forEachChild(sourceFile, cb);
+    }
+
+    private checkForEmptyLine(start: number, firstNode: ts.Node, lineRequired: boolean, nodeType: string) {
+        let line = ts.getLineAndCharacterOfPosition(this.sourceFile, start).line;
+        let firstNodeStart = firstNode.getStart(this.sourceFile);
+        let firstLine = ts.getLineAndCharacterOfPosition(this.sourceFile, firstNodeStart).line;
+        const comments = ts.getLeadingCommentRanges(this.sourceFile.text, firstNode.pos);
+
+        if (comments !== undefined) {
+            // check for blank lines between comments
+            for (let i = comments.length - 1; i >= 0; --i) {
+                const endLine = ts.getLineAndCharacterOfPosition(this.sourceFile, comments[i].end).line;
+                if (endLine < firstLine - 1) {
+                    // found empty line between nodes
+                    start = comments[i].end;
+                    firstLine = ts.getLineAndCharacterOfPosition(this.sourceFile, firstNodeStart).line;
+                    break;
+                }
+
+                firstNodeStart = comments[i].pos;
+                firstLine = ts.getLineAndCharacterOfPosition(this.sourceFile, firstNodeStart).line;
+            }
+        }
+
+        if (!lineRequired && line + 1 < firstLine) {
+            this.addFailure(
+                start + 1, ts.getPositionOfLineAndCharacter(this.sourceFile, firstLine, 0) - 1,
+                Rule.FAILURE_STRING_FACTORY(ViolationType.needless, nodeType)
+            );
+        } else if (lineRequired && line + 1 === firstLine) {
+            const pos = ts.getPositionOfLineAndCharacter(this.sourceFile, firstLine, 0);
+            this.addFailure(pos, pos, Rule.FAILURE_STRING_FACTORY(ViolationType.needed, nodeType));
+        }
+    }
+}

test/rules/newline-before-return/default/test.ts.lint

@@ -3,14 +3,14 @@ function foo(bar) {
         return;
     }
     return bar;
-    ~nil  [0]
+~nil  [0]
 }
 
 function foo(bar) {
     if (!bar) {
         var statement = '';
         return statement;
-        ~nil  [0]
+~nil  [0]
     }
 
     return bar;
@@ -21,7 +21,7 @@ function foo(bar) {
         return;
     }
     /* multi-line
-    ~nil  [0]
+~nil  [0]
     comment */
     return bar;
 }
@@ -30,12 +30,7 @@ var fn = () => null;
 function foo() {
     fn();
     return;
-    ~nil  [0]
-}
-
-function foo(fn) {
-    fn(); return;
-          ~nil [0]
+~nil  [0]
 }
 
 function foo() {
@@ -105,7 +100,7 @@ function foo() {
 function bar() {
     "some statement";
     //comment
-    ~nil [0]
+~nil [0]
     //comment
     //comment
     return;

test/rules/newline-before-return/default/test.tsx.lint

@@ -3,7 +3,7 @@ import * as React from 'react';
 <div>{ [].map((child: any) => {
     let i = 0;
     return <span />;
-    ~nil  [0]
+~nil  [0]
 }) }</div>
 
 <div>{ [].map((child: any) => {

test/rules/newline/all/test.ts.lint

@@ -0,0 +1,72 @@
+function foo(bar) {
+    // test
+
+    var statement = '';
+}
+
+function foo(bar) {
+    // test
+
+    // test
+    var statement = '';
+}
+
+function foo(bar) {
+    // test
+~nil [0]
+    // test
+    var statement = '';
+}
+
+function foo(bar) {
+    var statement = '';
+~nil [0]
+}
+
+function foo(bar) {
+    // test
+~nil [0]
+    var statement = '';
+}
+
+function foo(bar) {
+    // test
+
+    var statement = '';
+}
+
+function foo(bar) {
+
+
+    var statement = '';
+}
+
+function foo(bar) {
+    if (!bar) {
+~nil [0]
+        return;
+~nil [0]
+    }
+    return bar;
+~nil [1]
+}
+
+class Foo {}
+
+class Foo {
+
+}
+
+class Foo {
+    public bar = 'foo';
+~nil [2]
+}
+
+class Foo {
+
+    public bar = 'foo';
+}
+
+[0]: Missing blank line before first statement
+[1]: Missing blank line before return
+[2]: Missing blank line before first member

test/rules/newline/all/test.tsx.lint

@@ -0,0 +1,20 @@
+import * as React from 'react';
+
+<div>{ [].map((child: any) => {
+    let i = 0;
+~nil [0 % ('first statement')]
+    return <span />;
+~nil [0 % ('return')]
+}) }</div>
+
+<div>{ [].map((child: any) => {
+    return <span />;
+~nil [0 % ('first statement')]
+}) }</div>
+
+<div>{ [].map((child: any) =>
+    <span />;
+) }</div>
+
+
+[0]: Missing blank line before %s

test/rules/newline/all/tslint.json

@@ -0,0 +1,5 @@
+{
+    "rules": {
+        "newline": [true, {"class": "always", "block": "always", "return": "always", "functionBlock": "always"}]
+    }
+}