From 4e21e43cb228879f1cbc750ced8d23d5d3337eb9 Mon Sep 17 00:00:00 2001 From: Thomas den Hollander Date: Fri, 11 Jan 2019 15:59:24 +0100 Subject: [PATCH] [bugfix] `no-unsafe-any`: allow implicitly downcasting `any` to `unknown` (#4442) --- src/rules/noUnsafeAnyRule.ts | 54 +++++++----- test/rules/no-unsafe-any/unknown/test.ts.lint | 83 +++++++++++++++++++ .../rules/no-unsafe-any/unknown/tsconfig.json | 7 ++ test/rules/no-unsafe-any/unknown/tslint.json | 5 ++ 4 files changed, 127 insertions(+), 22 deletions(-) create mode 100644 test/rules/no-unsafe-any/unknown/test.ts.lint create mode 100644 test/rules/no-unsafe-any/unknown/tsconfig.json create mode 100644 test/rules/no-unsafe-any/unknown/tslint.json diff --git a/src/rules/noUnsafeAnyRule.ts b/src/rules/noUnsafeAnyRule.ts index ab438a280b4..8519d4ed889 100644 --- a/src/rules/noUnsafeAnyRule.ts +++ b/src/rules/noUnsafeAnyRule.ts @@ -34,6 +34,7 @@ export class Rule extends Lint.Rules.TypedRule { description: Lint.Utils.dedent` Warns when using an expression of type 'any' in a dynamic way. Uses are only allowed if they would work for \`{} | null | undefined\`. + Downcasting to unknown is always safe. Type casts and tests are allowed. Expressions that work on all values (such as \`"" + x\`) are allowed.`, optionsDescription: "Not configurable.", @@ -153,7 +154,7 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { initializer !== undefined && this.visitNode( initializer, - isPropertyAny(node as ts.PropertyDeclaration, this.checker), + isPropertyAnyOrUnknown(node as ts.PropertyDeclaration, this.checker), ) ); } @@ -315,7 +316,8 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { private checkContextualType(node: ts.Expression, allowIfNoContextualType?: boolean) { const type = this.checker.getContextualType(node); - return this.visitNode(node, (type === undefined && allowIfNoContextualType) || isAny(type)); + const anyOk = (type === undefined && allowIfNoContextualType) || isAny(type, true); + return this.visitNode(node, anyOk); } // Allow `const x = foo;` and `const x: any = foo`, but not `const x: Foo = foo;`. @@ -325,17 +327,15 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { initializer, }: ts.VariableDeclaration | ts.ParameterDeclaration) { this.checkBindingName(name); - // Always allow the LHS to be `any`. Just don't allow RHS to be `any` when LHS isn't. - return ( - initializer !== undefined && - this.visitNode( - initializer, - /*anyOk*/ - (name.kind === ts.SyntaxKind.Identifier && - (type === undefined || type.kind === ts.SyntaxKind.AnyKeyword)) || - (type !== undefined && type.kind === ts.SyntaxKind.AnyKeyword), - ) - ); + // Always allow the LHS to be `any`. Just don't allow RHS to be `any` when LHS isn't `any` or `unknown`. + const anyOk = + (name.kind === ts.SyntaxKind.Identifier && + (type === undefined || + type.kind === ts.SyntaxKind.AnyKeyword || + type.kind === ts.SyntaxKind.UnknownKeyword)) || + (type !== undefined && type.kind === ts.SyntaxKind.AnyKeyword) || + (type !== undefined && type.kind === ts.SyntaxKind.UnknownKeyword); + return initializer !== undefined && this.visitNode(initializer, anyOk); } private checkBinaryExpression(node: ts.BinaryExpression, anyOk: boolean | undefined) { @@ -357,7 +357,7 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { case ts.SyntaxKind.EqualsToken: // Allow assignment if the lhs is also *any*. allowAnyLeft = true; - allowAnyRight = isNodeAny(node.left, this.checker); + allowAnyRight = isNodeAny(node.left, this.checker, true); break; case ts.SyntaxKind.PlusToken: // Allow implicit stringification case ts.SyntaxKind.PlusEqualsToken: @@ -416,8 +416,11 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { } /** Check if property has no type annotation in this class and the base class */ -function isPropertyAny(node: ts.PropertyDeclaration, checker: ts.TypeChecker) { - if (!isNodeAny(node.name, checker) || node.name.kind === ts.SyntaxKind.ComputedPropertyName) { +function isPropertyAnyOrUnknown(node: ts.PropertyDeclaration, checker: ts.TypeChecker) { + if ( + !isNodeAny(node.name, checker, true) || + node.name.kind === ts.SyntaxKind.ComputedPropertyName + ) { return false; } for (const base of checker.getBaseTypes(checker.getTypeAtLocation( @@ -425,13 +428,16 @@ function isPropertyAny(node: ts.PropertyDeclaration, checker: ts.TypeChecker) { ) as ts.InterfaceType)) { const prop = base.getProperty(node.name.text); if (prop !== undefined && prop.declarations !== undefined) { - return isAny(checker.getTypeOfSymbolAtLocation(prop, prop.declarations[0])); + return isAny(checker.getTypeOfSymbolAtLocation(prop, prop.declarations[0]), true); } } return true; } -function isNodeAny(node: ts.Node, checker: ts.TypeChecker): boolean { +/** + * @param orUnknown If true, this function will also return true when the node is unknown. + */ +function isNodeAny(node: ts.Node, checker: ts.TypeChecker, orUnknown: boolean = false): boolean { let symbol = checker.getSymbolAtLocation(node); if (symbol !== undefined && isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)) { symbol = checker.getAliasedSymbol(symbol); @@ -442,7 +448,7 @@ function isNodeAny(node: ts.Node, checker: ts.TypeChecker): boolean { return false; } if (isSymbolFlagSet(symbol, ts.SymbolFlags.Type)) { - return isAny(checker.getDeclaredTypeOfSymbol(symbol)); + return isAny(checker.getDeclaredTypeOfSymbol(symbol), orUnknown); } } @@ -451,7 +457,7 @@ function isNodeAny(node: ts.Node, checker: ts.TypeChecker): boolean { return false; } - return isAny(checker.getTypeAtLocation(node)); + return isAny(checker.getTypeAtLocation(node), orUnknown); } const jsxElementTypes = new Set([ @@ -477,6 +483,10 @@ function isStringLike(expr: ts.Expression, checker: ts.TypeChecker): boolean { return isTypeFlagSet(checker.getTypeAtLocation(expr), ts.TypeFlags.StringLike); } -function isAny(type: ts.Type | undefined): boolean { - return type !== undefined && isTypeFlagSet(type, ts.TypeFlags.Any); +function isAny(type: ts.Type | undefined, orUnknown: boolean = false): boolean { + return ( + type !== undefined && + (isTypeFlagSet(type, ts.TypeFlags.Any) || + (orUnknown && isTypeFlagSet(type, ts.TypeFlags.Unknown))) + ); } diff --git a/test/rules/no-unsafe-any/unknown/test.ts.lint b/test/rules/no-unsafe-any/unknown/test.ts.lint new file mode 100644 index 00000000000..1f955c599cb --- /dev/null +++ b/test/rules/no-unsafe-any/unknown/test.ts.lint @@ -0,0 +1,83 @@ +[typescript]: >= 3.0.0 + +/* + * It is not unsafe to pass any where unknown is allowed. This file checks + * these uses. + */ + +declare const x: any; + +declare function takesUnknown(a: unknown, ...bs: unknown[]): void; +takesUnknown(x, x); + +declare function templateTakesUnknown(arr: TemplateStringsArray, a: unknown, ...bs: unknown[]): any; +templateTakesUnknown`${x}${x}`; + +declare function decoratorTakesUnknown(value: unknown): Function; + +class C { + @decoratorTakesUnknown(x) f() {} +} + +function f(x: any, retAny: () => any): unknown { + const v2: unknown = x; + let v5: unknown; + v5 = x; + + // Return OK if return type is 'any' + return x; +} + +class X { + constructor(y: any) {} + prop: unknown = x; +} + +takesUnknown(x ? x : x); + +function *gen(): IterableIterator { + yield x; +} + +void x; + +{ + class C { + prop: unknown = x; + } + class D extends C { + prop = x; + } +} + +function hasThisParameter(this: any) { + const u: unknown = this; +} + +(async function(): Promise { + return x; +}); + +const obj = { property: "value" } as any; +const result: unknown = JSON.parse("{}"); + +const hasUnknownProp: { prop: unknown, obj: unknown } = { prop: obj, obj }; +hasUnknownProp.prop = obj; + +function acceptsUnknown(a: unknown, b: unknown = x) { } + +acceptsUnknown(obj); + +interface ContainsUnknownProperty { + e: unknown; +} + +const p: ContainsUnknownProperty = { e: (123 as any) }; + +function g() { + try { + + } catch (e) { + acceptsUnknown(e); + } +} diff --git a/test/rules/no-unsafe-any/unknown/tsconfig.json b/test/rules/no-unsafe-any/unknown/tsconfig.json new file mode 100644 index 00000000000..2ea9f28254e --- /dev/null +++ b/test/rules/no-unsafe-any/unknown/tsconfig.json @@ -0,0 +1,7 @@ +{ + "compilerOptions": { + "module": "commonjs", + "target": "es6", + "experimentalDecorators": true + } +} diff --git a/test/rules/no-unsafe-any/unknown/tslint.json b/test/rules/no-unsafe-any/unknown/tslint.json new file mode 100644 index 00000000000..b44ff572bc8 --- /dev/null +++ b/test/rules/no-unsafe-any/unknown/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-unsafe-any": true + } +}