Skip to content

Commit

Permalink
[bugfix] no-unsafe-any: allow implicitly downcasting any to `unkn…
Browse files Browse the repository at this point in the history
…own` (palantir#4442)
  • Loading branch information
ThomasdenH authored and adidahiya committed Jan 11, 2019
1 parent 6d3e941 commit 4e21e43
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 22 deletions.
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.
*/

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<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
}
}

0 comments on commit 4e21e43

Please sign in to comment.