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 6 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
58 changes: 36 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),
isPropertyAny(node as ts.PropertyDeclaration, this.checker, true),
)
);
}
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,32 @@ 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 isPropertyAny(
node: ts.PropertyDeclaration,
checker: ts.TypeChecker,
orUnknown: boolean = false,
ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved
) {
if (
!isNodeAny(node.name, checker, orUnknown) ||
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]), orUnknown);
}
}
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 +452,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 +461,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 +487,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)))
);
}
3 changes: 3 additions & 0 deletions test/rules/no-unsafe-any/unknown/commonjsModule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const x: any = 0;
namespace x {}
export = x;
8 changes: 8 additions & 0 deletions test/rules/no-unsafe-any/unknown/es6Module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const defaultExport: any = 0;
export default defaultExport;
export const namedExport: any = 0;
export type T = number;

export namespace NS {
export interface ITest {}
}
82 changes: 82 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,82 @@
[typescript]: >= 3.0.0

import importEquals = require("./commonjsModule");
import importAlias = importEquals;
namespace N { export const x: any = 0; }
import importQualifiedName = N.x;
import * as namespaceImport from "./es6Module";

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

declare function templateTakesUnknown(arr: TemplateStringsArray, a: any, ...bs: any[]): 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;
ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved

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 };
ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved

function f() {
ThomasdenH marked this conversation as resolved.
Show resolved Hide resolved
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
}
}