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

Add strict-comparisons Rule #4519

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
96a0c36
add noAsyncWithoutAwait rule
eranshabi Jun 6, 2018
5b4e3c3
fix lint
eranshabi Jun 8, 2018
7f6d5c7
code review updates
eranshabi Jun 17, 2018
5abe892
Merge remote-tracking branch 'upstream/master'
eranshabi Jun 17, 2018
f4680ef
allow return as well as await
eranshabi Jul 6, 2018
d28b2d3
remove unneeded lint ignore
eranshabi Jul 6, 2018
337cd30
Merge remote-tracking branch 'upstream/master'
eranshabi Jul 6, 2018
7e11840
Merge remote-tracking branch 'upstream/master'
eranshabi Aug 2, 2018
4f51780
Merge remote-tracking branch 'upstream/master'
eranshabi Nov 19, 2018
bfa4c91
improve rationale & fix performance issue
eranshabi Nov 19, 2018
37aa7b5
fixes according to review
eranshabi Dec 26, 2018
0d58f68
finish fixing according to review
eranshabi Dec 27, 2018
3c74c2e
Add `no-object-comparison` rule
AndreasGassmann Feb 15, 2019
6b2e2a6
Revert styling
AndreasGassmann Feb 15, 2019
a974708
Fix code styling
AndreasGassmann Feb 15, 2019
51686eb
Fix enum error
AndreasGassmann Feb 15, 2019
a595f5b
Rename object-comparison rule
AndreasGassmann Feb 17, 2019
2d974eb
Rename test folder of object-comparison rule
AndreasGassmann Feb 17, 2019
76360df
Change rule argument to object
AndreasGassmann Feb 17, 2019
a505bb1
Refactor object-comparison rule
AndreasGassmann Feb 17, 2019
a79c3af
Add new option to object-comparison rule
AndreasGassmann Feb 18, 2019
e4818f1
Add more test cases
AndreasGassmann Feb 20, 2019
f2dd93c
Handle numeric and string enums
AndreasGassmann Feb 21, 2019
5c41c38
Fix corner cases with undefined
AndreasGassmann Feb 23, 2019
d4b57b7
Make rule compatible with ts 2.1
AndreasGassmann Feb 23, 2019
62344ad
feat(strict-comparisons): rename rule, fix error message, add example
AndreasGassmann Mar 14, 2019
6a7920d
feat(strict-comparisons): update examples url
AndreasGassmann Mar 21, 2019
96f39ed
Initial feedback cleanups
Jun 16, 2019
7a782a5
Merge remote-tracking branch 'upstream/master'
Jun 16, 2019
65e10f5
Refactored to walk function
Jun 16, 2019
201e4f2
Merge branch 'master' of https://github.com/palantir/tslint
Jun 16, 2019
90a38c7
Merge branch 'master' of https://github.com/palantir/tslint
Jun 16, 2019
cc27b12
Disabled strict-comparisons in tslint.json
Jun 16, 2019
fa16950
Delete unused objectComparisonRule.ts
Jun 16, 2019
6d47a43
Merge branch 'master' into feat/no-object-comparison
Jun 16, 2019
0551984
Un-formatted src/configs/all.ts
Jun 16, 2019
dfbd4ac
Un-formatted tslint.json
Jun 16, 2019
251a22d
Excluded string enum complaints from TS 2.1
Jun 16, 2019
1d28282
Formatted strictComparisonsRule.ts
Jun 16, 2019
7520bd4
Some more TS 2.1 disabling
Jun 16, 2019
c0fd4c7
I'm here all night folks
Jun 16, 2019
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
6 changes: 6 additions & 0 deletions src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ export const rules = {
"function-constructor": true,
// "import-blacklist": no sensible default
"label-position": true,
"object-comparison": {
options: {
"allow-object-equal-comparison": true,
"allow-string-order-comparison": true,
},
},
"no-arg": true,
"no-bitwise": true,
"no-conditional-assignment": true,
Expand Down
55 changes: 55 additions & 0 deletions src/rules/code-examples/objectComparison.examples.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* @license
* Copyright 2019 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 Lint from "../../index";

// tslint:disable: object-literal-sort-keys
export const codeExamples = [
{
description: "Disallows usage of comparison operators with non-primitive types.",
config: Lint.Utils.dedent`
"rules": { "object-comparison": true }
`,
pass: Lint.Utils.dedent`
const object1 = {};
const object2 = {};
if (isEqual(object1, object2)) {}
`,
fail: Lint.Utils.dedent`
const object1 = {};
const object2 = {};
if (object1 === object2) {}
`,
},
{
AndreasGassmann marked this conversation as resolved.
Show resolved Hide resolved
description:
"Allows equality operators to be used with non-primitive types, while still disallowing the use of greater than and less than.",
config: Lint.Utils.dedent`
"rules": { "object-comparison": [true, { "allow-object-equal-comparison": true }] }
`,
pass: Lint.Utils.dedent`
const object1 = {};
const object2 = {};
if (object1 === object2) {}
`,
fail: Lint.Utils.dedent`
const object1 = {};
const object2 = {};
if (object1 < object2) {}
`,
},
];
278 changes: 278 additions & 0 deletions src/rules/objectComparisonRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
/**
* @license
* Copyright 2019 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 { isBinaryExpression, isTypeFlagSet, isUnionType } from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";

import { codeExamples } from "./code-examples/objectComparison.examples";

const OPTION_ALLOW_OBJECT_EQUAL_COMPARISON = "allow-object-equal-comparison";
const OPTION_ALLOW_STRING_ORDER_COMPARISON = "allow-string-order-comparison";

const enum TypeKind {
Any = 0,
Number = 1,
Enum = 2,
String = 3,
Boolean = 4,
NullOrUndefined = 5,
Object = 6,
}

const typeNames = {
[TypeKind.Any]: "any",
[TypeKind.Number]: "number",
[TypeKind.Enum]: "enum",
[TypeKind.String]: "string",
[TypeKind.Boolean]: "boolean",
[TypeKind.NullOrUndefined]: "null | undefined",
AndreasGassmann marked this conversation as resolved.
Show resolved Hide resolved
[TypeKind.Object]: "object",
};

interface Options {
[OPTION_ALLOW_OBJECT_EQUAL_COMPARISON]?: boolean;
[OPTION_ALLOW_STRING_ORDER_COMPARISON]?: boolean;
}

export class Rule extends Lint.Rules.TypedRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "object-comparison",
description: "Only allow comparisons between primitives.",
optionsDescription: Lint.Utils.dedent`
One of the following arguments may be optionally provided:
* \`${OPTION_ALLOW_OBJECT_EQUAL_COMPARISON}\` allows \`!=\` \`==\` \`!==\` \`===\` comparison between any types.
* \`${OPTION_ALLOW_STRING_ORDER_COMPARISON}\` allows \`>\` \`<\` \`>=\` \`<=\` comparison between strings.`,
options: {
type: "object",
properties: {
[OPTION_ALLOW_OBJECT_EQUAL_COMPARISON]: {
type: "boolean",
},
[OPTION_ALLOW_STRING_ORDER_COMPARISON]: {
type: "boolean",
},
},
},
optionExamples: [
true,
[
true,
{
[OPTION_ALLOW_OBJECT_EQUAL_COMPARISON]: false,
[OPTION_ALLOW_STRING_ORDER_COMPARISON]: false,
},
],
],
rationale: Lint.Utils.dedent`
When using comparison operators to compare objects, they compare references and not values.
This is often done accidentally.
With this rule, \`>\`, \`>=\`, \`<\`, \`<=\` operators are only allowed when comparing \`numbers\`.
\`===\`, \`!==\` are allowed for \`number\` \`string\` and \`boolean\` types and if one of the
operands is \`null\` or \`undefined\`.
`,
type: "functionality",
typescriptOnly: false,
requiresTypeInfo: true,
codeExamples,
};
/* tslint:enable:object-literal-sort-keys */

public static INVALID_TYPES(types1: TypeKind[], types2: TypeKind[]) {
const types1String = types1.map(type => typeNames[type]).join(" | ");
const types2String = types2.map(type => typeNames[type]).join(" | ");

return `cannot compare type '${types1String}' to type '${types2String}'`;
}

public static INVALID_TYPE_FOR_OPERATOR(type: TypeKind, comparator: string) {
return `cannot use '${comparator}' comparator for type '${typeNames[type]}'`;
}

public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk, this.getRuleOptions(), program);
}

private getRuleOptions(): Options {
if (this.ruleArguments[0] === undefined) {
return {};
} else {
return this.ruleArguments[0] as Options;
}
}
}

function walk(ctx: Lint.WalkContext<Options>, program: ts.Program) {
const { sourceFile, options } = ctx;

const checker = program.getTypeChecker();

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

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

const operandKind = getStrictestComparableType(leftKinds, rightKinds);

if (operandKind === undefined) {
const failureString = Rule.INVALID_TYPES(leftKinds, rightKinds);
ctx.addFailureAtNode(node, failureString);
} else {
const failureString = Rule.INVALID_TYPE_FOR_OPERATOR(
operandKind,
node.operatorToken.getText(),
);
const isEquality = isEqualityOperator(node);
if (isEquality) {
// Check !=, ==, !==, ===
switch (operandKind) {
case TypeKind.Any:
case TypeKind.Number:
case TypeKind.Enum:
case TypeKind.String:
case TypeKind.Boolean:
break;
case TypeKind.NullOrUndefined:
case TypeKind.Object:
if (options[OPTION_ALLOW_OBJECT_EQUAL_COMPARISON]) {
break;
}
ctx.addFailureAtNode(node, failureString);
break;
default:
ctx.addFailureAtNode(node, failureString);
}
} else {
// Check >, <, >=, <=
switch (operandKind) {
case TypeKind.Any:
case TypeKind.Number:
break;
case TypeKind.String:
if (options[OPTION_ALLOW_STRING_ORDER_COMPARISON]) {
break;
}
ctx.addFailureAtNode(node, failureString);
break;
default:
ctx.addFailureAtNode(node, failureString);
}
}
}
}
return ts.forEachChild(node, cb);
});
}

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;
return isUnionType(types)
? Array.from(new Set(types.types.map(getKind)))
: isTypeFlagSet(types, ts.TypeFlags.EnumLiteral) && typeof baseType !== "undefined"
? [getKind(baseType)]
: [getKind(types)];
}

function getStrictestComparableType(types1: TypeKind[], types2: TypeKind[]): TypeKind | undefined {
const overlappingTypes = types1.filter(type1 => types2.indexOf(type1) >= 0);

if (overlappingTypes.length > 0) {
return getStrictestKind(overlappingTypes);
} else {
// In case one of the types is "any", get the strictest type of the other array
if (arrayContainsKind(types1, TypeKind.Any)) {
return getStrictestKind(types2);
}
if (arrayContainsKind(types2, TypeKind.Any)) {
return getStrictestKind(types1);
}

// In case one array contains NullOrUndefined and the other an Object, return Object
if (
(arrayContainsKind(types1, TypeKind.NullOrUndefined) &&
arrayContainsKind(types2, TypeKind.Object)) ||
(arrayContainsKind(types2, TypeKind.NullOrUndefined) &&
arrayContainsKind(types1, TypeKind.Object))
) {
return TypeKind.Object;
}
return undefined;
}
}

function arrayContainsKind(types: TypeKind[], typeToCheck: TypeKind): boolean {
return types.some(type => type === typeToCheck);
}

function getStrictestKind(types: TypeKind[]): TypeKind {
// tslint:disable-next-line:no-unsafe-any
return Math.max.apply(Math, types);
}

function isComparisonOperator(node: ts.BinaryExpression): boolean {
switch (node.operatorToken.kind) {
case ts.SyntaxKind.LessThanToken:
case ts.SyntaxKind.GreaterThanToken:
case ts.SyntaxKind.LessThanEqualsToken:
case ts.SyntaxKind.GreaterThanEqualsToken:
case ts.SyntaxKind.EqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsToken:
case ts.SyntaxKind.EqualsEqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
return true;
default:
return false;
}
}

function isEqualityOperator(node: ts.BinaryExpression): boolean {
switch (node.operatorToken.kind) {
case ts.SyntaxKind.EqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsToken:
case ts.SyntaxKind.EqualsEqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
return true;
default:
return false;
}
}

function getKind(type: ts.Type): TypeKind {
// tslint:disable:no-bitwise
return is(ts.TypeFlags.String | ts.TypeFlags.StringLiteral)
? TypeKind.String
: is(ts.TypeFlags.Number | ts.TypeFlags.NumberLiteral)
? TypeKind.Number
: is(ts.TypeFlags.BooleanLike)
? TypeKind.Boolean
: is(ts.TypeFlags.Null | ts.TypeFlags.Undefined | ts.TypeFlags.Void)
? TypeKind.NullOrUndefined
: is(ts.TypeFlags.Any)
? TypeKind.Any
: TypeKind.Object;
// tslint:enable:no-bitwise

function is(flags: ts.TypeFlags) {
return isTypeFlagSet(type, flags);
}
}
Loading