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

[bugfix][enhancement][rule-change] Get rid of check-type option in whitespace rule #4110

Closed
Closed
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
1 change: 0 additions & 1 deletion src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ export const rules = {
"check-operator",
"check-module",
"check-separator",
"check-type",
"check-typecast",
"check-preblock",
"check-type-operator",
Expand Down
77 changes: 50 additions & 27 deletions src/rules/whitespaceRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const OPTION_OPERATOR = "check-operator";
const OPTION_MODULE = "check-module";
const OPTION_SEPARATOR = "check-separator";
const OPTION_REST_SPREAD = "check-rest-spread";
const OPTION_TYPE = "check-type";
const OPTION_TYPECAST = "check-typecast";
const OPTION_TYPE_OPERATOR = "check-type-operator";
const OPTION_PREBLOCK = "check-preblock";
Expand All @@ -47,7 +46,6 @@ export class Rule extends Lint.Rules.AbstractRule {
* \`"check-module"\` checks for whitespace in import & export statements.
* \`"check-separator"\` checks for whitespace after separator tokens (\`,\`/\`;\`).
* \`"check-rest-spread"\` checks that there is no whitespace after rest/spread operator (\`...\`).
* \`"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`,
Expand All @@ -56,17 +54,24 @@ export class Rule extends Lint.Rules.AbstractRule {
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-typecast",
"check-type-operator",
"check-preblock"
]
},
minLength: 0,
maxLength: 10,
maxLength: 10
},
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 +83,17 @@ 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"
| "typecast"
| "typeOperator"
| "preblock",
boolean
>;

function parseOptions(ruleArguments: string[]): Options {
return {
Expand All @@ -89,10 +103,9 @@ function parseOptions(ruleArguments: string[]): Options {
module: has(OPTION_MODULE),
separator: has(OPTION_SEPARATOR),
restSpread: has(OPTION_REST_SPREAD),
type: has(OPTION_TYPE),
typecast: has(OPTION_TYPECAST),
typeOperator: has(OPTION_TYPE_OPERATOR),
preblock: has(OPTION_PREBLOCK),
preblock: has(OPTION_PREBLOCK)
};

function has(option: string): boolean {
Expand Down Expand Up @@ -244,10 +257,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 +291,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 @@ -290,14 +306,11 @@ function walk(ctx: Lint.WalkContext<Options>) {
prevTokenShouldBeFollowedByWhitespace = true;
}
break;
case ts.SyntaxKind.ColonToken:
if (options.type) {
prevTokenShouldBeFollowedByWhitespace = true;
}
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 +327,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 +342,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
6 changes: 0 additions & 6 deletions test/rules/_integration/react/test.tsx.lint
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ interface IFooProps extends React.Props<FooComponent> {
fooProp: string;
}

interface IFooState {
bar:string[]; // whitespace failure
~ [missing whitespace]
}

export class FooComponent extends React.Component<IFooProps, IFooState> {
public state = {
bar: [] as string[]
Expand All @@ -31,7 +26,6 @@ export class FooComponent extends React.Component<IFooProps, IFooState> {
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
{this.state.bar.map((s) => <span>{s}</span>)}
<BazComponent someProp={123} anotherProp={456} />
</div>
Expand Down
38 changes: 19 additions & 19 deletions test/rules/_integration/react/tslint.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
{
"rules": {
"curly": true,
"eofline": true,
"indent": [true, "spaces"],
"max-line-length": [true, 120],
"no-bitwise": true,
"no-unused-expression": true,
"quotemark": [true, "double"],
"semicolon": [true, "always"],
"whitespace": [true,
"check-branch",
"check-decl",
"check-operator",
"check-module",
"check-separator",
"check-type",
"check-typecast"
]
}
"rules": {
"curly": true,
"eofline": true,
"indent": [true, "spaces"],
"max-line-length": [true, 120],
"no-bitwise": true,
"no-unused-expression": true,
"quotemark": [true, "double"],
"semicolon": [true, "always"],
"whitespace": [
true,
"check-branch",
"check-decl",
"check-operator",
"check-module",
"check-separator",
"check-typecast"
]
}
}
15 changes: 3 additions & 12 deletions test/rules/whitespace/all/test.ts.fix
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,15 @@ import ast = AST;
module M {
export var ast = AST;

var x: number;

var y = (x === 10) ? 1 : 2;
var y = (x === 10) ? 1 :2;

var zz = (y === 4);

var z = y;

var a, b;

switch (x) {
case 1: break;
default: break;
}

for (x = 1; x < 2; ++x) {
goto: console.log("hi");
}

for (x = 2; x--;) {}
Expand Down Expand Up @@ -56,10 +48,9 @@ var test = `
<div class="repeat"
`;

var withinTemplateExpression = `${name === "something" ? "foo" : "bar"}`;
var withinTemplateExpression = `${name === "something" ? "foo" :"bar"}`;
var withinTemplateExpression = `${name === "something" ? "foo" : "bar"}`;

var objectInsideTemplateExpression = `${({foo: "bar"}).foo}`;
var objectInsideTemplateExpression = `${({foo: "bar"}).foo}`;

import { importA } from "libA";
Expand Down Expand Up @@ -92,7 +83,7 @@ function foorbar()

if () {
//
} else {}
}

if ()
{}
Expand Down
20 changes: 1 addition & 19 deletions test/rules/whitespace/all/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ module M {
~ [missing whitespace]
~ [missing whitespace]

var x:number;
~ [missing whitespace]

var y = (x === 10)?1:2;
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]

var zz = (y===4);
~ [missing whitespace]
Expand All @@ -26,21 +22,11 @@ module M {
var a,b;
~ [missing whitespace]

switch(x) {
~ [missing whitespace]
case 1:break;
~ [missing whitespace]
default:break;
~ [missing whitespace]
}

for(x = 1;x <2; ++x){
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
goto:console.log("hi");
~ [missing whitespace]
}

for (x = 2; x--;) {}
Expand Down Expand Up @@ -97,11 +83,8 @@ var withinTemplateExpression = `${name==="something"?"foo":"bar"}`;
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
~ [missing whitespace]
var withinTemplateExpression = `${name === "something" ? "foo" : "bar"}`;

var objectInsideTemplateExpression = `${({foo:"bar"}).foo}`;
~ [missing whitespace]
var objectInsideTemplateExpression = `${({foo: "bar"}).foo}`;

import { importA } from "libA";
Expand Down Expand Up @@ -148,8 +131,7 @@ function foorbar()
if (){
~ [missing whitespace]
//
} else{}
~ [missing whitespace]
}

if ()
{}
Expand Down
1 change: 0 additions & 1 deletion test/rules/whitespace/all/tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"check-preblock",
"check-separator",
"check-rest-spread",
"check-type",
"check-typecast",
"check-type-operator"
]
Expand Down