Skip to content

Commit

Permalink
Add no-boolean-literal-compare rule (palantir#2013)
Browse files Browse the repository at this point in the history
  • Loading branch information
andy-hanson authored and nchen63 committed Jan 11, 2017
1 parent 25de3f9 commit a6d3384
Show file tree
Hide file tree
Showing 10 changed files with 349 additions and 22 deletions.
51 changes: 43 additions & 8 deletions docs/_data/rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 `///<reference>`\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 `///<reference>`\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
Expand Down Expand Up @@ -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.",
Expand Down
68 changes: 54 additions & 14 deletions docs/rules/comment-format/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,46 @@
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:

* `"check-space"` requires that all single-line comments must begin with a space, as in `// comment`
* note that comments starting with `///` are also allowed, for things such as `///<reference>`
* `"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
Expand All @@ -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
}
---
14 changes: 14 additions & 0 deletions docs/rules/no-boolean-compare/index.html
Original file line number Diff line number Diff line change
@@ -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'
---
15 changes: 15 additions & 0 deletions docs/rules/no-boolean-literal-compare/index.html
Original file line number Diff line number Diff line change
@@ -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'
---
4 changes: 4 additions & 0 deletions src/language/walker/ruleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
131 changes: 131 additions & 0 deletions src/rules/noBooleanLiteralCompareRule.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
30 changes: 30 additions & 0 deletions test/rules/no-boolean-literal-compare/test.ts.fix
Original file line number Diff line number Diff line change
@@ -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;

Loading

0 comments on commit a6d3384

Please sign in to comment.