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

Commit

Permalink
Fix strict comparison false positive (#4786)
Browse files Browse the repository at this point in the history 
* add check for null, undefined and add test

* fix test
  • Loading branch information
@tanmoyopenroot
tanmoyopenroot authored and Josh Goldberg committed Jul 16, 2019
1 parent aa2af99 commit 1e48ae7
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 89 deletions.
17 changes: 15 additions & 2 deletions src/rules/strictComparisonsRule.ts
View file
Expand Up @@ -127,12 +127,19 @@ function walk(ctx: Lint.WalkContext<Options>, program: ts.Program) {

return ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
if (isBinaryExpression(node) && isComparisonOperator(node)) {
const isEquality = isEqualityOperator(node);
const leftType = checker.getTypeAtLocation(node.left);
const rightType = checker.getTypeAtLocation(node.right);

if (
(containsNullOrUndefined(leftType) || containsNullOrUndefined(rightType)) &&
isEquality
) {
return;
}

const leftKinds: TypeKind[] = getTypes(leftType);
const rightKinds: TypeKind[] = getTypes(rightType);

const operandKind = getStrictestComparableType(leftKinds, rightKinds);

if (operandKind === undefined) {
Expand All @@ -143,7 +150,6 @@ function walk(ctx: Lint.WalkContext<Options>, program: ts.Program) {
operandKind,
node.operatorToken.getText(),
);
const isEquality = isEqualityOperator(node);
if (isEquality) {
// Check !=, ==, !==, ===
switch (operandKind) {
Expand Down Expand Up @@ -186,6 +192,13 @@ function walk(ctx: Lint.WalkContext<Options>, program: ts.Program) {
});
}

function containsNullOrUndefined(type: ts.Type) {
return (
(type as ts.IntrinsicType).intrinsicName === "null" ||
(type as ts.IntrinsicType).intrinsicName === "undefined"
);
}

function getTypes(types: ts.Type): TypeKind[] {
// Compatibility for TypeScript pre-2.4, which used EnumLiteralType instead of LiteralType
const baseType = ((types as any) as { baseType: ts.LiteralType }).baseType;
Expand Down
79 changes: 56 additions & 23 deletions test/rules/strict-comparisons/allow-object-equal-comparison/test.ts.lint
View file
Expand Up @@ -8,48 +8,45 @@ if (2 === 1) {}
if (2 != 1) {}
if (2 !== 1) {}
if (2 > undefined) {}
~~~~~~~~~~~~~ [Cannot compare type 'number' to type 'undefined'.]
~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (undefined === 1) {}
~~~~~~~~~~~~~~~ [Cannot compare type 'undefined' to type 'number'.]
if (2 > undefined) {}
~~~~~~~~~~~~~ [Cannot compare type 'number' to type 'undefined'.]
if (undefined === 1) {}
~~~~~~~~~~~~~~~ [Cannot compare type 'undefined' to type 'number'.]
~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (true) {}
if (true > false) {}
~~~~~~~~~~~~ [Cannot use '>' comparator for type 'boolean'.]
~~~~~~~~~~~~ [CANNOT_USE % (">", "boolean")]
if (true < false) {}
~~~~~~~~~~~~ [Cannot use '<' comparator for type 'boolean'.]
~~~~~~~~~~~~ [CANNOT_USE % ("<", "boolean")]
if (true >= false) {}
~~~~~~~~~~~~~ [Cannot use '>=' comparator for type 'boolean'.]
~~~~~~~~~~~~~ [CANNOT_USE % (">=", "boolean")]
if (true <= false) {}
~~~~~~~~~~~~~ [Cannot use '<=' comparator for type 'boolean'.]
~~~~~~~~~~~~~ [CANNOT_USE % ("<=", "boolean")]
if (true == false) {}
if (true === false) {}
if (true != false) {}
if (true !== false) {}
if ('') {}
if ('' > '') {}
~~~~~~~ [Cannot use '>' comparator for type 'string'.]
~~~~~~~ [CANNOT_USE % (">", "string")]
if ('' < '') {}
~~~~~~~ [Cannot use '<' comparator for type 'string'.]
~~~~~~~ [CANNOT_USE % ("<", "string")]
if ('' >= '') {}
~~~~~~~~ [Cannot use '>=' comparator for type 'string'.]
~~~~~~~~ [CANNOT_USE % (">=", "string")]
if ('' <= '') {}
~~~~~~~~ [Cannot use '<=' comparator for type 'string'.]
~~~~~~~~ [CANNOT_USE % ("<=", "string")]
if ('' == '') {}
if ('' === '') {}
if ('' != '') {}
if ('' !== '') {}
if ({}) {}
if ({} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "object")]
if ({} < {}) {}
~~~~~~~ [Cannot use '<' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % ("<", "object")]
if ({} >= {}) {}
~~~~~~~~ [Cannot use '>=' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % (">=", "object")]
if ({} <= {}) {}
~~~~~~~~ [Cannot use '<=' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % ("<=", "object")]
if ({} == {}) {}
if ({} === {}) {}
if ({} != {}) {}
Expand All @@ -58,11 +55,11 @@ if ([] === []) {}

if (3 > 2 || 2 > 1 && true === true) {}
if ('' > '' || 2 > 1 || {} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'string'.]
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "string")]
~~~~~~~ [CANNOT_USE % (">", "object")]
if ('' > '' && 2 > 1 && {} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'string'.]
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "string")]
~~~~~~~ [CANNOT_USE % (">", "object")]

if ({} === null) {}
if (null === {}) {}
Expand Down Expand Up @@ -134,9 +131,9 @@ const g1: TestStringEnum = TestStringEnum.One
if (g1 === TestStringEnum.Two) {}
if (TestStringEnum.Two === g1) {}
if (g1 > TestStringEnum.Two) {}
~~~~~~~~~~~~~~~~~~~~~~~ [Cannot use '>' comparator for type 'string'.]
~~~~~~~~~~~~~~~~~~~~~~~ [CANNOT_USE % (">", "string")]
if (TestStringEnum.Two > g1) {}
~~~~~~~~~~~~~~~~~~~~~~~ [Cannot use '>' comparator for type 'string'.]
~~~~~~~~~~~~~~~~~~~~~~~ [CANNOT_USE % (">", "string")]
#endif

const h1: string | number = Math.random() > 0.5 ? 'text' : 5;
Expand All @@ -147,3 +144,39 @@ if (h2 > h1) {}
~~~~~~~ [Cannot use '>' comparator for type 'string'.]
if (h1 === h2) {}
if (h2 === h1) {}

if (undefined === null) {}
if (null !== undefined) {}

if (1 > null) {}
~~~~~~~~ [CANNOT_COMPARE % ("number", "null")]
if (null > 1) {}
~~~~~~~~ [CANNOT_COMPARE % ("null", "number")]
if (1 >= undefined) {}
~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (undefined >= 1) {}
~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("undefined", "number")]
if (undefined <= null) {}
~~~~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("undefined", "null")]
if (null >= undefined) {}
~~~~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("null", "undefined")]

const func = (param?: boolean): boolean => {
return param === undefined ? false : param;
}
const func = (param?: boolean): boolean => {
return param === null ? false : true;
}
const func = (param: number): number => {
return param + 1;
}
const func = (arg1: number, arg2: number): boolean => {
return arg1 === arg2;
}
const func = (param: string | null = null) => {
if (param !== null) {}
if (param !== undefined) {}
}

[CANNOT_USE]: Cannot use '%s' comparator for type '%s'.
[CANNOT_COMPARE]: Cannot compare type '%s' to type '%s'.
83 changes: 56 additions & 27 deletions test/rules/strict-comparisons/allow-string-order-comparison/test.ts.lint
View file
Expand Up @@ -8,22 +8,19 @@ if (2 === 1) {}
if (2 != 1) {}
if (2 !== 1) {}
if (2 > undefined) {}
~~~~~~~~~~~~~ [Cannot compare type 'number' to type 'undefined'.]
~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (undefined === 1) {}
~~~~~~~~~~~~~~~ [Cannot compare type 'undefined' to type 'number'.]
if (2 > undefined) {}
~~~~~~~~~~~~~ [Cannot compare type 'number' to type 'undefined'.]
if (undefined === 1) {}
~~~~~~~~~~~~~~~ [Cannot compare type 'undefined' to type 'number'.]
~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (true) {}
if (true > false) {}
~~~~~~~~~~~~ [Cannot use '>' comparator for type 'boolean'.]
~~~~~~~~~~~~ [CANNOT_USE % (">", "boolean")]
if (true < false) {}
~~~~~~~~~~~~ [Cannot use '<' comparator for type 'boolean'.]
~~~~~~~~~~~~ [CANNOT_USE % ("<", "boolean")]
if (true >= false) {}
~~~~~~~~~~~~~ [Cannot use '>=' comparator for type 'boolean'.]
~~~~~~~~~~~~~ [CANNOT_USE % (">=", "boolean")]
if (true <= false) {}
~~~~~~~~~~~~~ [Cannot use '<=' comparator for type 'boolean'.]
~~~~~~~~~~~~~ [CANNOT_USE % ("<=", "boolean")]
if (true == false) {}
if (true === false) {}
if (true != false) {}
Expand All @@ -39,42 +36,38 @@ if ('' != '') {}
if ('' !== '') {}
if ({}) {}
if ({} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "object")]
if ({} < {}) {}
~~~~~~~ [Cannot use '<' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % ("<", "object")]
if ({} >= {}) {}
~~~~~~~~ [Cannot use '>=' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % (">=", "object")]
if ({} <= {}) {}
~~~~~~~~ [Cannot use '<=' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % ("<=", "object")]
if ({} == {}) {}
~~~~~~~~ [Cannot use '==' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % ("==", "object")]
if ({} === {}) {}
~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
~~~~~~~~~ [CANNOT_USE % ("===", "object")]
if ({} != {}) {}
~~~~~~~~ [Cannot use '!=' comparator for type 'object'.]
~~~~~~~~ [CANNOT_USE % ("!=", "object")]
if ({} !== {}) {}
~~~~~~~~~ [Cannot use '!==' comparator for type 'object'.]
~~~~~~~~~ [CANNOT_USE % ("!==", "object")]
if ([] === []) {}
~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
~~~~~~~~~ [CANNOT_USE % ("===", "object")]

if (3 > 2 || 2 > 1 && true === true) {}
if ('' > '' || 2 > 1 || {} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "object")]
if ('' > '' && 2 > 1 && {} > {}) {}
~~~~~~~ [Cannot use '>' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % (">", "object")]

if ({} === null) {}
~~~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
if (null === {}) {}
~~~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
if ({} === undefined) {}
~~~~~~~~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
if (undefined === {}) {}
~~~~~~~~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]

function sameObject<T>(a: T, b: T): boolean {
return a === b;
~~~~~~~ [Cannot use '===' comparator for type 'object'.]
~~~~~~~ [CANNOT_USE % ("===", "object")]
}

function sameObject<T>(a: any, b: any): boolean {
Expand All @@ -100,9 +93,9 @@ const c1: myObject = {}
const c2: myObject = {}

if (c1 === c2) {}
~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
~~~~~~~~~ [CANNOT_USE % ("===", "object")]
if (c2 === c1) {}
~~~~~~~~~ [Cannot use '===' comparator for type 'object'.]
~~~~~~~~~ [CANNOT_USE % ("===", "object")]

const d1: any = 'string'
const d2: any = 2
Expand Down Expand Up @@ -147,3 +140,39 @@ if (h1 > h2) {}
if (h2 > h1) {}
if (h1 === h2) {}
if (h2 === h1) {}

if (undefined === null) {}
if (null !== undefined) {}

if (1 > null) {}
~~~~~~~~ [CANNOT_COMPARE % ("number", "null")]
if (null > 1) {}
~~~~~~~~ [CANNOT_COMPARE % ("null", "number")]
if (1 >= undefined) {}
~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("number", "undefined")]
if (undefined >= 1) {}
~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("undefined", "number")]
if (undefined <= null) {}
~~~~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("undefined", "null")]
if (null >= undefined) {}
~~~~~~~~~~~~~~~~~ [CANNOT_COMPARE % ("null", "undefined")]

const func = (param?: boolean): boolean => {
return param === undefined ? false : param;
}
const func = (param?: boolean): boolean => {
return param === null ? false : true;
}
const func = (param: number): number => {
return param + 1;
}
const func = (arg1: number, arg2: number): boolean => {
return arg1 === arg2;
}
const func = (param: string | null = null) => {
if (param !== null) {}
if (param !== undefined) {}
}

[CANNOT_USE]: Cannot use '%s' comparator for type '%s'.
[CANNOT_COMPARE]: Cannot compare type '%s' to type '%s'.

0 comments on commit 1e48ae7

Please sign in to comment.