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

Fix strict comparison false positive #4786

Merged
merged 2 commits into from Jul 16, 2019
Merged

Fix strict comparison false positive #4786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbb9511
add check for null, undefined and add test
tanmoyopenroot Jul 6, 2019
ed6938e
fix test
tanmoyopenroot Jul 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/rules/strictComparisonsRule.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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'.