Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(eslint-plugin): add rule no-unsafe-return (#1644)
  • Loading branch information
bradzacher committed Mar 3, 2020
1 parent ba22ea7 commit cfc3ef1
Show file tree
Hide file tree
Showing 9 changed files with 749 additions and 48 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -133,6 +133,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Enforces that type arguments will not be used if not required | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-member-access`](./docs/rules/no-unsafe-member-access.md) | Disallows member access on any typed variables | | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-return`](./docs/rules/no-unsafe-return.md) | Disallows returning any from a function | | | :thought_balloon: |
| [`@typescript-eslint/no-unused-vars-experimental`](./docs/rules/no-unused-vars-experimental.md) | Disallow unused variables and arguments | | | :thought_balloon: |
| [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :heavy_check_mark: | | |
| [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | | :wrench: | |
Expand Down
75 changes: 75 additions & 0 deletions packages/eslint-plugin/docs/rules/no-unsafe-return.md
@@ -0,0 +1,75 @@
# Disallows returning any from a function (`no-unsafe-return`)

Despite your best intentions, the `any` type can sometimes leak into your codebase.
Returned `any` typed values not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase.

## Rule Details

This rule disallows returning `any` or `any[]` from a function.
This rule also compares the return type to the function's declared/inferred return type to ensure you don't return an unsafe `any` in a generic position to a receiver that's expecting a specific type. For example, it will error if you return `Set<any>` from a function declared as returning `Set<string>`.

Examples of **incorrect** code for this rule:

```ts
function foo1() {
return 1 as any;
}
function foo2() {
return Object.create(null);
}
const foo3 = () => {
return 1 as any;
};
const foo4 = () => Object.create(null);

function foo5() {
return [] as any[];
}
function foo6() {
return [] as Array<any>;
}
function foo7() {
return [] as readonly any[];
}
function foo8() {
return [] as Readonly<any[]>;
}
const foo9 = () => {
return [] as any[];
};
const foo10 = () => [] as any[];

const foo11 = (): string[] => [1, 2, 3] as any[];

// generic position examples
function assignability1(): Set<string> {
return new Set<any>([1]);
}
type TAssign = () => Set<string>;
const assignability2: TAssign = () => new Set<any>([true]);
```

Examples of **correct** code for this rule:

```ts
function foo1() {
return 1;
}
function foo2() {
return Object.create(null) as Record<string, unknown>;
}

const foo3 = () => [];
const foo4 = () => ['a'];

function assignability1(): Set<string> {
return new Set<string>(['foo']);
}
type TAssign = () => Set<string>;
const assignability2: TAssign = () => new Set(['foo']);
```

## Related to

- [`no-explicit-any`](./no-explicit-any.md)
- TSLint: [`no-unsafe-any`](https://palantir.github.io/tslint/rules/no-unsafe-any/)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -62,6 +62,7 @@
"@typescript-eslint/no-unnecessary-type-arguments": "error",
"@typescript-eslint/no-unnecessary-type-assertion": "error",
"@typescript-eslint/no-unsafe-member-access": "error",
"@typescript-eslint/no-unsafe-return": "error",
"no-unused-expressions": "off",
"@typescript-eslint/no-unused-expressions": "error",
"no-unused-vars": "off",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -54,6 +54,7 @@ import noUnnecessaryQualifier from './no-unnecessary-qualifier';
import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments';
import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion';
import noUnsafeMemberAccess from './no-unsafe-member-access';
import noUnsafeReturn from './no-unsafe-return';
import noUntypedPublicSignature from './no-untyped-public-signature';
import noUnusedExpressions from './no-unused-expressions';
import noUnusedVars from './no-unused-vars';
Expand Down Expand Up @@ -146,6 +147,7 @@ export default {
'no-unnecessary-type-arguments': noUnnecessaryTypeArguments,
'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion,
'no-unsafe-member-access': noUnsafeMemberAccess,
'no-unsafe-return': noUnsafeReturn,
'no-untyped-public-signature': noUntypedPublicSignature,
'no-unused-expressions': noUnusedExpressions,
'no-unused-vars-experimental': noUnusedVarsExperimental,
Expand Down
@@ -1,12 +1,7 @@
import { TSESTree } from '@typescript-eslint/experimental-utils';
import {
isCallExpression,
isJsxExpression,
isNewExpression,
isObjectType,
isObjectFlagSet,
isParameterDeclaration,
isPropertyDeclaration,
isStrictCompilerOptionEnabled,
isTypeFlagSet,
isVariableDeclaration,
Expand Down Expand Up @@ -91,48 +86,6 @@ export default util.createRule<Options, MessageIds>({
return true;
}

/**
* Returns the contextual type of a given node.
* Contextual type is the type of the target the node is going into.
* i.e. the type of a called function's parameter, or the defined type of a variable declaration
*/
function getContextualType(
checker: ts.TypeChecker,
node: ts.Expression,
): ts.Type | undefined {
const parent = node.parent;
if (!parent) {
return;
}

if (isCallExpression(parent) || isNewExpression(parent)) {
if (node === parent.expression) {
// is the callee, so has no contextual type
return;
}
} else if (
isVariableDeclaration(parent) ||
isPropertyDeclaration(parent) ||
isParameterDeclaration(parent)
) {
return parent.type
? checker.getTypeFromTypeNode(parent.type)
: undefined;
} else if (isJsxExpression(parent)) {
return checker.getContextualType(parent);
} else if (
![ts.SyntaxKind.TemplateSpan, ts.SyntaxKind.JsxExpression].includes(
parent.kind,
)
) {
// parent is not something we know we can get the contextual type of
return;
}
// TODO - support return statement checking

return checker.getContextualType(node);
}

/**
* Returns true if there's a chance the variable has been used before a value has been assigned to it
*/
Expand Down Expand Up @@ -196,7 +149,7 @@ export default util.createRule<Options, MessageIds>({
// we know it's a nullable type
// so figure out if the variable is used in a place that accepts nullable types

const contextualType = getContextualType(checker, originalNode);
const contextualType = util.getContextualType(checker, originalNode);
if (contextualType) {
// in strict mode you can't assign null to undefined, so we have to make sure that
// the two types share a nullable type
Expand Down
135 changes: 135 additions & 0 deletions packages/eslint-plugin/src/rules/no-unsafe-return.ts
@@ -0,0 +1,135 @@
import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import { isExpression } from 'tsutils';
import * as util from '../util';

export default util.createRule({
name: 'no-unsafe-return',
meta: {
type: 'problem',
docs: {
description: 'Disallows returning any from a function',
category: 'Possible Errors',
recommended: false,
requiresTypeChecking: true,
},
messages: {
unsafeReturn: 'Unsafe return of an {{type}} typed value',
unsafeReturnAssignment:
'Unsafe return of type {{sender}} from function with return type {{receiver}}',
},
schema: [],
},
defaultOptions: [],
create(context) {
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();

function getParentFunctionNode(
node: TSESTree.Node,
):
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| null {
let current = node.parent;
while (current) {
if (
current.type === AST_NODE_TYPES.ArrowFunctionExpression ||
current.type === AST_NODE_TYPES.FunctionDeclaration ||
current.type === AST_NODE_TYPES.FunctionExpression
) {
return current;
}

current = current.parent;
}

// this shouldn't happen in correct code, but someone may attempt to parse bad code
// the parser won't error, so we shouldn't throw here
/* istanbul ignore next */ return null;
}

function checkReturn(
returnNode: TSESTree.Node,
reportingNode: TSESTree.Node = returnNode,
): void {
const tsNode = esTreeNodeToTSNodeMap.get(returnNode);
const anyType = util.isAnyOrAnyArrayTypeDiscriminated(tsNode, checker);
if (anyType !== util.AnyType.Safe) {
return context.report({
node: reportingNode,
messageId: 'unsafeReturn',
data: {
type: anyType === util.AnyType.Any ? 'any' : 'any[]',
},
});
}

const functionNode = getParentFunctionNode(returnNode);
/* istanbul ignore if */ if (!functionNode) {
return;
}

// function has an explicit return type, so ensure it's a safe return
const returnNodeType = checker.getTypeAtLocation(
esTreeNodeToTSNodeMap.get(returnNode),
);
const functionTSNode = esTreeNodeToTSNodeMap.get(functionNode);

// function expressions will not have their return type modified based on receiver typing
// so we have to use the contextual typing in these cases, i.e.
// const foo1: () => Set<string> = () => new Set<any>();
// the return type of the arrow function is Set<any> even though the variable is typed as Set<string>
let functionType = isExpression(functionTSNode)
? util.getContextualType(checker, functionTSNode)
: checker.getTypeAtLocation(functionTSNode);
if (!functionType) {
functionType = checker.getTypeAtLocation(functionTSNode);
}

for (const signature of functionType.getCallSignatures()) {
const functionReturnType = signature.getReturnType();
if (returnNodeType === functionReturnType) {
// don't bother checking if they're the same
// either the function is explicitly declared to return the same type
// or there was no declaration, so the return type is implicit
return;
}

const result = util.isUnsafeAssignment(
returnNodeType,
functionReturnType,
checker,
);
if (!result) {
return;
}

const { sender, receiver } = result;
return context.report({
node: reportingNode,
messageId: 'unsafeReturnAssignment',
data: {
sender: checker.typeToString(sender),
receiver: checker.typeToString(receiver),
},
});
}
}

return {
ReturnStatement(node): void {
const argument = node.argument;
if (!argument) {
return;
}

checkReturn(argument, node);
},
'ArrowFunctionExpression > :not(BlockStatement).body': checkReturn,
};
},
});

0 comments on commit cfc3ef1

Please sign in to comment.