Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Fix handling of comments in newline-before-return #2321

Merged
merged 1 commit into from
Mar 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 33 additions & 28 deletions src/rules/newlineBeforeReturnRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import * as utils from "tsutils";
import { getPreviousStatement } from "tsutils";
import * as ts from "typescript";
import * as Lint from "../index";

Expand All @@ -33,46 +33,51 @@ export class Rule extends Lint.Rules.AbstractRule {
};
/* tslint:enable:object-literal-sort-keys */

public static FAILURE_STRING_FACTORY() {
return "Missing blank line before return";
};
public static FAILURE_STRING = "Missing blank line before return";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new NewlineBeforeReturnWalker(sourceFile, this.getOptions()));
return this.applyWithWalker(new NewlineBeforeReturnWalker(sourceFile, this.ruleName, undefined));
}
}

class NewlineBeforeReturnWalker extends Lint.RuleWalker {

public visitReturnStatement(node: ts.ReturnStatement) {
super.visitReturnStatement(node);

const parent = node.parent!;
if (!utils.isBlockLike(parent)) {
// `node` is the only statement within this "block scope". No need to do any further validation.
return;
}
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);
}

const index = parent.statements.indexOf(node as ts.Statement);
if (index === 0) {
// First element in the block so no need to check for the blank line.
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();
const comments = ts.getLeadingCommentRanges(this.getSourceFile().text, node.getFullStart());
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) {
// In case the return statement is preceded by a comment, we use comments start as the starting position
start = comments[0].pos;
// 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 lc = this.getLineAndCharacterOfPosition(start);

const prev = parent.statements[index - 1];
const prevLine = this.getLineAndCharacterOfPosition(prev.getEnd()).line;
const prevLine = ts.getLineAndCharacterOfPosition(this.sourceFile, prev.end).line;

if (prevLine >= lc.line - 1) {
if (prevLine >= line - 1) {
// Previous statement is on the same or previous line
this.addFailureFromStartToEnd(start, start, Rule.FAILURE_STRING_FACTORY());
this.addFailure(start, start, Rule.FAILURE_STRING);
}
}
}
24 changes: 24 additions & 0 deletions test/rules/newline-before-return/default/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,28 @@ function foo() {
return;
}

function test() {
console.log("Any statement");
// Any comment

return;
}

function foo() {
fn();
// comment

// comment
return;
}

function bar() {
"some statement";
//comment
~nil [0]
//comment
//comment
return;
}

[0]: Missing blank line before return