diff --git a/docs/_data/rules.json b/docs/_data/rules.json index fa358c66214..7e799d38986 100644 --- a/docs/_data/rules.json +++ b/docs/_data/rules.json @@ -139,22 +139,44 @@ "ruleName": "comment-format", "description": "Enforces formatting rules for single-line comments.", "rationale": "Helps maintain a consistent, readable style in your codebase.", - "optionsDescription": "\nThree arguments may be optionally provided:\n\n* `\"check-space\"` requires that all single-line comments must begin with a space, as in `// comment`\n * note that comments starting with `///` are also allowed, for things such as `///`\n* `\"check-lowercase\"` requires that the first non-whitespace character of a comment must be lowercase, if applicable.\n* `\"check-uppercase\"` requires that the first non-whitespace character of a comment must be uppercase, if applicable.", + "optionsDescription": "\nThree arguments may be optionally provided:\n\n* `\"check-space\"` requires that all single-line comments must begin with a space, as in `// comment`\n * note that comments starting with `///` are also allowed, for things such as `///`\n* `\"check-lowercase\"` requires that the first non-whitespace character of a comment must be lowercase, if applicable.\n* `\"check-uppercase\"` requires that the first non-whitespace character of a comment must be uppercase, if applicable.\n\nExceptions to `\"check-lowercase\"` or `\"check-uppercase\"` can be managed with object that may be passed as last argument.\n\nOne of two options can be provided in this object:\n \n * `\"ignoreWords\"` - array of strings - words that will be ignored at the beginning of the comment.\n * `\"ignorePattern\"` - string - RegExp pattern that will be ignored at the beginning of the comment.\n", "options": { "type": "array", "items": { - "type": "string", - "enum": [ - "check-space", - "check-lowercase", - "check-uppercase" + "anyOf": [ + { + "type": "string", + "enum": [ + "check-space", + "check-lowercase", + "check-uppercase" + ] + }, + { + "type": "object", + "properties": { + "ignoreWords": { + "type": "array", + "items": { + "type": "string" + } + }, + "ignorePattern": { + "type": "string" + } + }, + "minProperties": 1, + "maxProperties": 1 + } ] }, "minLength": 1, - "maxLength": 3 + "maxLength": 4 }, "optionExamples": [ - "[true, \"check-space\", \"check-lowercase\"]" + "[true, \"check-space\", \"check-uppercase\"]", + "[true, \"check-lowercase\", {\"ignoreWords\": [\"TODO\", \"HACK\"]}]", + "[true, \"check-lowercase\", {\"ignorePattern\": \"STD\\w{2,3}\\b\"}]" ], "type": "style", "typescriptOnly": false @@ -559,6 +581,19 @@ "type": "functionality", "typescriptOnly": false }, + { + "ruleName": "no-boolean-literal-compare", + "description": "Warns on comparison to a boolean literal, as in `x === true`.", + "hasFix": true, + "optionsDescription": "Not configurable.", + "options": null, + "optionExamples": [ + "true" + ], + "type": "style", + "typescriptOnly": true, + "requiresTypeInfo": true + }, { "ruleName": "no-conditional-assignment", "description": "Disallows any type of assignment in conditionals.", diff --git a/docs/rules/comment-format/index.html b/docs/rules/comment-format/index.html index cbc4b7813a0..65450916bb9 100644 --- a/docs/rules/comment-format/index.html +++ b/docs/rules/comment-format/index.html @@ -2,7 +2,7 @@ ruleName: comment-format description: Enforces formatting rules for single-line comments. rationale: 'Helps maintain a consistent, readable style in your codebase.' -optionsDescription: |- +optionsDescription: | Three arguments may be optionally provided: @@ -10,18 +10,38 @@ * note that comments starting with `///` are also allowed, for things such as `///` * `"check-lowercase"` requires that the first non-whitespace character of a comment must be lowercase, if applicable. * `"check-uppercase"` requires that the first non-whitespace character of a comment must be uppercase, if applicable. + + Exceptions to `"check-lowercase"` or `"check-uppercase"` can be managed with object that may be passed as last argument. + + One of two options can be provided in this object: + + * `"ignoreWords"` - array of strings - words that will be ignored at the beginning of the comment. + * `"ignorePattern"` - string - RegExp pattern that will be ignored at the beginning of the comment. options: type: array items: - type: string - enum: - - check-space - - check-lowercase - - check-uppercase + anyOf: + - type: string + enum: + - check-space + - check-lowercase + - check-uppercase + - type: object + properties: + ignoreWords: + type: array + items: + type: string + ignorePattern: + type: string + minProperties: 1 + maxProperties: 1 minLength: 1 - maxLength: 3 + maxLength: 4 optionExamples: - - '[true, "check-space", "check-lowercase"]' + - '[true, "check-space", "check-uppercase"]' + - '[true, "check-lowercase", {"ignoreWords": ["TODO", "HACK"]}]' + - '[true, "check-lowercase", {"ignorePattern": "STD\w{2,3}\b"}]' type: style typescriptOnly: false layout: rule @@ -30,14 +50,34 @@ { "type": "array", "items": { - "type": "string", - "enum": [ - "check-space", - "check-lowercase", - "check-uppercase" + "anyOf": [ + { + "type": "string", + "enum": [ + "check-space", + "check-lowercase", + "check-uppercase" + ] + }, + { + "type": "object", + "properties": { + "ignoreWords": { + "type": "array", + "items": { + "type": "string" + } + }, + "ignorePattern": { + "type": "string" + } + }, + "minProperties": 1, + "maxProperties": 1 + } ] }, "minLength": 1, - "maxLength": 3 + "maxLength": 4 } --- \ No newline at end of file diff --git a/docs/rules/no-boolean-compare/index.html b/docs/rules/no-boolean-compare/index.html new file mode 100644 index 00000000000..1c1a89423b2 --- /dev/null +++ b/docs/rules/no-boolean-compare/index.html @@ -0,0 +1,14 @@ +--- +ruleName: no-boolean-compare +description: 'Warns on comparison to a boolean literal, as in `x === true`.' +hasFix: true +optionsDescription: Not configurable. +options: null +optionExamples: + - 'true' +type: style +typescriptOnly: true +layout: rule +title: 'Rule: no-boolean-compare' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/docs/rules/no-boolean-literal-compare/index.html b/docs/rules/no-boolean-literal-compare/index.html new file mode 100644 index 00000000000..74860ef768f --- /dev/null +++ b/docs/rules/no-boolean-literal-compare/index.html @@ -0,0 +1,15 @@ +--- +ruleName: no-boolean-literal-compare +description: 'Warns on comparison to a boolean literal, as in `x === true`.' +hasFix: true +optionsDescription: Not configurable. +options: null +optionExamples: + - 'true' +type: style +typescriptOnly: true +requiresTypeInfo: true +layout: rule +title: 'Rule: no-boolean-literal-compare' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 83d469fac8e..40bc173e482 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -105,6 +105,10 @@ export class RuleWalker extends SyntaxWalker implements IWalker { return this.createReplacement(start, length, ""); } + public deleteFromTo(start: number, end: number): Replacement { + return this.createReplacement(start, end - start, ""); + } + public getRuleName(): string { return this.ruleName; } diff --git a/src/rules/noBooleanLiteralCompareRule.ts b/src/rules/noBooleanLiteralCompareRule.ts new file mode 100644 index 00000000000..73a776c3053 --- /dev/null +++ b/src/rules/noBooleanLiteralCompareRule.ts @@ -0,0 +1,131 @@ +/** + * @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 * as ts from "typescript"; + +import * as Lint from "../index"; + +export class Rule extends Lint.Rules.TypedRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "no-boolean-literal-compare", + description: "Warns on comparison to a boolean literal, as in `x === true`.", + hasFix: true, + optionsDescription: "Not configurable.", + options: null, + optionExamples: ["true"], + type: "style", + typescriptOnly: true, + requiresTypeInfo: true, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING(negate: boolean) { + return `This expression is unnecessarily compared to a boolean. Just ${negate ? "negate it" : "use it directly"}.`; + } + + public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), langSvc.getProgram())); + } +} + +class Walker extends Lint.ProgramAwareRuleWalker { + public visitBinaryExpression(node: ts.BinaryExpression) { + this.check(node); + super.visitBinaryExpression(node); + } + + private check(node: ts.BinaryExpression) { + const comparison = deconstructComparison(node); + if (comparison === undefined) { + return; + } + + const { negate, expression } = comparison; + const type = this.getTypeChecker().getTypeAtLocation(expression); + if (!Lint.isTypeFlagSet(type, ts.TypeFlags.Boolean)) { + return; + } + + const deleted = node.left === expression + ? this.deleteFromTo(node.left.end, node.end) + : this.deleteFromTo(node.getStart(), node.right.getStart()); + const replacements = [deleted]; + if (negate) { + if (needsParenthesesForNegate(expression)) { + replacements.push(this.appendText(node.getStart(), "!(")); + replacements.push(this.appendText(node.getEnd(), ")")); + } else { + replacements.push(this.appendText(node.getStart(), "!")); + } + } + + this.addFailureAtNode(expression, Rule.FAILURE_STRING(negate), this.createFix(...replacements)); + } +} + +function needsParenthesesForNegate(node: ts.Expression) { + switch (node.kind) { + case ts.SyntaxKind.AsExpression: + case ts.SyntaxKind.BinaryExpression: + return true; + default: + return false; + } +} + +function deconstructComparison(node: ts.BinaryExpression): { negate: boolean, expression: ts.Expression } | undefined { + const { left, operatorToken, right } = node; + const operator = operatorKind(operatorToken); + if (operator === undefined) { + return undefined; + } + + const leftValue = booleanFromExpression(left); + if (leftValue !== undefined) { + return { negate: leftValue !== operator, expression: right }; + } + const rightValue = booleanFromExpression(right); + if (rightValue !== undefined) { + return { negate: rightValue !== operator, expression: left }; + } + return undefined; +} + +function operatorKind(operatorToken: ts.BinaryOperatorToken): boolean | undefined { + switch (operatorToken.kind) { + case ts.SyntaxKind.EqualsEqualsToken: + case ts.SyntaxKind.EqualsEqualsEqualsToken: + return true; + case ts.SyntaxKind.ExclamationEqualsToken: + case ts.SyntaxKind.ExclamationEqualsEqualsToken: + return false; + default: + return undefined; + } +} + +function booleanFromExpression(node: ts.Expression): boolean | undefined { + switch (node.kind) { + case ts.SyntaxKind.TrueKeyword: + return true; + case ts.SyntaxKind.FalseKeyword: + return false; + default: + return undefined; + } +} diff --git a/test/rules/no-boolean-literal-compare/test.ts.fix b/test/rules/no-boolean-literal-compare/test.ts.fix new file mode 100644 index 00000000000..30bb48dc97e --- /dev/null +++ b/test/rules/no-boolean-literal-compare/test.ts.fix @@ -0,0 +1,30 @@ +declare const x: boolean; + +x; +x; + +!x; +!x; + +!x; +!x; + +x; +x; + +x; + +declare const y: boolean | undefined; +y === true; + +declare function f(): boolean; +!f(); + +declare const a: number, b: number; + +!(a as any as boolean); + +!(a < b); + +!!x; + diff --git a/test/rules/no-boolean-literal-compare/test.ts.lint b/test/rules/no-boolean-literal-compare/test.ts.lint new file mode 100644 index 00000000000..9e54c2c6e92 --- /dev/null +++ b/test/rules/no-boolean-literal-compare/test.ts.lint @@ -0,0 +1,45 @@ +declare const x: boolean; + +x === true; +~ [T] +true === x; + ~ [T] + +x === false; +~ [F] +false === x; + ~ [F] + +x !== true; +~ [F] +true !== x; + ~ [F] + +x !== false; +~ [T] +false !== x; + ~ [T] + +x == true; +~ [T] + +declare const y: boolean | undefined; +y === true; + +declare function f(): boolean; +f() === false; +~~~ [F] + +declare const a: number, b: number; + +a as any as boolean === false; +~~~~~~~~~~~~~~~~~~~ [F] + +a < b === false; +~~~~~ [F] + +!x === false; +~~ [F] + +[T]: This expression is unnecessarily compared to a boolean. Just use it directly. +[F]: This expression is unnecessarily compared to a boolean. Just negate it. diff --git a/test/rules/no-boolean-literal-compare/tsconfig.json b/test/rules/no-boolean-literal-compare/tsconfig.json new file mode 100644 index 00000000000..1cc3bc85ee9 --- /dev/null +++ b/test/rules/no-boolean-literal-compare/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "strictNullChecks": true + } +} \ No newline at end of file diff --git a/test/rules/no-boolean-literal-compare/tslint.json b/test/rules/no-boolean-literal-compare/tslint.json new file mode 100644 index 00000000000..79f31adfee3 --- /dev/null +++ b/test/rules/no-boolean-literal-compare/tslint.json @@ -0,0 +1,8 @@ +{ + "linterOptions": { + "typeCheck": true + }, + "rules": { + "no-boolean-literal-compare": true + } +} \ No newline at end of file