diff --git a/docs/_data/rules.json b/docs/_data/rules.json index bdf69bd6a6b..73b495125eb 100644 --- a/docs/_data/rules.json +++ b/docs/_data/rules.json @@ -1455,6 +1455,18 @@ "typescriptOnly": true, "requiresTypeInfo": true }, + { + "ruleName": "strict-type-predicates", + "description": "\nWarns for type predicates that are always true or always false.\nWorks for 'typeof' comparisons to constants (e.g. 'typeof foo === \"string\"'), and equality comparison to 'null'/'undefined'.\nDoes not yet work for 'instanceof'.\nDoes *not* warn for 'if (x.y)' where 'x.y' is always truthy. For that, see strict-boolean-expressions.", + "optionsDescription": "Not configurable.", + "options": null, + "optionExamples": [ + "true" + ], + "type": "functionality", + "typescriptOnly": true, + "requiresTypeInfo": true + }, { "ruleName": "switch-default", "description": "Require a `default` case in all `switch` statements.", @@ -1720,7 +1732,7 @@ "ruleName": "whitespace", "description": "Enforces whitespace style conventions.", "rationale": "Helps maintain a readable, consistent style in your codebase.", - "optionsDescription": "\nSeven arguments may be optionally provided:\n\n* `\"check-branch\"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace.\n* `\"check-decl\"`checks that variable declarations have whitespace around the equals token.\n* `\"check-operator\"` checks for whitespace around operator tokens.\n* `\"check-module\"` checks for whitespace in import & export statements.\n* `\"check-separator\"` checks for whitespace after separator tokens (`,`/`;`).\n* `\"check-type\"` checks for whitespace before a variable type specification.\n* `\"check-typecast\"` checks for whitespace between a typecast and its target.", + "optionsDescription": "\nSeven arguments may be optionally provided:\n\n* `\"check-branch\"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace.\n* `\"check-decl\"`checks that variable declarations have whitespace around the equals token.\n* `\"check-operator\"` checks for whitespace around operator tokens.\n* `\"check-module\"` checks for whitespace in import & export statements.\n* `\"check-separator\"` checks for whitespace after separator tokens (`,`/`;`).\n* `\"check-type\"` checks for whitespace before a variable type specification.\n* `\"check-typecast\"` checks for whitespace between a typecast and its target.\n* `\"check-preblock\"` checks for whitespace before the opening brace of a block", "options": { "type": "array", "items": { @@ -1732,7 +1744,8 @@ "check-module", "check-separator", "check-type", - "check-typecast" + "check-typecast", + "check-preblock" ] }, "minLength": 0, diff --git a/docs/rules/strict-type-predicates/index.html b/docs/rules/strict-type-predicates/index.html new file mode 100644 index 00000000000..289aa8775b8 --- /dev/null +++ b/docs/rules/strict-type-predicates/index.html @@ -0,0 +1,19 @@ +--- +ruleName: strict-type-predicates +description: |- + + Warns for type predicates that are always true or always false. + Works for 'typeof' comparisons to constants (e.g. 'typeof foo === "string"'), and equality comparison to 'null'/'undefined'. + Does not yet work for 'instanceof'. + Does *not* warn for 'if (x.y)' where 'x.y' is always truthy. For that, see strict-boolean-expressions. +optionsDescription: Not configurable. +options: null +optionExamples: + - 'true' +type: functionality +typescriptOnly: true +requiresTypeInfo: true +layout: rule +title: 'Rule: strict-type-predicates' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/docs/rules/whitespace/index.html b/docs/rules/whitespace/index.html index 259fd6a8e02..97d6f7bfb29 100644 --- a/docs/rules/whitespace/index.html +++ b/docs/rules/whitespace/index.html @@ -13,6 +13,7 @@ * `"check-separator"` checks for whitespace after separator tokens (`,`/`;`). * `"check-type"` checks for whitespace before a variable type specification. * `"check-typecast"` checks for whitespace between a typecast and its target. + * `"check-preblock"` checks for whitespace before the opening brace of a block options: type: array items: @@ -25,6 +26,7 @@ - check-separator - check-type - check-typecast + - check-preblock minLength: 0 maxLength: 7 optionExamples: @@ -45,7 +47,8 @@ "check-module", "check-separator", "check-type", - "check-typecast" + "check-typecast", + "check-preblock" ] }, "minLength": 0, diff --git a/src/language/utils.ts b/src/language/utils.ts index b79bff5b45f..bcba3927019 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -178,6 +178,11 @@ export function isTypeFlagSet(type: ts.Type, flagToCheck: ts.TypeFlags): boolean /* tslint:enable:no-bitwise */ } +/** Type predicate to test for a union type. */ +export function isUnionType(type: ts.Type): type is ts.UnionType { + return isTypeFlagSet(type, ts.TypeFlags.Union); +} + /** * Bitwise check for object flags. * Does not work with TypeScript 2.0.x diff --git a/src/rules/strictTypePredicatesRule.ts b/src/rules/strictTypePredicatesRule.ts new file mode 100644 index 00000000000..9cd77387488 --- /dev/null +++ b/src/rules/strictTypePredicatesRule.ts @@ -0,0 +1,210 @@ +/** + * @license + * Copyright 2016 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"; + +// tslint:disable:no-bitwise + +export class Rule extends Lint.Rules.TypedRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "strict-type-predicates", + description: Lint.Utils.dedent` + Warns for type predicates that are always true or always false. + Works for 'typeof' comparisons to constants (e.g. 'typeof foo === "string"'), and equality comparison to 'null'/'undefined'. + (TypeScript won't let you compare '1 === 2', but it has an exception for '1 === undefined'.) + Does not yet work for 'instanceof'. + Does *not* warn for 'if (x.y)' where 'x.y' is always truthy. For that, see strict-boolean-expressions.`, + optionsDescription: "Not configurable.", + options: null, + optionExamples: ["true"], + type: "functionality", + typescriptOnly: true, + requiresTypeInfo: true, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING(value: boolean): string { + return `Expression is always ${value}.`; + } + + public applyWithProgram(srcFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(srcFile, this.getOptions(), langSvc.getProgram())); + } +} + +class Walker extends Lint.ProgramAwareRuleWalker { + public visitBinaryExpression(node: ts.BinaryExpression) { + const equals = getEquals(node.operatorToken.kind); + if (equals) { + this.checkEquals(node, equals); + } + super.visitBinaryExpression(node); + } + + private checkEquals(node: ts.BinaryExpression, { isStrict, isPositive }: Equals) { + const exprPred = getTypePredicate(node, isStrict); + if (!exprPred) { + return; + } + + const { expression, predicate, isNullOrUndefined } = exprPred; + const checker = this.getTypeChecker(); + const exprType = checker.getTypeAtLocation(expression); + // TODO: could use checker.getBaseConstraintOfType to help with type parameters, but it's not publicly exposed. + if (Lint.isTypeFlagSet(exprType, ts.TypeFlags.Any | ts.TypeFlags.TypeParameter)) { + return; + } + + const value = getConstantBoolean(exprType, predicate); + // 'null'/'undefined' are the only two values *not* assignable to '{}'. + if (value !== undefined && (isNullOrUndefined || !isEmptyType(checker, exprType))) { + this.addFailureAtNode(node, Rule.FAILURE_STRING(value === isPositive)); + } + } +} + +/** Detects a type predicate given `left === right`. */ +function getTypePredicate(node: ts.BinaryExpression, isStrictEquals: boolean): TypePredicate | undefined { + const { left, right } = node; + return getTypePredicateOneWay(left, right, isStrictEquals) || getTypePredicateOneWay(right, left, isStrictEquals); +} + +/** Only gets the type predicate if the expression is on the left. */ +function getTypePredicateOneWay(left: ts.Expression, right: ts.Expression, isStrictEquals: boolean): TypePredicate | undefined { + switch (right.kind) { + case ts.SyntaxKind.StringLiteral: + if (left.kind !== ts.SyntaxKind.TypeOfExpression) { + return undefined; + } + const expression = (left as ts.TypeOfExpression).expression; + const kind = (right as ts.StringLiteral).text; + return { expression, predicate: getTypePredicateForKind(kind), isNullOrUndefined: kind === "undefined" }; + + case ts.SyntaxKind.NullKeyword: + return nullOrUndefined(false); + + case ts.SyntaxKind.Identifier: + if ((right as ts.Identifier).text === "undefined") { + return nullOrUndefined(true); + } + + default: + return undefined; + } + + function nullOrUndefined(isUndefined: boolean): TypePredicate { + const flags = isStrictEquals + ? isUndefined ? undefinedFlags : ts.TypeFlags.Null + : (ts.TypeFlags.Null | undefinedFlags); + return { expression: left, predicate: flagPredicate(flags), isNullOrUndefined: true }; + } +} + +function isEmptyType(checker: ts.TypeChecker, type: ts.Type) { + return checker.typeToString(type) === "{}"; +} + +const undefinedFlags = ts.TypeFlags.Undefined | ts.TypeFlags.Void; + +interface TypePredicate { + expression: ts.Expression; + predicate: Predicate; + isNullOrUndefined: boolean; +} + +type Predicate = (type: ts.Type) => boolean; + +function getTypePredicateForKind(kind: string): Predicate { + switch (kind) { + case "undefined": + return flagPredicate(undefinedFlags); + case "boolean": + return flagPredicate(ts.TypeFlags.BooleanLike); + case "number": + return flagPredicate(ts.TypeFlags.NumberLike); + case "string": + return flagPredicate(ts.TypeFlags.StringLike); + case "symbol": + return flagPredicate(ts.TypeFlags.ESSymbol); + case "function": + return isFunction; + case "object": + // It's an object if it's not any of the above. + const allFlags = ts.TypeFlags.Undefined | ts.TypeFlags.Void | ts.TypeFlags.BooleanLike | + ts.TypeFlags.NumberLike | ts.TypeFlags.StringLike | ts.TypeFlags.ESSymbol; + return (type) => !Lint.isTypeFlagSet(type, allFlags) && !isFunction(type); + default: + return (_) => false; + } +} + +function flagPredicate(testedFlag: ts.TypeFlags): Predicate { + return (type) => Lint.isTypeFlagSet(type, testedFlag); +} + +function isFunction(t: ts.Type): boolean { + if (t.getCallSignatures().length !== 0) { + return true; + } + const symbol = t.getSymbol(); + return (symbol && symbol.getName()) === "Function"; +} + +/** Returns a boolean value if that should always be the result of a type predicate. */ +function getConstantBoolean(type: ts.Type, predicate: (t: ts.Type) => boolean): boolean | undefined { + if (!Lint.isUnionType(type)) { + return predicate(type); + } + + let anyTrue = false; + let anyFalse = false; + for (const ty of type.types) { + if (predicate(ty)) { + anyTrue = true; + } else { + anyFalse = true; + } + + if (anyTrue && anyFalse) { + return undefined; + } + } + + return anyTrue; +} + +interface Equals { + isPositive: boolean; // True for "===" and "==" + isStrict: boolean; +} + +function getEquals(kind: ts.SyntaxKind): Equals | undefined { + switch (kind) { + case ts.SyntaxKind.EqualsEqualsToken: + return { isPositive: true, isStrict: false }; + case ts.SyntaxKind.EqualsEqualsEqualsToken: + return { isPositive: true, isStrict: true }; + case ts.SyntaxKind.ExclamationEqualsToken: + return { isPositive: false, isStrict: false }; + case ts.SyntaxKind.ExclamationEqualsEqualsToken: + return { isPositive: false, isStrict: true }; + default: + return undefined; + } +} diff --git a/test/rules/strict-type-predicates/test.ts.lint b/test/rules/strict-type-predicates/test.ts.lint new file mode 100644 index 00000000000..5b9d9213a16 --- /dev/null +++ b/test/rules/strict-type-predicates/test.ts.lint @@ -0,0 +1,154 @@ +declare function get(): T; + +// typeof undefined +{ + typeof get() === "undefined"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get() === "undefined"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "undefined"; + + declare const c: any; + typeof get() === "undefined"; + + // 'undefined' is not assignable to '{}' + typeof get<{}>() === "undefined"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] +} + +// typeof boolean +{ + declare const a: boolean; + typeof get() === "boolean"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "boolean"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get() === "boolean"; + + typeof get<{}>() === "boolean"; +} + +// typeof number +{ + enum E {} + + typeof get() === "number"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "number"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] +} + +// typeof string +{ + typeof get<"abc" | "def">() === "string"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "string"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get<"abc" | undefined>() === "string"; +} + +// typeof symbol +{ + typeof get() === "symbol"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "symbol"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get() === "symbol"; +} + +// typeof function +{ + typeof get<() => void>() === "function"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + typeof get() === "function"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + + typeof get() === "function"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get void)>() === "function"; +} + +// typeof object +{ + typeof get void) | Function>() === "object"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + typeof get<{}> === "object"; +} + +// === null / undefined +{ + get() === null; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + get() === null; + + get() === undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + get() === undefined; + get() == null; + get() == undefined; + get() == null; + get() == undefined; + + // 'null' and 'undefined' are not assignable to '{}' + + get<{}>() === null; + ~~~~~~~~~~~~~~~~~~ [F] + + get<{}>() === undefined; + ~~~~~~~~~~~~~~~~~~~~~~~ [F] +} + +// negation +{ + get() !== null; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + get() !== null; + + get() !== undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [T] + + get() !== undefined; + + typeof get() !== "string"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] +} + +// reverse left/right +{ + "string" === typeof get(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [F] + + undefined === get(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ [T] +} + +// type parameters +{ + function f(t: T) { + typeof t === "boolean"; + } + + // TODO: Would be nice to catch this. + function g(t: T) { + typeof t === "boolean"; + } +} + +[T]: Expression is always true. +[F]: Expression is always false. diff --git a/test/rules/strict-type-predicates/tsconfig.json b/test/rules/strict-type-predicates/tsconfig.json new file mode 100644 index 00000000000..1cc3bc85ee9 --- /dev/null +++ b/test/rules/strict-type-predicates/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "strictNullChecks": true + } +} \ No newline at end of file diff --git a/test/rules/strict-type-predicates/tslint.json b/test/rules/strict-type-predicates/tslint.json new file mode 100644 index 00000000000..268b470e4be --- /dev/null +++ b/test/rules/strict-type-predicates/tslint.json @@ -0,0 +1,8 @@ +{ + "linterOptions": { + "typeCheck": true + }, + "rules": { + "strict-type-predicates": true + } +}