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

[bugfix] no-unsafe-any: allow implicitly downcasting any to unknown #4442

Merged
merged 8 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 32 additions & 22 deletions src/rules/noUnsafeAnyRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -153,7 +154,7 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker<void> {
initializer !== undefined &&
this.visitNode(
initializer,
isPropertyAny(node as ts.PropertyDeclaration, this.checker),
isPropertyAnyOrUnknown(node as ts.PropertyDeclaration, this.checker),
)
);
}
Expand Down Expand Up @@ -315,7 +316,8 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker<void> {

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;`.
Expand All @@ -325,17 +327,15 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker<void> {
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) {
Expand All @@ -357,7 +357,7 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker<void> {
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:
Expand Down Expand Up @@ -416,22 +416,28 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker<void> {
}

/** 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(
node.parent,
) 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);
Expand All @@ -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);
}
}

Expand All @@ -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<ts.SyntaxKind>([
Expand All @@ -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)))
);
}
83 changes: 83 additions & 0 deletions test/rules/no-unsafe-any/unknown/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
[typescript]: >= 3.0.0

/*
* It is not unsafe to pass any where unknown is allowed. This file checks
* these uses.
*/

ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved
declare const x: any;

declare function takesUnknown(a: unknown, ...bs: unknown[]): void;
takesUnknown(x, x);
ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved

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<unknown> {
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<unknown> {
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);
}
}
7 changes: 7 additions & 0 deletions test/rules/no-unsafe-any/unknown/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es6",
"experimentalDecorators": true
}
}
5 changes: 5 additions & 0 deletions test/rules/no-unsafe-any/unknown/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"no-unsafe-any": true
}
}