diff --git a/src/enableDisableRules.ts b/src/enableDisableRules.ts index 64d4cdccfde..fa359bcc819 100644 --- a/src/enableDisableRules.ts +++ b/src/enableDisableRules.ts @@ -15,6 +15,9 @@ * limitations under the License. */ +// tslint:disable prefer-switch +// (waiting on https://github.com/palantir/tslint/pull/2369) + import * as utils from "tsutils"; import * as ts from "typescript"; import { IOptions } from "./language/rule/rule"; diff --git a/src/language/utils.ts b/src/language/utils.ts index 8d9daf86e75..7ca54db9fdd 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -339,44 +339,58 @@ export function forEachComment(node: ts.Node, cb: ForEachCommentCallback) { /** Exclude leading positions that would lead to scanning for trivia inside JsxText */ function canHaveLeadingTrivia(tokenKind: ts.SyntaxKind, parent: ts.Node): boolean { - if (tokenKind === ts.SyntaxKind.JsxText) { - return false; // there is no trivia before JsxText - } - if (tokenKind === ts.SyntaxKind.OpenBraceToken) { - // before a JsxExpression inside a JsxElement's body can only be other JsxChild, but no trivia - return parent.kind !== ts.SyntaxKind.JsxExpression || parent.parent!.kind !== ts.SyntaxKind.JsxElement; - } - if (tokenKind === ts.SyntaxKind.LessThanToken) { - if (parent.kind === ts.SyntaxKind.JsxClosingElement) { - return false; // would be inside the element body - } - if (parent.kind === ts.SyntaxKind.JsxOpeningElement || parent.kind === ts.SyntaxKind.JsxSelfClosingElement) { - // there can only be leading trivia if we are at the end of the top level element - return parent.parent!.parent!.kind !== ts.SyntaxKind.JsxElement; - } + switch (tokenKind) { + case ts.SyntaxKind.JsxText: + return false; // there is no trivia before JsxText + + case ts.SyntaxKind.OpenBraceToken: + // before a JsxExpression inside a JsxElement's body can only be other JsxChild, but no trivia + return parent.kind !== ts.SyntaxKind.JsxExpression || parent.parent!.kind !== ts.SyntaxKind.JsxElement; + + case ts.SyntaxKind.LessThanToken: + switch (parent.kind) { + case ts.SyntaxKind.JsxClosingElement: + return false; // would be inside the element body + case ts.SyntaxKind.JsxOpeningElement: + case ts.SyntaxKind.JsxSelfClosingElement: + // there can only be leading trivia if we are at the end of the top level element + return parent.parent!.parent!.kind !== ts.SyntaxKind.JsxElement; + default: + return true; + } + + default: + return true; } - return true; } /** Exclude trailing positions that would lead to scanning for trivia inside JsxText */ function canHaveTrailingTrivia(tokenKind: ts.SyntaxKind, parent: ts.Node): boolean { - if (tokenKind === ts.SyntaxKind.JsxText) { - return false; // there is no trivia after JsxText - } - if (tokenKind === ts.SyntaxKind.CloseBraceToken) { - // after a JsxExpression inside a JsxElement's body can only be other JsxChild, but no trivia - return parent.kind !== ts.SyntaxKind.JsxExpression || parent.parent!.kind !== ts.SyntaxKind.JsxElement; - } - if (tokenKind === ts.SyntaxKind.GreaterThanToken) { - if (parent.kind === ts.SyntaxKind.JsxOpeningElement) { - return false; // would be inside the element - } - if (parent.kind === ts.SyntaxKind.JsxClosingElement || parent.kind === ts.SyntaxKind.JsxSelfClosingElement) { - // there can only be trailing trivia if we are at the end of the top level element - return parent.parent!.parent!.kind !== ts.SyntaxKind.JsxElement; - } + switch (tokenKind) { + case ts.SyntaxKind.JsxText: + // there is no trivia after JsxText + return false; + + case ts.SyntaxKind.CloseBraceToken: + // after a JsxExpression inside a JsxElement's body can only be other JsxChild, but no trivia + return parent.kind !== ts.SyntaxKind.JsxExpression || parent.parent!.kind !== ts.SyntaxKind.JsxElement; + + case ts.SyntaxKind.GreaterThanToken: + switch (parent.kind) { + case ts.SyntaxKind.JsxOpeningElement: + return false; // would be inside the element + case ts.SyntaxKind.JsxClosingElement: + case ts.SyntaxKind.JsxSelfClosingElement: + // there can only be trailing trivia if we are at the end of the top level element + return parent.parent!.parent!.kind !== ts.SyntaxKind.JsxElement; + + default: + return true; + } + + default: + return true; } - return true; } /** diff --git a/src/rules/indentRule.ts b/src/rules/indentRule.ts index ef323f357a0..28522c358ef 100644 --- a/src/rules/indentRule.ts +++ b/src/rules/indentRule.ts @@ -128,11 +128,12 @@ class IndentWalker extends Lint.RuleWalker { } } - if (currentScannedType === ts.SyntaxKind.SingleLineCommentTrivia - || currentScannedType === ts.SyntaxKind.MultiLineCommentTrivia - || currentScannedType === ts.SyntaxKind.NewLineTrivia) { - // ignore lines that have comments before the first token - continue; + switch (currentScannedType) { + case ts.SyntaxKind.SingleLineCommentTrivia: + case ts.SyntaxKind.MultiLineCommentTrivia: + case ts.SyntaxKind.NewLineTrivia: + // ignore lines that have comments before the first token + continue; } if (fullLeadingWhitespace.match(this.regExp)) { diff --git a/src/rules/noUnnecessaryTypeAssertionRule.ts b/src/rules/noUnnecessaryTypeAssertionRule.ts index a6cbc49bc32..eaee8f84e5a 100644 --- a/src/rules/noUnnecessaryTypeAssertionRule.ts +++ b/src/rules/noUnnecessaryTypeAssertionRule.ts @@ -46,10 +46,11 @@ class Walker extends Lint.AbstractWalker { public walk(sourceFile: ts.SourceFile) { const cb = (node: ts.Node): void => { - if (node.kind === ts.SyntaxKind.TypeAssertionExpression || - node.kind === ts.SyntaxKind.NonNullExpression || - node.kind === ts.SyntaxKind.AsExpression) { - this.verifyCast(node as ts.TypeAssertion | ts.NonNullExpression | ts.AsExpression); + switch (node.kind) { + case ts.SyntaxKind.TypeAssertionExpression: + case ts.SyntaxKind.NonNullExpression: + case ts.SyntaxKind.AsExpression: + this.verifyCast(node as ts.TypeAssertion | ts.NonNullExpression | ts.AsExpression); } return ts.forEachChild(node, cb); diff --git a/src/rules/preferConstRule.ts b/src/rules/preferConstRule.ts index e6e86b40ab1..505b4169c77 100644 --- a/src/rules/preferConstRule.ts +++ b/src/rules/preferConstRule.ts @@ -201,33 +201,41 @@ class PreferConstWalker extends Lint.AbstractWalker { } private handleExpression(node: ts.Expression): void { - if (node.kind === ts.SyntaxKind.Identifier) { - this.scope.reassigned.add((node as ts.Identifier).text); - } else if (node.kind === ts.SyntaxKind.ParenthesizedExpression) { - return this.handleExpression((node as ts.ParenthesizedExpression).expression); - } else if (node.kind === ts.SyntaxKind.ArrayLiteralExpression) { - for (const element of (node as ts.ArrayLiteralExpression).elements) { - if (element.kind === ts.SyntaxKind.SpreadElement) { - this.handleExpression((element as ts.SpreadElement).expression); - } else { - this.handleExpression(element); - } - } - } else if (node.kind === ts.SyntaxKind.ObjectLiteralExpression) { - for (const property of (node as ts.ObjectLiteralExpression).properties) { - if (property.kind === ts.SyntaxKind.ShorthandPropertyAssignment) { - this.scope.reassigned.add(property.name.text); - } else if (property.kind === ts.SyntaxKind.SpreadAssignment) { - if (property.name !== undefined) { - this.scope.reassigned.add((property.name as ts.Identifier).text); + switch (node.kind) { + case ts.SyntaxKind.Identifier: + this.scope.reassigned.add((node as ts.Identifier).text); + break; + case ts.SyntaxKind.ParenthesizedExpression: + this.handleExpression((node as ts.ParenthesizedExpression).expression); + break; + case ts.SyntaxKind.ArrayLiteralExpression: + for (const element of (node as ts.ArrayLiteralExpression).elements) { + if (element.kind === ts.SyntaxKind.SpreadElement) { + this.handleExpression((element as ts.SpreadElement).expression); } else { - // handle `...(variable)` - this.handleExpression(property.expression!); + this.handleExpression(element); } - } else { - this.handleExpression((property as ts.PropertyAssignment).initializer); } - } + break; + case ts.SyntaxKind.ObjectLiteralExpression: + for (const property of (node as ts.ObjectLiteralExpression).properties) { + switch (property.kind) { + case ts.SyntaxKind.ShorthandPropertyAssignment: + this.scope.reassigned.add(property.name.text); + break; + case ts.SyntaxKind.SpreadAssignment: + if (property.name !== undefined) { + this.scope.reassigned.add((property.name as ts.Identifier).text); + } else { + // handle `...(variable)` + this.handleExpression(property.expression!); + } + break; + default: + this.handleExpression((property as ts.PropertyAssignment).initializer); + } + } + break; } } diff --git a/src/rules/preferSwitchRule.ts b/src/rules/preferSwitchRule.ts index c45b1b9f280..d1a06ff58f3 100644 --- a/src/rules/preferSwitchRule.ts +++ b/src/rules/preferSwitchRule.ts @@ -30,7 +30,7 @@ export class Rule extends Lint.Rules.AbstractRule { optionsDescription: Lint.Utils.dedent` An optional object with the property '${OPTION_MIN_CASES}'. This is the number cases needed before a switch statement is recommended. - Defaults to 2.`, + Defaults to 3.`, options: { type: "object", properties: { @@ -46,7 +46,7 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "Use a switch statement instead of using multiple '===' checks."; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - let minCases = 2; + let minCases = 3; if (this.ruleArguments.length) { const obj = this.ruleArguments[0] as { "min-cases": number }; minCases = obj[OPTION_MIN_CASES]; diff --git a/test/rules/prefer-switch/default/test.ts.lint b/test/rules/prefer-switch/default/test.ts.lint index 0bc40ea65ea..d9bb698628f 100644 --- a/test/rules/prefer-switch/default/test.ts.lint +++ b/test/rules/prefer-switch/default/test.ts.lint @@ -1,28 +1,6 @@ -if (x === 1) {} - ~~~~~~~ [0] -else if (x === 2) {} // Notice that we don't get *another* error here. -// Dangling `else` OK (this becomes `default:`) -else if (x === 3) {} -else {} +if (x === 1 || x === 2) {} -if (x === 1) {} else if (x === 2) {} - ~~~~~~~ [0] - -// Works with `||` -if (this === 1 || this === 2) {} - ~~~~~~~~~~~~~~~~~~~~~~~~ [0] - -// Default minumum of 2 cases. -if (x === 1) {} else {} - -// Different variables, no failure -if (x === 1) {} else if (y === 2) {} - -// Non-simple cases, no failure -if (x === f()) {} else if (x === g()) {} - -// Allow properties -if (x.y.z === a.b) else if (x.y.z === c.d) {} - ~~~~~~~~~~~~~ [0] +if (x === 1 || x === 2 || x === 3) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] [0]: Use a switch statement instead of using multiple '===' checks. diff --git a/test/rules/prefer-switch/min-cases-2/test.ts.lint b/test/rules/prefer-switch/min-cases-2/test.ts.lint new file mode 100644 index 00000000000..0bc40ea65ea --- /dev/null +++ b/test/rules/prefer-switch/min-cases-2/test.ts.lint @@ -0,0 +1,28 @@ +if (x === 1) {} + ~~~~~~~ [0] +else if (x === 2) {} // Notice that we don't get *another* error here. +// Dangling `else` OK (this becomes `default:`) +else if (x === 3) {} +else {} + +if (x === 1) {} else if (x === 2) {} + ~~~~~~~ [0] + +// Works with `||` +if (this === 1 || this === 2) {} + ~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +// Default minumum of 2 cases. +if (x === 1) {} else {} + +// Different variables, no failure +if (x === 1) {} else if (y === 2) {} + +// Non-simple cases, no failure +if (x === f()) {} else if (x === g()) {} + +// Allow properties +if (x.y.z === a.b) else if (x.y.z === c.d) {} + ~~~~~~~~~~~~~ [0] + +[0]: Use a switch statement instead of using multiple '===' checks. diff --git a/test/rules/prefer-switch/min-cases-3/tslint.json b/test/rules/prefer-switch/min-cases-2/tslint.json similarity index 71% rename from test/rules/prefer-switch/min-cases-3/tslint.json rename to test/rules/prefer-switch/min-cases-2/tslint.json index 0f9fb911fdb..e776dd404f0 100644 --- a/test/rules/prefer-switch/min-cases-3/tslint.json +++ b/test/rules/prefer-switch/min-cases-2/tslint.json @@ -1,7 +1,7 @@ { "rules": { "prefer-switch": [true, { - "min-cases": 3 + "min-cases": 2 }] } } diff --git a/test/rules/prefer-switch/min-cases-3/test.ts.lint b/test/rules/prefer-switch/min-cases-3/test.ts.lint deleted file mode 100644 index d9bb698628f..00000000000 --- a/test/rules/prefer-switch/min-cases-3/test.ts.lint +++ /dev/null @@ -1,6 +0,0 @@ -if (x === 1 || x === 2) {} - -if (x === 1 || x === 2 || x === 3) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - -[0]: Use a switch statement instead of using multiple '===' checks. diff --git a/tslint.json b/tslint.json index 81848236739..dad027e9dc7 100644 --- a/tslint.json +++ b/tslint.json @@ -57,7 +57,6 @@ "no-use-before-declare": false, "no-void-expression": false, "prefer-function-over-method": false, - "prefer-switch": false, "prefer-template": false, "restrict-plus-operands": false, "return-undefined": false, @@ -82,7 +81,7 @@ "public-before-private", "static-before-instance", "variables-before-functions" - ], + ], "no-console": { "options": ["log"] },