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

Add no-boolean-literal-compare rule #2013

Merged
merged 2 commits into from
Jan 11, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions docs/_data/rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@
"typescriptOnly": false
},
{
"ruleName": "no-boolean-compare",
"ruleName": "no-boolean-literal-compare",
"description": "Warns on comparison to a boolean literal, as in `x === true`.",
"hasFix": true,
"optionsDescription": "Not configurable.",
Expand All @@ -591,7 +591,8 @@
"true"
],
"type": "style",
"typescriptOnly": true
"typescriptOnly": true,
"requiresTypeInfo": true
},
{
"ruleName": "no-conditional-assignment",
Expand Down
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: 0 additions & 4 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ export class Replacement {
return replacements.reduce((text, r) => r.apply(text), content);
}

public static deleteFromTo(start: number, end: number): Replacement {
return new Replacement(start, end - start, "");
}

constructor(private innerStart: number, private innerLength: number, private innerText: string) {
}

Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ 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-compare",
ruleName: "no-boolean-literal-compare",
description: "Warns on comparison to a boolean literal, as in `x === true`.",
hasFix: true,
optionsDescription: "Not configurable.",
Expand Down Expand Up @@ -62,17 +62,32 @@ class Walker extends Lint.ProgramAwareRuleWalker {
}

const deleted = node.left === expression
? Lint.Replacement.deleteFromTo(node.left.end, node.end)
: Lint.Replacement.deleteFromTo(node.getStart(), node.right.getStart());
? this.deleteFromTo(node.left.end, node.end)
: this.deleteFromTo(node.getStart(), node.right.getStart());
const replacements = [deleted];
if (negate) {
replacements.push(this.appendText(node.getStart(), "!"));
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a more reliable way to check?

ternary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a better way to do this, unfortunately.
A ternary operator wouldn't happen here since it === binds more tightly, so the ternary would have to be parenthesized already anyway.

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);
Expand All @@ -93,9 +108,11 @@ function deconstructComparison(node: ts.BinaryExpression): { negate: boolean, ex

function operatorKind(operatorToken: ts.BinaryOperatorToken): boolean | undefined {
switch (operatorToken.kind) {
case ts.SyntaxKind.EqualsEqualsToken: case ts.SyntaxKind.EqualsEqualsEqualsToken:
case ts.SyntaxKind.EqualsEqualsToken:
case ts.SyntaxKind.EqualsEqualsEqualsToken:
return true;
case ts.SyntaxKind.ExclamationEqualsToken: case ts.SyntaxKind.ExclamationEqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsToken:
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
return false;
default:
return undefined;
Expand Down
19 changes: 0 additions & 19 deletions test/rules/no-boolean-compare/test.ts.fix

This file was deleted.

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;

Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,20 @@ x == true;
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.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"typeCheck": true
},
"rules": {
"no-boolean-compare": true
"no-boolean-literal-compare": true
}
}