Skip to content

Commit

Permalink
add whitespace rule for open braces (palantir#4068)
Browse files Browse the repository at this point in the history
  • Loading branch information
erikseulean authored and johnwiseheart committed Aug 23, 2018
1 parent 4702306 commit e766f51
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 19 deletions.
88 changes: 69 additions & 19 deletions src/rules/whitespaceRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const OPTION_TYPE = "check-type";
const OPTION_TYPECAST = "check-typecast";
const OPTION_TYPE_OPERATOR = "check-type-operator";
const OPTION_PREBLOCK = "check-preblock";
const OPTION_POSTBRACE = "check-postbrace";

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
Expand All @@ -50,23 +51,33 @@ export class Rule extends Lint.Rules.AbstractRule {
* \`"check-type"\` checks for whitespace before a variable type specification.
* \`"check-typecast"\` checks for whitespace between a typecast and its target.
* \`"check-type-operator"\` checks for whitespace between type operators \`|\` and \`&\`.
* \`"check-preblock"\` checks for whitespace before the opening brace of a block`,
* \`"check-preblock"\` checks for whitespace before the opening brace of a block.
* \`"check-postbrace"\` checks for whitespace after an opening brace.`,
options: {
type: "array",
items: {
type: "string",
enum: [
"check-branch", "check-decl", "check-operator", "check-module", "check-separator",
"check-rest-spread", "check-type", "check-typecast", "check-type-operator", "check-preblock",
],
"check-branch",
"check-decl",
"check-operator",
"check-module",
"check-separator",
"check-rest-spread",
"check-type",
"check-typecast",
"check-type-operator",
"check-preblock",
"check-postbrace"
]
},
minLength: 0,
maxLength: 10,
maxLength: 11
},
optionExamples: [[true, "check-branch", "check-operator", "check-typecast"]],
type: "style",
typescriptOnly: false,
hasFix: true,
hasFix: true
};

public static FAILURE_STRING_MISSING = "missing whitespace";
Expand All @@ -78,8 +89,19 @@ export class Rule extends Lint.Rules.AbstractRule {
}

type Options = Record<
"branch" | "decl" | "operator" | "module" | "separator" | "restSpread" | "type" | "typecast" | "typeOperator" | "preblock",
boolean>;
| "branch"
| "decl"
| "operator"
| "module"
| "separator"
| "restSpread"
| "type"
| "typecast"
| "typeOperator"
| "preblock"
| "postbrace",
boolean
>;

function parseOptions(ruleArguments: string[]): Options {
return {
Expand All @@ -93,6 +115,7 @@ function parseOptions(ruleArguments: string[]): Options {
typecast: has(OPTION_TYPECAST),
typeOperator: has(OPTION_TYPE_OPERATOR),
preblock: has(OPTION_PREBLOCK),
postbrace: has(OPTION_POSTBRACE)
};

function has(option: string): boolean {
Expand Down Expand Up @@ -244,10 +267,11 @@ function walk(ctx: Lint.WalkContext<Options>) {

let prevTokenShouldBeFollowedByWhitespace = false;
utils.forEachTokenWithTrivia(sourceFile, (_text, tokenKind, range, parent) => {
if (tokenKind === ts.SyntaxKind.WhitespaceTrivia ||
if (
tokenKind === ts.SyntaxKind.WhitespaceTrivia ||
tokenKind === ts.SyntaxKind.NewLineTrivia ||
tokenKind === ts.SyntaxKind.EndOfFileToken) {

tokenKind === ts.SyntaxKind.EndOfFileToken
) {
prevTokenShouldBeFollowedByWhitespace = false;
return;
} else if (prevTokenShouldBeFollowedByWhitespace) {
Expand Down Expand Up @@ -277,9 +301,11 @@ function walk(ctx: Lint.WalkContext<Options>) {
}

const nextPosition = range.pos + 1;
const semicolonInTrivialFor = parent.kind === ts.SyntaxKind.ForStatement &&
const semicolonInTrivialFor =
parent.kind === ts.SyntaxKind.ForStatement &&
nextPosition !== sourceFile.end &&
(sourceFile.text[nextPosition] === ";" || sourceFile.text[nextPosition] === ")");
(sourceFile.text[nextPosition] === ";" ||
sourceFile.text[nextPosition] === ")");

if (!semicolonInTrivialFor) {
prevTokenShouldBeFollowedByWhitespace = true;
Expand All @@ -295,9 +321,23 @@ function walk(ctx: Lint.WalkContext<Options>) {
prevTokenShouldBeFollowedByWhitespace = true;
}
break;
case ts.SyntaxKind.OpenBraceToken:
const nextPos = range.pos + 1;
if (
options.postbrace &&
(sourceFile.text[nextPos] !== " " &&
sourceFile.text[nextPos] !== "\r" &&
sourceFile.text[nextPos] !== "\t" &&
sourceFile.text[nextPos] !== "\n")
) {
addMissingWhitespaceErrorAt(nextPos);
}
break;
case ts.SyntaxKind.ImportKeyword:
if (parent.kind === ts.SyntaxKind.CallExpression &&
(parent as ts.CallExpression).expression.kind === ts.SyntaxKind.ImportKeyword) {
if (
parent.kind === ts.SyntaxKind.CallExpression &&
(parent as ts.CallExpression).expression.kind === ts.SyntaxKind.ImportKeyword
) {
return; // Don't check ImportCall
}
// falls through
Expand All @@ -314,7 +354,11 @@ function walk(ctx: Lint.WalkContext<Options>) {
return;
}

const equalsGreaterThanToken = utils.getChildOfKind(node, ts.SyntaxKind.EqualsGreaterThanToken, sourceFile);
const equalsGreaterThanToken = utils.getChildOfKind(
node,
ts.SyntaxKind.EqualsGreaterThanToken,
sourceFile
);
// condition so we don't crash if the arrow is somehow missing
if (equalsGreaterThanToken === undefined) {
return;
Expand All @@ -325,22 +369,28 @@ function walk(ctx: Lint.WalkContext<Options>) {
}

function checkForTrailingWhitespace(position: number, whiteSpacePos: number = position): void {
if (position !== sourceFile.end && !Lint.isWhiteSpace(sourceFile.text.charCodeAt(position))) {
if (
position !== sourceFile.end &&
!Lint.isWhiteSpace(sourceFile.text.charCodeAt(position))
) {
addMissingWhitespaceErrorAt(whiteSpacePos);
}
}

function addMissingWhitespaceErrorAt(position: number): void {
// TODO: this rule occasionally adds duplicate failures.
if (ctx.failures.some((f) => f.getStartPosition().getPosition() === position)) {
if (ctx.failures.some(f => f.getStartPosition().getPosition() === position)) {
return;
}
const fix = Lint.Replacement.appendText(position, " ");
ctx.addFailureAt(position, 1, Rule.FAILURE_STRING_MISSING, fix);
}

function checkForExcessiveWhitespace(position: number): void {
if (position !== sourceFile.end && Lint.isWhiteSpace(sourceFile.text.charCodeAt(position))) {
if (
position !== sourceFile.end &&
Lint.isWhiteSpace(sourceFile.text.charCodeAt(position))
) {
addInvalidWhitespaceErrorAt(position);
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/rules/whitespace/check-postbrace/test.ts.fix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function() { return 5;}

function() {
const something: number = 5;
}
6 changes: 6 additions & 0 deletions test/rules/whitespace/check-postbrace/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function() {return 5;}
~ [missing whitespace]

function() {
const something: number = 5;
}
5 changes: 5 additions & 0 deletions test/rules/whitespace/check-postbrace/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"whitespace": [true, "check-postbrace"]
}
}

0 comments on commit e766f51

Please sign in to comment.