Content-Length: 19518 | pFad | http://github.com/typescript-eslint/typescript-eslint/pull/10010.patch

thub.com From 6a7c1a693e53c6288bf7efd34796cbecc1105802 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 22:11:17 -0600 Subject: [PATCH 1/6] [no-unsafe-call] check calls of Function --- .../docs/rules/no-unsafe-call.mdx | 28 ++++++++++ .../docs/rules/no-unsafe-function-type.mdx | 1 + .../eslint-plugin/src/rules/no-unsafe-call.ts | 18 ++++-- .../no-unsafe-call.shot | 25 ++++++--- .../tests/rules/no-unsafe-call.test.ts | 56 +++++++++++++++++++ .../rules/no-unsafe-function-type.test.ts | 8 ++- 6 files changed, 122 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-call.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-call.mdx index 3e56c1e0f092..6fc46d6cbf18 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-call.mdx +++ b/packages/eslint-plugin/docs/rules/no-unsafe-call.mdx @@ -59,6 +59,34 @@ String.raw`foo`; +## The Unsafe `Function` Type + +The `Function` type is behaves almost identically to `any` when called, so this rule also disallows calling values of type `Function`. + + + + +```ts +const f: Function = () => {}; +f(); +``` + + + + +Note that whereas [no-unsafe-function-type](./no-unsafe-function-type.mdx) helps prevent the _creation_ of `Function` types, this rule helps prevent the unsafe _use_ of `Function` types, which may creep into your codebase without explicitly referencing the `Function` type at all. +See, for example, the following code: + +```ts +function unsafe(maybeFunction: unknown): string { + if (typeof maybeFunction === 'function') { + // TypeScript allows this, but it's completely unsound. + return maybeFunction('call', 'with', 'any', 'args'); + } + // etc +} +``` + ## When Not To Use It If your codebase has many existing `any`s or areas of unsafe code, it may be difficult to enable this rule. diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-function-type.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-function-type.mdx index ea7b60794e4e..ee1a84395d8d 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-function-type.mdx +++ b/packages/eslint-plugin/docs/rules/no-unsafe-function-type.mdx @@ -60,4 +60,5 @@ You might consider using [ESLint disable comments](https://eslint.org/docs/lates - [`no-empty-object-type`](./no-empty-object-type.mdx) - [`no-restricted-types`](./no-restricted-types.mdx) +- [`no-unsafe-call`](./no-unsafe-call.mdx) - [`no-wrapper-object-types`](./no-wrapper-object-types.mdx) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 6bdec17a4427..9bf464ff40a3 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -1,11 +1,13 @@ import type { TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; +import type * as ts from 'typescript'; import { createRule, getConstrainedTypeAtLocation, getParserServices, getThisExpression, + isBuiltinSymbolLike, isTypeAnyType, } from '../util'; @@ -25,13 +27,13 @@ export default createRule<[], MessageIds>({ requiresTypeChecking: true, }, messages: { - unsafeCall: 'Unsafe call of an {{type}} typed value.', + unsafeCall: 'Unsafe call of a(n) {{type}} typed value.', unsafeCallThis: [ - 'Unsafe call of an `any` typed value. `this` is typed as `any`.', + 'Unsafe call of a(n) {{type}} typed value. `this` is typed as {{type}}.', 'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.', ].join('\n'), - unsafeNew: 'Unsafe construction of an any type value.', - unsafeTemplateTag: 'Unsafe any typed template tag.', + unsafeNew: 'Unsafe construction of a(n) {{type}} typed value.', + unsafeTemplateTag: 'Unsafe use of a(n) {{type}} typed template tag.', }, schema: [], }, @@ -74,6 +76,14 @@ export default createRule<[], MessageIds>({ type: isErrorType ? '`error` type' : '`any`', }, }); + } else if (isBuiltinSymbolLike(services.program, type, 'Function')) { + context.report({ + node: reportingNode, + messageId, + data: { + type: '`Function`', + }, + }); } } diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-call.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-call.shot index bfc1388d6cf9..7b3ee29e7a93 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-call.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-call.shot @@ -7,24 +7,24 @@ declare const anyVar: any; declare const nestedAny: { prop: any }; anyVar(); -~~~~~~ Unsafe call of an \`any\` typed value. +~~~~~~ Unsafe call of a(n) \`any\` typed value. anyVar.a.b(); -~~~~~~~~~~ Unsafe call of an \`any\` typed value. +~~~~~~~~~~ Unsafe call of a(n) \`any\` typed value. nestedAny.prop(); -~~~~~~~~~~~~~~ Unsafe call of an \`any\` typed value. +~~~~~~~~~~~~~~ Unsafe call of a(n) \`any\` typed value. nestedAny.prop['a'](); -~~~~~~~~~~~~~~~~~~~ Unsafe call of an \`any\` typed value. +~~~~~~~~~~~~~~~~~~~ Unsafe call of a(n) \`any\` typed value. new anyVar(); -~~~~~~~~~~~~ Unsafe construction of an any type value. +~~~~~~~~~~~~ Unsafe construction of a(n) \`any\` typed value. new nestedAny.prop(); -~~~~~~~~~~~~~~~~~~~~ Unsafe construction of an any type value. +~~~~~~~~~~~~~~~~~~~~ Unsafe construction of a(n) \`any\` typed value. anyVar\`foo\`; -~~~~~~ Unsafe any typed template tag. +~~~~~~ Unsafe use of a(n) \`any\` typed template tag. nestedAny.prop\`foo\`; -~~~~~~~~~~~~~~ Unsafe any typed template tag. +~~~~~~~~~~~~~~ Unsafe use of a(n) \`any\` typed template tag. " `; @@ -44,3 +44,12 @@ new Map(); String.raw\`foo\`; " `; + +exports[`Validating rule docs no-unsafe-call.mdx code examples ESLint output 3`] = ` +"Incorrect + +const f: Function = () => {}; +f(); +~ Unsafe call of a(n) \`Function\` typed value. +" +`; diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts index 1a26a4ef3d33..258250199caa 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts @@ -44,6 +44,15 @@ function foo(x: { a?: () => void }) { x(); } `, + ` + // create a scope since it's illegal to declare a duplicate identifier + // 'Function' in the global script scope. + { + type Function = () => void; + const badFunction: Function = (() => {}) as Function; + badFunction(); + } + `, ], invalid: [ { @@ -251,5 +260,52 @@ value(); }, ], }, + { + code: ` +const t: Function = () => {}; +t(); + `, + errors: [ + { + messageId: 'unsafeCall', + line: 3, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +const f: Function = () => {}; +f\`oo\`; + `, + errors: [ + { + messageId: 'unsafeTemplateTag', + line: 3, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +declare const maybeFunction: unknown; +if (typeof maybeFunction === 'function') { + maybeFunction('call', 'with', 'any', 'args'); +} + `, + errors: [ + { + messageId: 'unsafeCall', + line: 4, + data: { + type: '`Function`', + }, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-function-type.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-function-type.test.ts index 75c116a66aa5..ae5b96d1ad51 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-function-type.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-function-type.test.ts @@ -9,8 +9,12 @@ ruleTester.run('no-unsafe-function-type', rule, { 'let value: () => void;', 'let value: (t: T) => T;', ` - type Function = () => void; - let value: Function; + // create a scope since it's illegal to declare a duplicate identifier + // 'Function' in the global script scope. + { + type Function = () => void; + let value: Function; + } `, ], invalid: [ From 9e8d85423d26c6bdc441bdbb3131f21a8ec1a0f1 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 22:17:57 -0600 Subject: [PATCH 2/6] lint --- packages/eslint-plugin/src/rules/no-unsafe-call.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 9bf464ff40a3..642b07b88bea 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -1,6 +1,5 @@ import type { TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; -import type * as ts from 'typescript'; import { createRule, From 4076849a6dee8537af51315adfc93a4bd67be473 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Wed, 18 Sep 2024 00:04:01 -0600 Subject: [PATCH 3/6] weird edge cases --- .../eslint-plugin/src/rules/no-unsafe-call.ts | 40 ++++++- .../tests/rules/no-unsafe-call.test.ts | 106 ++++++++++++++++++ .../website/src/components/lib/parseConfig.ts | 2 +- 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 642b07b88bea..4a6d51741413 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -75,7 +75,44 @@ export default createRule<[], MessageIds>({ type: isErrorType ? '`error` type' : '`any`', }, }); - } else if (isBuiltinSymbolLike(services.program, type, 'Function')) { + return; + } + + if (isBuiltinSymbolLike(services.program, type, 'Function')) { + // this also matches subtypes of `Function`, like `interface Foo extends Function {}`. + // + // For weird TS reasons that I don't understand, these are + // + // safe to construct if: + // - they have at least one call signature _that is not void-returning_, + // - OR they have at least one construct signature. + // + // safe to call (including as template) if: + // - they have at least one call signature + // - OR they have at least one construct signature. + + const constructSignatures = type.getConstructSignatures(); + if (constructSignatures.length > 0) { + return; + } + + const callSignatures = type.getCallSignatures(); + if (messageId === 'unsafeNew') { + const nonVoidReturnCallSignatures = callSignatures.filter( + signature => + !tsutils.isIntrinsicVoidType(signature.getReturnType()), + ); + + if (nonVoidReturnCallSignatures.length > 0) { + return; + } + } else { + // eslint-disable-next-line no-lonely-if -- readability + if (callSignatures.length > 0) { + return; + } + } + context.report({ node: reportingNode, messageId, @@ -83,6 +120,7 @@ export default createRule<[], MessageIds>({ type: '`Function`', }, }); + return; } } diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts index 258250199caa..167d2d5551ce 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts @@ -53,6 +53,28 @@ function foo(x: { a?: () => void }) { badFunction(); } `, + ` +interface SurprisinglySafe extends Function { + (): string; +} +declare const safe: SurprisinglySafe; +safe(); + `, + ` +interface CallGoodConstructBad extends Function { + (): void; +} +declare const safe: CallGoodConstructBad; +safe(); + `, + // Function has type FunctionConstructor, so it's not within this rule's purview + ` + new Function('lol'); + `, + // Function has type FunctionConstructor, so it's not within this rule's purview + ` + Function('lol'); + `, ], invalid: [ { @@ -307,5 +329,89 @@ if (typeof maybeFunction === 'function') { }, ], }, + { + code: ` +interface Unsafe extends Function {} +declare const unsafe: Unsafe; +unsafe(); + `, + errors: [ + { + messageId: 'unsafeCall', + line: 4, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +interface Unsafe extends Function {} +declare const unsafe: Unsafe; +unsafe\`bad\`; + `, + errors: [ + { + messageId: 'unsafeTemplateTag', + line: 4, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +interface Unsafe extends Function {} +declare const unsafe: Unsafe; +new unsafe(); + `, + errors: [ + { + messageId: 'unsafeNew', + line: 4, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +interface UnsafeToConstruct extends Function { + (): void; +} +declare const unsafe: UnsafeToConstruct; +new unsafe(); + `, + errors: [ + { + messageId: 'unsafeNew', + line: 6, + data: { + type: '`Function`', + }, + }, + ], + }, + { + code: ` +interface StillUnsafe extends Function { + property: string; +} +declare const unsafe: StillUnsafe; +unsafe(); + `, + errors: [ + { + messageId: 'unsafeCall', + line: 6, + data: { + type: '`Function`', + }, + }, + ], + }, ], }); diff --git a/packages/website/src/components/lib/parseConfig.ts b/packages/website/src/components/lib/parseConfig.ts index 2915e9f350db..29cd8d6f4a91 100644 --- a/packages/website/src/components/lib/parseConfig.ts +++ b/packages/website/src/components/lib/parseConfig.ts @@ -55,7 +55,7 @@ export function parseTSConfig(code?: string): TSConfig { const moduleRegexp = /(module\.exports\s*=)/g; function constrainedScopeEval(obj: string): unknown { - // eslint-disable-next-line @typescript-eslint/no-implied-eval + // eslint-disable-next-line @typescript-eslint/no-implied-eval, @typescript-eslint/no-unsafe-call return new Function(` "use strict"; var module = { exports: {} }; From 6de308b14455a20104fd5d1e351c8b846d58cb1e Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Wed, 18 Sep 2024 00:16:52 -0600 Subject: [PATCH 4/6] comment --- packages/eslint-plugin/src/rules/no-unsafe-call.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 4a6d51741413..98b18460adbb 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -107,7 +107,7 @@ export default createRule<[], MessageIds>({ return; } } else { - // eslint-disable-next-line no-lonely-if -- readability + // eslint-disable-next-line no-lonely-if if (callSignatures.length > 0) { return; } From 2239388c233cccd5836f38b0a0cdc9ec5cf5c04a Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Wed, 18 Sep 2024 00:24:55 -0600 Subject: [PATCH 5/6] naming --- packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts index 167d2d5551ce..50e45ce7e35a 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts @@ -49,8 +49,8 @@ function foo(x: { a?: () => void }) { // 'Function' in the global script scope. { type Function = () => void; - const badFunction: Function = (() => {}) as Function; - badFunction(); + const notGlobalFunctionType: Function = (() => {}) as Function; + notGlobalFunctionType(); } `, ` From d1e954c63e4fa1a1f15529acc033cb3d1940aedf Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Wed, 18 Sep 2024 08:47:28 -0600 Subject: [PATCH 6/6] feedback --- .../eslint-plugin/src/rules/no-unsafe-call.ts | 19 ++++++++----------- .../tests/rules/no-unsafe-call.test.ts | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 98b18460adbb..8149f2736a15 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -98,19 +98,16 @@ export default createRule<[], MessageIds>({ const callSignatures = type.getCallSignatures(); if (messageId === 'unsafeNew') { - const nonVoidReturnCallSignatures = callSignatures.filter( - signature => - !tsutils.isIntrinsicVoidType(signature.getReturnType()), - ); - - if (nonVoidReturnCallSignatures.length > 0) { - return; - } - } else { - // eslint-disable-next-line no-lonely-if - if (callSignatures.length > 0) { + if ( + callSignatures.some( + signature => + !tsutils.isIntrinsicVoidType(signature.getReturnType()), + ) + ) { return; } + } else if (callSignatures.length > 0) { + return; } context.report({ diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts index 50e45ce7e35a..8baf831f96e6 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts @@ -65,6 +65,21 @@ interface CallGoodConstructBad extends Function { (): void; } declare const safe: CallGoodConstructBad; +safe(); + `, + ` +interface ConstructSignatureMakesSafe extends Function { + new (): ConstructSignatureMakesSafe; +} +declare const safe: ConstructSignatureMakesSafe; +new safe(); + `, + ` +interface SafeWithNonVoidCallSignature extends Function { + (): void; + (x: string): string; +} +declare const safe: SafeWithNonVoidCallSignature; safe(); `, // Function has type FunctionConstructor, so it's not within this rule's purview

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!