Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [no-unsafe-assignment] [no-unsafe-return] add never support #2746

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ 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-unnecessary-type-constraint`](./docs/rules/no-unnecessary-type-constraint.md) | Disallows unnecessary constraints on generic types | | :wrench: | |
| [`@typescript-eslint/no-unsafe-assignment`](./docs/rules/no-unsafe-assignment.md) | Disallows assigning any to variables and properties | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-assignment`](./docs/rules/no-unsafe-assignment.md) | Disallows assigning unsafe types to variables and properties | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-call`](./docs/rules/no-unsafe-call.md) | Disallows calling an any type value | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-member-access`](./docs/rules/no-unsafe-member-access.md) | Disallows member access on any typed variables | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-return`](./docs/rules/no-unsafe-return.md) | Disallows returning any from a function | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-unsafe-return`](./docs/rules/no-unsafe-return.md) | Disallows returning unsafe types from a function | :heavy_check_mark: | | :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 | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/prefer-enum-initializers`](./docs/rules/prefer-enum-initializers.md) | Prefer initializing each enums member value | | | |
Expand Down
25 changes: 20 additions & 5 deletions packages/eslint-plugin/docs/rules/no-unsafe-assignment.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
# Disallows assigning any to variables and properties (`no-unsafe-assignment`)
# Disallows assigning unsafe types to variables and properties (`no-unsafe-assignment`)

Despite your best intentions, the `any` type can sometimes leak into your codebase.
Despite your best intentions, unsafe types (such as `any` and `never`) type can sometimes leak into your codebase.
Assigning an `any` typed value to a variable can be hard to pick up on, particularly if it leaks in from an external library. Operations on the variable will not be checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase.
A `never` type is almost always an unsafe action and is often the result of a variable's type being narrowed down to `never`. This often indicates a code path which is unreachable.

## Rule Details

This rule disallows assigning `any` to a variable, and assigning `any[]` to an array destructuring.
This rule also compares the assigned type to the variable's type to ensure you don't assign an unsafe `any` in a generic position to a receiver that's expecting a specific type. For example, it will error if you assign `Set<any>` to a variable declared as `Set<string>`.
This rule disallows assigning `any` or `never` to a variable, and assigning `any[]` or `never[]` to an array destructuring.
This rule also compares the assigned type to the variable's type to ensure you don't assign an unsafe `any` or `never` in a generic position to a receiver that's expecting a specific type. For example, it will error if you assign `Set<any>` to a variable declared as `Set<string>`.

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

```ts
const x = 1 as any,
y = 1 as any;
y = 1 as any,
z = 1 as never;
const [x] = 1 as any;
const [x] = [] as any[];
const [x] = [1 as any];
Expand All @@ -26,8 +28,15 @@ class Foo {
private a = 1 as any;
}

// type narrowing
const x = 1;
if (typeof x === 'string') {
const y = x; // x is now `never`
}

// generic position examples
const x: Set<string> = new Set<any>();
const x: Set<string> = new Set<never>();
const x: Map<string, string> = new Map<string, any>();
const x: Set<string[]> = new Set<any[]>();
const x: Set<Set<Set<string>>> = new Set<Set<Set<any>>>();
Expand All @@ -49,6 +58,12 @@ class Foo {
private a = 1;
}

// type narrowing
const x = 1;
if (typeof x === 'number') {
const y = x; // x is still `number`
}

// generic position examples
const x: Set<string> = new Set<string>();
const x: Map<string, string> = new Map<string, string>();
Expand Down
19 changes: 16 additions & 3 deletions packages/eslint-plugin/docs/rules/no-unsafe-return.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Disallows returning any from a function (`no-unsafe-return`)
# Disallows returning unsafe types 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.
Despite your best intentions, unsafe types (such as `any` and `never`) type can sometimes leak into your codebase.
Returned `any` typed values are not checked at all by TypeScript, so it creates a potential safety hole, and source of bugs in your codebase.
Returned `never` typed values are often the source of the variable's type being narrowed down to `never`.

## Rule Details

Expand All @@ -11,6 +12,12 @@ This rule also compares the return type to the function's declared/inferred retu
Examples of **incorrect** code for this rule:

```ts
function bar(a: string) {
if (typeof a === 'string') {
return 'b';
}
return a;
}
function foo1() {
return 1 as any;
}
Expand Down Expand Up @@ -52,6 +59,12 @@ const assignability2: TAssign = () => new Set<any>([true]);
Examples of **correct** code for this rule:

```ts
function bar(a: string | number) {
if (typeof a === 'string') {
return 'b';
}
return a;
}
function foo1() {
return 1;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ export = {
'@typescript-eslint/no-type-alias': 'error',
'@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error',
'@typescript-eslint/no-unnecessary-condition': 'error',
'@typescript-eslint/no-unnecessary-type-constraint': 'error',
'@typescript-eslint/no-unnecessary-qualifier': 'error',
'@typescript-eslint/no-unnecessary-type-arguments': 'error',
'@typescript-eslint/no-unnecessary-type-assertion': 'error',
'@typescript-eslint/no-unnecessary-type-constraint': 'error',
'@typescript-eslint/no-unsafe-assignment': 'error',
'@typescript-eslint/no-unsafe-call': 'error',
'@typescript-eslint/no-unsafe-member-access': 'error',
Expand Down
73 changes: 53 additions & 20 deletions packages/eslint-plugin/src/rules/no-unsafe-assignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,21 @@ export default util.createRule({
meta: {
type: 'problem',
docs: {
description: 'Disallows assigning any to variables and properties',
description:
'Disallows assigning unsafe types to variables and properties',
category: 'Possible Errors',
recommended: 'error',
requiresTypeChecking: true,
},
messages: {
anyAssignment: 'Unsafe assignment of an any value.',
unsafeArrayPattern: 'Unsafe array destructuring of an any array value.',
assignment: 'Unsafe assignment of an {{ type }} value.',
unsafeArrayPattern:
'Unsafe array destructuring of an {{ type }} array value.',
unsafeArrayPatternFromTuple:
'Unsafe array destructuring of a tuple element with an any value.',
'Unsafe array destructuring of a tuple element with an {{ type }} value.',
unsafeAssignment:
'Unsafe assignment of type {{sender}} to a variable of type {{receiver}}.',
unsafeArraySpread: 'Unsafe spread of an any value in an array.',
unsafeArraySpread: 'Unsafe spread of an {{ type }} value in an array.',
},
schema: [],
},
Expand All @@ -40,6 +42,14 @@ export default util.createRule({
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();

function isAnyOrNeverType(type: ts.Type, checker: ts.TypeChecker): boolean {
const isAny =
util.isTypeAnyType(type) || util.isTypeAnyArrayType(type, checker);
const isNever =
util.isTypeNeverType(type) || util.isTypeNeverArrayType(type, checker);
return isAny || isNever;
}

// returns true if the assignment reported
function checkArrayDestructureHelper(
receiverNode: TSESTree.Node,
Expand All @@ -61,12 +71,19 @@ export default util.createRule({
senderType: ts.Type,
senderNode: ts.Node,
): boolean {
// any array
// any or never array
// const [x] = ([] as any[]);
if (util.isTypeAnyArrayType(senderType, checker)) {
// const [x] = ([] as never[]);
if (
util.isTypeAnyArrayType(senderType, checker) ||
util.isTypeNeverArrayType(senderType, checker)
) {
context.report({
node: receiverNode,
messageId: 'unsafeArrayPattern',
data: {
type: checker.typeToString(senderType),
},
});
return false;
}
Expand All @@ -77,8 +94,9 @@ export default util.createRule({

const tupleElements = util.getTypeArguments(senderType, checker);

// tuple with any
// tuple with any or never
// const [x] = [1 as any];
// const [x] = [1 as never];
let didReport = false;
for (
let receiverIndex = 0;
Expand All @@ -100,11 +118,17 @@ export default util.createRule({
continue;
}

// check for the any type first so we can handle [[[x]]] = [any]
if (util.isTypeAnyType(senderType)) {
// check for the any or never type first so we can handle [[[x]]] = [any] or [[[x]]] = [never]
if (
util.isTypeAnyType(senderType) ||
util.isTypeNeverType(senderType)
) {
context.report({
node: receiverElement,
messageId: 'unsafeArrayPatternFromTuple',
data: {
type: checker.typeToString(senderType),
},
});
// we want to report on every invalid element in the tuple
didReport = true;
Expand Down Expand Up @@ -191,11 +215,14 @@ export default util.createRule({
continue;
}

// check for the any type first so we can handle {x: {y: z}} = {x: any}
if (util.isTypeAnyType(senderType)) {
// check for the any type first so we can handle {x: {y: z}} = {x: any} or {x: {y: z}} = {x: never}
if (isAnyOrNeverType(senderType, checker)) {
context.report({
node: receiverProperty.value,
messageId: 'unsafeArrayPatternFromTuple',
data: {
type: checker.typeToString(senderType),
},
});
didReport = true;
} else if (
Expand Down Expand Up @@ -237,15 +264,21 @@ export default util.createRule({
esTreeNodeToTSNodeMap.get(senderNode),
);

if (util.isTypeAnyType(senderType)) {
// handle cases when we assign any ==> unknown.
if (util.isTypeUnknownType(receiverType)) {
if (util.isTypeAnyType(senderType) || util.isTypeNeverType(senderType)) {
// handle cases when we assign any or never ==> unknown.
if (
util.isTypeUnknownType(receiverType) ||
util.isTypeUnknownArrayType(receiverType, checker)
) {
return false;
}

context.report({
node: reportingNode,
messageId: 'anyAssignment',
messageId: 'assignment',
data: {
type: checker.typeToString(senderType),
},
});
return true;
}
Expand Down Expand Up @@ -344,13 +377,13 @@ export default util.createRule({
'ArrayExpression > SpreadElement'(node: TSESTree.SpreadElement): void {
const resetNode = esTreeNodeToTSNodeMap.get(node.argument);
const restType = checker.getTypeAtLocation(resetNode);
if (
util.isTypeAnyType(restType) ||
util.isTypeAnyArrayType(restType, checker)
) {
if (isAnyOrNeverType(restType, checker)) {
context.report({
node: node,
messageId: 'unsafeArraySpread',
data: {
type: checker.typeToString(restType),
},
});
}
},
Expand Down
19 changes: 17 additions & 2 deletions packages/eslint-plugin/src/rules/no-unsafe-return.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default util.createRule({
meta: {
type: 'problem',
docs: {
description: 'Disallows returning any from a function',
description: 'Disallows returning unsafe types from a function',
category: 'Possible Errors',
recommended: 'error',
requiresTypeChecking: true,
Expand All @@ -19,6 +19,7 @@ export default util.createRule({
unsafeReturn: 'Unsafe return of an {{type}} typed value',
unsafeReturnAssignment:
'Unsafe return of type {{sender}} from function with return type {{receiver}}.',
unsafeReturnNever: 'Unsafe return of type {{sender}} from function.',
},
schema: [],
},
Expand Down Expand Up @@ -112,7 +113,21 @@ export default util.createRule({

for (const signature of functionType.getCallSignatures()) {
const functionReturnType = signature.getReturnType();
if (returnNodeType === functionReturnType) {

if (util.isTypeNeverType(returnNodeType)) {
return context.report({
node: reportingNode,
messageId: 'unsafeReturnNever',
data: {
sender: checker.typeToString(returnNodeType),
},
});
}

if (
checker.typeToString(returnNodeType) ===
checker.typeToString(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
Expand Down