From 75812fcd26d7f7231943fec87d13cd81a3782be6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 19 Feb 2024 22:32:44 +0200 Subject: [PATCH 01/37] feat(eslint-plugin): [no-misused-spread] add new rule Closes #748 --- .../docs/rules/no-misused-spread.md | 57 ++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/configs/disable-type-checked.ts | 1 + .../src/configs/strict-type-checked.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-misused-spread.ts | 160 ++++ .../tests/rules/no-misused-spread.test.ts | 735 ++++++++++++++++++ .../schema-snapshots/no-misused-spread.shot | 14 + 8 files changed, 971 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-misused-spread.md create mode 100644 packages/eslint-plugin/src/rules/no-misused-spread.ts create mode 100644 packages/eslint-plugin/tests/rules/no-misused-spread.test.ts create mode 100644 packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.md new file mode 100644 index 000000000000..3489727ef238 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.md @@ -0,0 +1,57 @@ +--- +description: "Disallow using the spread operator on types that can't be spread." +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-misused-spread** for documentation. + +The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript, but it can only be used with types that support it. This rule disallows using the spread operator on types that can't be spread, or types that spreading them in specific cases can lead to unexpected behavior. + +## Examples + + + +### ❌ Incorrect + +```ts +declare const name: string; +const chars = [...name]; + +declare const arr: number[]; +const arrSpread = { ...arr }; + +declare const set: Set; +const setSpread = { ...set }; + +declare const map: Map; +const mapSpread = { ...map }; + +declare function getName(): string; +const getNameSpread = { ...getName }; +``` + +### ✅ Correct + +```ts +declare const name: string; +const chars = name.split(''); + +declare const arr: number[]; +const arrSpread = [...arr]; + +declare const set: Set; +const setSpread = [...set]; + +declare const map: Map; +const mapSpread = [...map]; + +declare function getName(): string; +const getNameSpread = { ...getName() }; +``` + + + +## When Not To Use It + +When you want to allow using the spread operator on types that can't be spread. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index b1890165c7ad..49fc6e0537e1 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -74,6 +74,7 @@ export = { '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 09a5c07fd3e7..3982906387af 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -23,6 +23,7 @@ export = { '@typescript-eslint/no-implied-eval': 'off', '@typescript-eslint/no-meaningless-void-operator': 'off', '@typescript-eslint/no-misused-promises': 'off', + '@typescript-eslint/no-misused-spread': 'off', '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-throw-literal': 'off', diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts index 5666c64035da..fa4012d05a5d 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked.ts @@ -34,6 +34,7 @@ export = { '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index e497019debec..ab689d3f8227 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -64,6 +64,7 @@ import noMagicNumbers from './no-magic-numbers'; import noMeaninglessVoidOperator from './no-meaningless-void-operator'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; +import noMisusedSpread from './no-misused-spread'; import noMixedEnums from './no-mixed-enums'; import noNamespace from './no-namespace'; import noNonNullAssertedNullishCoalescing from './no-non-null-asserted-nullish-coalescing'; @@ -206,6 +207,7 @@ export default { 'no-meaningless-void-operator': noMeaninglessVoidOperator, 'no-misused-new': noMisusedNew, 'no-misused-promises': noMisusedPromises, + 'no-misused-spread': noMisusedSpread, 'no-mixed-enums': noMixedEnums, 'no-namespace': noNamespace, 'no-non-null-asserted-nullish-coalescing': noNonNullAssertedNullishCoalescing, diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts new file mode 100644 index 000000000000..0e4f990573d8 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -0,0 +1,160 @@ +import { type TSESTree } from '@typescript-eslint/utils'; +import * as tsutils from 'ts-api-utils'; +import * as ts from 'typescript'; + +import { + createRule, + getConstrainedTypeAtLocation, + getParserServices, + isBuiltinSymbolLike, + isTypeFlagSet, +} from '../util'; + +type MessageIds = + | 'noStringSpreadInArray' + | 'noSpreadInObject' + | 'noFunctionSpreadInObject'; + +export default createRule<[], MessageIds>({ + name: 'no-misused-spread', + meta: { + type: 'problem', + docs: { + description: + "Disallow using the spread operator on types that can't be spread", + recommended: 'strict', + requiresTypeChecking: true, + }, + messages: { + noStringSpreadInArray: + "Using the spread operator on a string is not allowed in an array. Use `String.split('')` instead.", + + noSpreadInObject: + 'Using the spread operator on `{{type}}` is not allowed in an object.', + + noFunctionSpreadInObject: + 'Using the spread operator on `Function` without properties is not allowed in an object. Did you forget to call the function?', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + + function checkArraySpread(node: TSESTree.SpreadElement): void { + const type = getConstrainedTypeAtLocation(services, node.argument); + + if (isString(type, checker)) { + context.report({ + node, + messageId: 'noStringSpreadInArray', + }); + + return; + } + } + + function checkObjectSpread(node: TSESTree.SpreadElement): void { + const type = getConstrainedTypeAtLocation(services, node.argument); + + if (isArray(type, checker)) { + context.report({ + node, + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + }); + + return; + } + + if (isBuiltinSymbolLike(services.program, type, 'Set')) { + context.report({ + node, + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + }); + + return; + } + + if (isBuiltinSymbolLike(services.program, type, 'Map')) { + context.report({ + node, + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + }); + + return; + } + + const symbol = type.getSymbol(); + + if ( + symbol && + tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Function) && + type.getProperties().length === 0 + ) { + context.report({ + node, + messageId: 'noFunctionSpreadInObject', + }); + + return; + } + + if (isIterable(type, checker)) { + context.report({ + node, + messageId: 'noSpreadInObject', + data: { + type: 'Iterator', + }, + }); + + return; + } + } + + return { + 'ArrayExpression > SpreadElement': checkArraySpread, + 'ObjectExpression > SpreadElement': checkObjectSpread, + }; + }, +}); + +function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { + if (type.isUnion()) { + return type.types.every(t => isArray(t, checker)); + } + + return checker.isArrayType(type) || checker.isTupleType(type); +} + +function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { + if (type.isUnion()) { + return type.types.every(t => isIterable(t, checker)); + } + + const iterator = tsutils.getWellKnownSymbolPropertyOfType( + type, + 'iterator', + checker, + ); + + return iterator !== undefined; +} + +function isString(type: ts.Type, checker: ts.TypeChecker): boolean { + if (type.isIntersection()) { + return type.types.some(t => isString(t, checker)); + } + + return isTypeFlagSet(type, ts.TypeFlags.StringLike); +} diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts new file mode 100644 index 000000000000..5085f24a46e4 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -0,0 +1,735 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/no-misused-spread'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('no-misused-spread', rule, { + valid: [ + 'const a = [...[1, 2, 3]];', + 'const a = [...([1, 2, 3] as const)];', + + ` + declare const data: any; + const a = [...data]; + `, + + ` + declare const data: number[] | any; + const a = [...data]; + `, + + ` + declare const data: number[] & any; + const a = [...data]; + `, + + ` + const a = [1, 2, 3]; + const b = [...a]; + `, + + ` + const a = [1, 2, 3] as const; + const b = [...a]; + `, + + ` + declare function getArray(): number[]; + const a = [...getArray()]; + `, + + ` + declare function getTuple(): readonly number[]; + const a = [...getTuple()]; + `, + + ` + const iterator = { + *[Symbol.iterator]() { + yield 1; + yield 2; + yield 3; + }, + }; + + const a = [...iterator]; + `, + + ` + declare const data: Iterable | number[]; + + const a = [...data]; + `, + + ` + declare const data: Iterable & number[]; + + const a = [...data]; + `, + + ` + declare function getIterable(): Iterable; + + const a = [...getIterable()]; + `, + + 'const o = { ...{ a: 1, b: 2 } };', + + 'const o = { ...({ a: 1, b: 2 } as const) };', + + ` + declare const obj: any; + + const o = { ...obj }; + `, + + ` + declare const obj: { a: number; b: number } | any; + + const o = { ...obj }; + `, + + ` + declare const obj: { a: number; b: number } & any; + + const o = { ...obj }; + `, + + ` + const obj = { a: 1, b: 2 }; + const o = { ...obj }; + `, + + ` + declare const obj: { a: number; b: number }; + const o = { ...obj }; + `, + + ` + declare function getObject(): { a: number; b: number }; + const o = { ...getObject() }; + `, + + ` + class A { + a = 1; + b = 2; + } + + const o = { ...new A() }; + `, + + ` + class A { + a = 1; + b = 2; + } + + const a = new A(); + + const o = { ...a }; + `, + + ` + function f() {} + + f.prop = 1; + + const o = { ...f }; + `, + + ` + const f = () => {}; + + f.prop = 1; + + const o = { ...f }; + `, + + ` + function* generator() {} + + generator.prop = 1; + + const o = { ...generator }; + `, + ], + + invalid: [ + { + code: "const a = [...'test'];", + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 1, + column: 12, + endColumn: 21, + }, + ], + }, + + { + code: ` + const test = 'hello'; + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + + { + code: ` + const test = \`he\${'ll'}o\`; + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const test: string; + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const test: string | number[]; + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const test: string & { __brand: 'test' }; + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare function getString(): string; + const a = [...getString()]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 34, + }, + ], + }, + + { + code: ` + declare function getString(): T; + const a = [...getString()]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 34, + }, + ], + }, + + { + code: ` + declare function getString(): string & { __brand: 'test' }; + const a = [...getString()]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 34, + }, + ], + }, + + { + code: 'const o = { ...[1, 2, 3] };', + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 1, + column: 13, + endColumn: 25, + }, + ], + }, + + { + code: ` + const arr = [1, 2, 3]; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + const arr = [1, 2, 3] as const; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const arr: number[]; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const arr: readonly number[]; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const arr: number[] | string[]; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare function getArray(): number[]; + const o = { ...getArray() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 34, + }, + ], + }, + + { + code: ` + declare function getArray(): readonly number[]; + const o = { ...getArray() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 34, + }, + ], + }, + + { + code: 'const o = { ...new Set([1, 2, 3]) };', + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 1, + column: 13, + endColumn: 34, + }, + ], + }, + + { + code: ` + const set = new Set([1, 2, 3]); + const o = { ...set }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const set: Set; + const o = { ...set }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare function getSet(): Set; + const o = { ...getSet() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 32, + }, + ], + }, + + { + code: ` + const o = { + ...new Map([ + ['test-1', 1], + ['test-2', 2], + ]), + }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 3, + column: 11, + endLine: 6, + endColumn: 13, + }, + ], + }, + + { + code: ` + const map = new Map([ + ['test-1', 1], + ['test-2', 2], + ]); + + const o = { ...map }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 7, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare const map: Map; + const o = { ...map }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + + { + code: ` + declare function getMap(): Map; + const o = { ...getMap() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 3, + column: 21, + endColumn: 32, + }, + ], + }, + + { + code: ` + function f() {} + + const o = { ...f }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + const f = () => {}; + + const o = { ...f }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + declare function f(): void; + + const o = { ...f }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + // TODO: Make this test pass. + skip: true, + code: ` + declare function getFunction(): () => void; + + const o = { ...getFunction() }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 37, + }, + ], + }, + + { + code: ` + function* generator() {} + + const o = { ...generator }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 33, + }, + ], + }, + + { + code: ` + const iterator = { + *[Symbol.iterator]() { + yield 'test'; + }, + }; + + const o = { ...iterator }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Iterator', + }, + line: 8, + column: 21, + endColumn: 32, + }, + ], + }, + + { + code: ` + declare const iterator: Iterable; + + const o = { ...iterator }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Iterator', + }, + line: 4, + column: 21, + endColumn: 32, + }, + ], + }, + + { + code: ` + declare function getIterable(): Iterable; + + const o = { ...getIterable() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Iterator', + }, + line: 4, + column: 21, + endColumn: 37, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot new file mode 100644 index 000000000000..2f6b196189c8 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes no-misused-spread 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; From f18acb8501a441b69f705648f1a6fc32ba1b9a03 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 19 Feb 2024 22:41:50 +0200 Subject: [PATCH 02/37] fix docs --- .../docs/rules/no-misused-spread.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.md index 3489727ef238..85a2ba89f90b 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.md +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.md @@ -15,8 +15,8 @@ The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Re ### ❌ Incorrect ```ts -declare const name: string; -const chars = [...name]; +declare const userName: string; +const chars = [...userName]; declare const arr: number[]; const arrSpread = { ...arr }; @@ -27,15 +27,15 @@ const setSpread = { ...set }; declare const map: Map; const mapSpread = { ...map }; -declare function getName(): string; -const getNameSpread = { ...getName }; +declare function getObj(): { a: 1; b: 2 }; +const getObjSpread = { ...getObj }; ``` ### ✅ Correct ```ts -declare const name: string; -const chars = name.split(''); +declare const userName: string; +const chars = userName.split(''); declare const arr: number[]; const arrSpread = [...arr]; @@ -46,8 +46,8 @@ const setSpread = [...set]; declare const map: Map; const mapSpread = [...map]; -declare function getName(): string; -const getNameSpread = { ...getName() }; +declare function getObj(): { a: 1; b: 2 }; +const getObjSpread = { ...getObj() }; ``` From 22a50e5094d688733732ea5434a4e1a727ec2ea1 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 19 Feb 2024 22:53:58 +0200 Subject: [PATCH 03/37] add tests --- .../src/rules/no-misused-spread.ts | 41 ++++++++--- .../tests/rules/no-misused-spread.test.ts | 70 +++++++++++++++++++ 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 0e4f990573d8..975d32276fca 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -70,7 +70,7 @@ export default createRule<[], MessageIds>({ return; } - if (isBuiltinSymbolLike(services.program, type, 'Set')) { + if (isBuiltinSymbol(services.program, type, 'Set')) { context.report({ node, messageId: 'noSpreadInObject', @@ -82,7 +82,7 @@ export default createRule<[], MessageIds>({ return; } - if (isBuiltinSymbolLike(services.program, type, 'Map')) { + if (isBuiltinSymbol(services.program, type, 'Map')) { context.report({ node, messageId: 'noSpreadInObject', @@ -94,13 +94,7 @@ export default createRule<[], MessageIds>({ return; } - const symbol = type.getSymbol(); - - if ( - symbol && - tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Function) && - type.getProperties().length === 0 - ) { + if (isFunctionWithoutProps(type, checker)) { context.report({ node, messageId: 'noFunctionSpreadInObject', @@ -158,3 +152,32 @@ function isString(type: ts.Type, checker: ts.TypeChecker): boolean { return isTypeFlagSet(type, ts.TypeFlags.StringLike); } + +function isBuiltinSymbol( + program: ts.Program, + type: ts.Type, + symbolName: string, +): boolean { + if (type.isUnion()) { + return type.types.some(t => isBuiltinSymbol(program, t, symbolName)); + } + + return isBuiltinSymbolLike(program, type, symbolName); +} + +function isFunctionWithoutProps( + type: ts.Type, + checker: ts.TypeChecker, +): boolean { + if (type.isUnion()) { + return type.types.some(t => isFunctionWithoutProps(t, checker)); + } + + const symbol = type.getSymbol(); + + return ( + !!symbol && + tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Function) && + type.getProperties().length === 0 + ); +} diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 5085f24a46e4..3a9e22d7b0c6 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -490,6 +490,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const set: Set | { a: number }; + const o = { ...set }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare function getSet(): Set; @@ -571,6 +589,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const map: Map | { a: number }; + const o = { ...map }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare function getMap(): Map; @@ -655,6 +691,40 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + skip: true, + code: ` + declare const f: () => void; + + const o = { ...f }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + skip: true, + code: ` + declare const f: () => void | { a: number }; + + const o = { ...f }; + `, + errors: [ + { + messageId: 'noFunctionSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + { code: ` function* generator() {} From 985795a3039a54633effb190b53c98e9fd583290 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 19 Feb 2024 22:57:28 +0200 Subject: [PATCH 04/37] wip --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 975d32276fca..b80f9d80c6b6 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -125,7 +125,7 @@ export default createRule<[], MessageIds>({ function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { if (type.isUnion()) { - return type.types.every(t => isArray(t, checker)); + return type.types.some(t => isArray(t, checker)); } return checker.isArrayType(type) || checker.isTupleType(type); @@ -133,7 +133,7 @@ function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { if (type.isUnion()) { - return type.types.every(t => isIterable(t, checker)); + return type.types.some(t => isIterable(t, checker)); } const iterator = tsutils.getWellKnownSymbolPropertyOfType( From 3ff215faf388ebf50b0a4114dc5ff1442f6ef664 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 20 Feb 2024 16:54:49 +0200 Subject: [PATCH 05/37] fix functions --- .../src/rules/no-misused-spread.ts | 6 +---- .../tests/rules/no-misused-spread.test.ts | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index b80f9d80c6b6..c089d3bc7210 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -173,11 +173,7 @@ function isFunctionWithoutProps( return type.types.some(t => isFunctionWithoutProps(t, checker)); } - const symbol = type.getSymbol(); - return ( - !!symbol && - tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Function) && - type.getProperties().length === 0 + type.getCallSignatures().length > 0 && type.getProperties().length === 0 ); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 3a9e22d7b0c6..31247c426edd 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -674,8 +674,6 @@ ruleTester.run('no-misused-spread', rule, { }, { - // TODO: Make this test pass. - skip: true, code: ` declare function getFunction(): () => void; @@ -692,7 +690,6 @@ ruleTester.run('no-misused-spread', rule, { }, { - skip: true, code: ` declare const f: () => void; @@ -709,7 +706,6 @@ ruleTester.run('no-misused-spread', rule, { }, { - skip: true, code: ` declare const f: () => void | { a: number }; @@ -783,6 +779,25 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const iterator: Iterable | { a: number }; + + const o = { ...iterator }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Iterator', + }, + line: 4, + column: 21, + endColumn: 32, + }, + ], + }, + { code: ` declare function getIterable(): Iterable; From 990006f144cd4c3c100d2d833fc50d175e42b027 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 21 Feb 2024 07:52:27 +0200 Subject: [PATCH 06/37] change type name to `Iterable` --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 2 +- .../eslint-plugin/tests/rules/no-misused-spread.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index c089d3bc7210..80c23cf8b35f 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -108,7 +108,7 @@ export default createRule<[], MessageIds>({ node, messageId: 'noSpreadInObject', data: { - type: 'Iterator', + type: 'Iterable', }, }); diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 31247c426edd..ca0a64662503 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -751,7 +751,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Iterator', + type: 'Iterable', }, line: 8, column: 21, @@ -770,7 +770,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Iterator', + type: 'Iterable', }, line: 4, column: 21, @@ -789,7 +789,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Iterator', + type: 'Iterable', }, line: 4, column: 21, @@ -808,7 +808,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Iterator', + type: 'Iterable', }, line: 4, column: 21, From 49692d261d6f96089d2ab5e418ec9551d90a2f01 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 27 Feb 2024 17:54:17 +0200 Subject: [PATCH 07/37] wip --- .../docs/rules/no-misused-spread.md | 10 ++- .../src/rules/no-misused-spread.ts | 22 +++-- .../tests/rules/no-misused-spread.test.ts | 84 +++++++++++++++++++ 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.md index 85a2ba89f90b..f496ddaff679 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.md +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.md @@ -1,12 +1,12 @@ --- -description: "Disallow using the spread operator on types that can't be spread." +description: 'Disallow using the spread operator when it might cause unexpected behavior.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 > > See **https://typescript-eslint.io/rules/no-misused-spread** for documentation. -The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript, but it can only be used with types that support it. This rule disallows using the spread operator on types that can't be spread, or types that spreading them in specific cases can lead to unexpected behavior. +The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript, but it can only be used with types that support it. This rule disallows using the spread operator on types where spreading can lead to unexpected behavior. ## Examples @@ -54,4 +54,8 @@ const getObjSpread = { ...getObj() }; ## When Not To Use It -When you want to allow using the spread operator on types that can't be spread. +If you intentionally want to use the spread operator in those cases, and expect +the specific behavior that comes with it, you might not want this rule. +For example, when you want to spread an array into an object and expect the +result to be an object with the array elements as values and the array indices +as keys. diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 80c23cf8b35f..65f844283094 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -21,7 +21,7 @@ export default createRule<[], MessageIds>({ type: 'problem', docs: { description: - "Disallow using the spread operator on types that can't be spread", + 'Disallow using the spread operator when it might cause unexpected behavior', recommended: 'strict', requiresTypeChecking: true, }, @@ -70,7 +70,10 @@ export default createRule<[], MessageIds>({ return; } - if (isBuiltinSymbol(services.program, type, 'Set')) { + if ( + isBuiltinSymbol(services.program, type, 'Set') || + isBuiltinSymbol(services.program, type, 'ReadonlySet') + ) { context.report({ node, messageId: 'noSpreadInObject', @@ -82,7 +85,10 @@ export default createRule<[], MessageIds>({ return; } - if (isBuiltinSymbol(services.program, type, 'Map')) { + if ( + isBuiltinSymbol(services.program, type, 'Map') || + isBuiltinSymbol(services.program, type, 'ReadonlyMap') + ) { context.report({ node, messageId: 'noSpreadInObject', @@ -124,7 +130,7 @@ export default createRule<[], MessageIds>({ }); function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion()) { + if (type.isUnion() || type.isIntersection()) { return type.types.some(t => isArray(t, checker)); } @@ -132,7 +138,7 @@ function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { } function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion()) { + if (type.isUnion() || type.isIntersection()) { return type.types.some(t => isIterable(t, checker)); } @@ -146,7 +152,7 @@ function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { } function isString(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isIntersection()) { + if (type.isUnion() || type.isIntersection()) { return type.types.some(t => isString(t, checker)); } @@ -158,7 +164,7 @@ function isBuiltinSymbol( type: ts.Type, symbolName: string, ): boolean { - if (type.isUnion()) { + if (type.isUnion() || type.isIntersection()) { return type.types.some(t => isBuiltinSymbol(program, t, symbolName)); } @@ -169,7 +175,7 @@ function isFunctionWithoutProps( type: ts.Type, checker: ts.TypeChecker, ): boolean { - if (type.isUnion()) { + if (type.isUnion() || type.isIntersection()) { return type.types.some(t => isFunctionWithoutProps(t, checker)); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index ca0a64662503..a32a37de90f2 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -163,6 +163,18 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...generator }; `, + + ` + declare const promise: Promise; + + const o = { ...promise }; + `, + + ` + declare const promise: PromiseLike; + + const o = { ...promise }; + `, ], invalid: [ @@ -403,6 +415,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const arr: number[] & string[]; + const o = { ...arr }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Array', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare function getArray(): number[]; @@ -490,6 +520,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const set: ReadonlySet; + const o = { ...set }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare const set: Set | { a: number }; @@ -589,6 +637,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const map: ReadonlyMap; + const o = { ...map }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Map', + }, + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare const map: Map | { a: number }; @@ -625,6 +691,24 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const a: Map & Set; + const o = { ...a }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Set', + }, + line: 3, + column: 21, + endColumn: 25, + }, + ], + }, + { code: ` function f() {} From c45038f48ea997926d8cda65be0f9d606e00707d Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 27 Feb 2024 18:24:12 +0200 Subject: [PATCH 08/37] fix lint --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 65f844283094..8983de4b56f6 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -1,4 +1,4 @@ -import { type TSESTree } from '@typescript-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; From 9f07bc1701c7656f92bb389f40122d647a9c3fc6 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 8 Mar 2024 16:27:52 +0200 Subject: [PATCH 09/37] wip --- .../docs/rules/no-misused-spread.md | 39 ++ .../src/configs/strict-type-checked-only.ts | 1 + .../src/rules/no-misused-spread.ts | 86 +++- .../tests/rules/no-misused-spread.test.ts | 372 ++++++++++++++++-- .../schema-snapshots/no-misused-spread.shot | 22 +- 5 files changed, 483 insertions(+), 37 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.md index f496ddaff679..bf4866565177 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.md +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.md @@ -52,6 +52,45 @@ const getObjSpread = { ...getObj() }; +## Options + +### `allowClassInstances` + +By default, this rule disallows using the spread operator on instances of classes: + + + +#### ❌ Incorrect + +```ts option='{ "allowClassInstances": false }' +class User { + name: string; + + constructor(name: string) { + this.name = name; + } +} + +const user = new User('John'); + +const userSpread = { ...user }; +``` + + + +If you want to allow this behavior, you can configure the rule with `"allowClassInstances": true`: + +```json +{ + "@typescript-eslint/no-misused-spread": [ + "error", + { + "allowClassInstances": true + } + ] +} +``` + ## When Not To Use It If you intentionally want to use the spread operator in those cases, and expect diff --git a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts index 4188f5f73465..47b22ffc3367 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts @@ -21,6 +21,7 @@ export = { '@typescript-eslint/no-implied-eval': 'error', '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-redundant-type-constituents': 'error', 'no-throw-literal': 'off', diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 8983de4b56f6..d855ca3cba2b 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -7,15 +7,23 @@ import { getConstrainedTypeAtLocation, getParserServices, isBuiltinSymbolLike, + isPromiseLike, isTypeFlagSet, } from '../util'; +type Options = [ + { + allowClassInstances: boolean; + }, +]; + type MessageIds = | 'noStringSpreadInArray' | 'noSpreadInObject' - | 'noFunctionSpreadInObject'; + | 'noFunctionSpreadInObject' + | 'noClassSpreadInObject'; -export default createRule<[], MessageIds>({ +export default createRule({ name: 'no-misused-spread', meta: { type: 'problem', @@ -27,18 +35,39 @@ export default createRule<[], MessageIds>({ }, messages: { noStringSpreadInArray: - "Using the spread operator on a string is not allowed in an array. Use `String.split('')` instead.", + "Using the spread operator on a string can cause unexpected behavior. Prefer `String.split('')` instead.", noSpreadInObject: - 'Using the spread operator on `{{type}}` is not allowed in an object.', + 'Using the spread operator on `{{type}}` can cause unexpected behavior.', noFunctionSpreadInObject: - 'Using the spread operator on `Function` without properties is not allowed in an object. Did you forget to call the function?', + 'Using the spread operator on `Function` without additional properties can cause unexpected behavior. Did you forget to call the function?', + + noClassSpreadInObject: + 'Using the spread operator on class instances can cause unexpected behavior.', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + allowClassInstances: { + description: + 'Whether to allow spreading class instances in objects.', + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], }, - defaultOptions: [], - create(context) { + + defaultOptions: [ + { + allowClassInstances: false, + }, + ], + + create(context, [options]) { const services = getParserServices(context); const checker = services.program.getTypeChecker(); @@ -100,6 +129,18 @@ export default createRule<[], MessageIds>({ return; } + if (isPromise(services.program, type, checker)) { + context.report({ + node, + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + }); + + return; + } + if (isFunctionWithoutProps(type, checker)) { context.report({ node, @@ -120,6 +161,15 @@ export default createRule<[], MessageIds>({ return; } + + if (!options.allowClassInstances && isClassInstance(type, checker)) { + context.report({ + node, + messageId: 'noClassSpreadInObject', + }); + + return; + } } return { @@ -183,3 +233,23 @@ function isFunctionWithoutProps( type.getCallSignatures().length > 0 && type.getProperties().length === 0 ); } + +function isPromise( + program: ts.Program, + type: ts.Type, + checker: ts.TypeChecker, +): boolean { + if (type.isUnion() || type.isIntersection()) { + return type.types.some(t => isPromise(program, t, checker)); + } + + return isPromiseLike(program, type); +} + +function isClassInstance(type: ts.Type, checker: ts.TypeChecker): boolean { + if (type.isUnion() || type.isIntersection()) { + return type.types.some(t => isClassInstance(t, checker)); + } + + return type.isClassOrInterface(); +} diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index a32a37de90f2..48a0cee1bd50 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -120,26 +120,6 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...getObject() }; `, - ` - class A { - a = 1; - b = 2; - } - - const o = { ...new A() }; - `, - - ` - class A { - a = 1; - b = 2; - } - - const a = new A(); - - const o = { ...a }; - `, - ` function f() {} @@ -165,16 +145,90 @@ ruleTester.run('no-misused-spread', rule, { `, ` - declare const promise: Promise; + declare const promiseLike: PromiseLike; - const o = { ...promise }; + const o = { ...promiseLike }; `, - ` - declare const promise: PromiseLike; + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + public b = 2; + private c = 3; + protected d = 4; + static e = 5; + } + + const o = { ...new A() }; + `, + }, - const o = { ...promise }; - `, + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + } + + const a = new A(); + + const o = { ...a }; + `, + }, + + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + } + + declare const a: A; + + const o = { ...a }; + `, + }, + + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + } + + class B extends A {} + + const o = { ...new B() }; + `, + }, + + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + } + + declare const a: A | { b: string }; + + const o = { ...a }; + `, + }, + + { + options: [{ allowClassInstances: true }], + code: ` + class A { + a = 1; + } + + declare const a: A & { b: string }; + + const o = { ...a }; + `, + }, ], invalid: [ @@ -709,6 +763,96 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const promise: Promise; + const o = { ...promise }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + line: 3, + column: 21, + endColumn: 31, + }, + ], + }, + + { + code: ` + declare const maybePromise: Promise | { a: number }; + const o = { ...maybePromise }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + line: 3, + column: 21, + endColumn: 36, + }, + ], + }, + + { + code: ` + declare const promise: Promise & { a: number }; + const o = { ...promise }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + line: 3, + column: 21, + endColumn: 31, + }, + ], + }, + + { + code: ` + declare function getPromise(): Promise; + const o = { ...getPromise() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + line: 3, + column: 21, + endColumn: 36, + }, + ], + }, + + { + code: ` + declare function getPromise>(arg: T): T; + const o = { ...getPromise() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Promise', + }, + line: 3, + column: 21, + endColumn: 36, + }, + ], + }, + { code: ` function f() {} @@ -900,5 +1044,181 @@ ruleTester.run('no-misused-spread', rule, { }, ], }, + + { + code: ` + const o = { ...new Date() }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 2, + column: 21, + endColumn: 34, + }, + ], + }, + + { + code: ` + class A { + a = 1; + public b = 2; + private c = 3; + protected d = 4; + static e = 5; + } + + const o = { ...new A() }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 10, + column: 21, + endColumn: 31, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + const a = new A(); + + const o = { ...a }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + declare const a: A; + + const o = { ...a }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + declare function getA(): A; + + const o = { ...getA() }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 30, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + declare function getA(arg: T): T; + + const o = { ...getA() }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 30, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + class B extends A {} + + const o = { ...new B() }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 31, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + declare const a: A | { b: string }; + + const o = { ...a }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + class A { + a = 1; + } + + declare const a: A & { b: string }; + + const o = { ...a }; + `, + errors: [ + { + messageId: 'noClassSpreadInObject', + line: 8, + column: 21, + endColumn: 25, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot index 2f6b196189c8..fc9ba84d9b1f 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot @@ -4,11 +4,27 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos " # SCHEMA: -[] +[ + { + "additionalProperties": false, + "properties": { + "allowClassInstances": { + "description": "Whether to allow spreading class instances in objects.", + "type": "boolean" + } + }, + "type": "object" + } +] # TYPES: -/** No options declared */ -type Options = [];" +type Options = [ + { + /** Whether to allow spreading class instances in objects. */ + allowClassInstances?: boolean; + }, +]; +" `; From b97a397b57bd049aa1e4821a26258d18e7552d82 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 10:21:21 +0300 Subject: [PATCH 10/37] wip --- .../docs/rules/no-misused-spread.md | 2 +- .../src/rules/no-misused-spread.ts | 134 ++++---------- .../tests/rules/no-misused-spread.test.ts | 174 +++++++++++++++--- 3 files changed, 191 insertions(+), 119 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.md index bf4866565177..8d2946509c96 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.md +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.md @@ -6,7 +6,7 @@ description: 'Disallow using the spread operator when it might cause unexpected > > See **https://typescript-eslint.io/rules/no-misused-spread** for documentation. -The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript, but it can only be used with types that support it. This rule disallows using the spread operator on types where spreading can lead to unexpected behavior. +The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript that can be misused in ways not always detectable by TypeScript. This rule disallows using the spread operator on types where spreading can lead to unexpected behavior. ## Examples diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index d855ca3cba2b..6ff1aefb4409 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -1,4 +1,5 @@ import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; @@ -6,7 +7,6 @@ import { createRule, getConstrainedTypeAtLocation, getParserServices, - isBuiltinSymbolLike, isPromiseLike, isTypeFlagSet, } from '../util'; @@ -21,7 +21,8 @@ type MessageIds = | 'noStringSpreadInArray' | 'noSpreadInObject' | 'noFunctionSpreadInObject' - | 'noClassSpreadInObject'; + | 'noClassInstanceSpreadInObject' + | 'noClassDeclarationSpreadInObject'; export default createRule({ name: 'no-misused-spread', @@ -38,13 +39,16 @@ export default createRule({ "Using the spread operator on a string can cause unexpected behavior. Prefer `String.split('')` instead.", noSpreadInObject: - 'Using the spread operator on `{{type}}` can cause unexpected behavior.', + 'Using the spread operator on `{{type}}` in an object can cause unexpected behavior.', noFunctionSpreadInObject: - 'Using the spread operator on `Function` without additional properties can cause unexpected behavior. Did you forget to call the function?', + 'Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function?', - noClassSpreadInObject: - 'Using the spread operator on class instances can cause unexpected behavior.', + noClassInstanceSpreadInObject: + 'Using the spread operator on class instances without `[Symbol.iterator]` can cause unexpected behavior.', + + noClassDeclarationSpreadInObject: + 'Using the spread operator on class declarations can cause unexpected behavior. Did you forget to instantiate the class?', }, schema: [ { @@ -74,7 +78,7 @@ export default createRule({ function checkArraySpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - if (isString(type, checker)) { + if (isString(type)) { context.report({ node, messageId: 'noStringSpreadInArray', @@ -99,73 +103,52 @@ export default createRule({ return; } - if ( - isBuiltinSymbol(services.program, type, 'Set') || - isBuiltinSymbol(services.program, type, 'ReadonlySet') - ) { + if (isPromise(services.program, type)) { context.report({ node, messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Promise', }, }); return; } - if ( - isBuiltinSymbol(services.program, type, 'Map') || - isBuiltinSymbol(services.program, type, 'ReadonlyMap') - ) { + if (isFunctionWithoutProps(type)) { context.report({ node, - messageId: 'noSpreadInObject', - data: { - type: 'Map', - }, + messageId: 'noFunctionSpreadInObject', }); return; } - if (isPromise(services.program, type, checker)) { + if (isIterable(type, checker)) { context.report({ node, messageId: 'noSpreadInObject', data: { - type: 'Promise', + type: 'Iterable', }, }); return; } - if (isFunctionWithoutProps(type, checker)) { - context.report({ - node, - messageId: 'noFunctionSpreadInObject', - }); - - return; - } - - if (isIterable(type, checker)) { + if (!options.allowClassInstances && isClassInstance(type)) { context.report({ node, - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noClassInstanceSpreadInObject', }); return; } - if (!options.allowClassInstances && isClassInstance(type, checker)) { + if (node.argument.type === AST_NODE_TYPES.ClassExpression) { context.report({ node, - messageId: 'noClassSpreadInObject', + messageId: 'noClassDeclarationSpreadInObject', }); return; @@ -188,68 +171,31 @@ function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { } function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isIterable(t, checker)); - } - - const iterator = tsutils.getWellKnownSymbolPropertyOfType( - type, - 'iterator', - checker, - ); - - return iterator !== undefined; + return tsutils + .typeParts(type) + .some( + t => !!tsutils.getWellKnownSymbolPropertyOfType(t, 'iterator', checker), + ); } -function isString(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isString(t, checker)); - } - - return isTypeFlagSet(type, ts.TypeFlags.StringLike); +function isString(type: ts.Type): boolean { + return tsutils + .typeParts(type) + .some(t => !!isTypeFlagSet(t, ts.TypeFlags.StringLike)); } -function isBuiltinSymbol( - program: ts.Program, - type: ts.Type, - symbolName: string, -): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isBuiltinSymbol(program, t, symbolName)); - } - - return isBuiltinSymbolLike(program, type, symbolName); +function isFunctionWithoutProps(type: ts.Type): boolean { + return tsutils + .typeParts(type) + .some( + t => t.getCallSignatures().length > 0 && t.getProperties().length === 0, + ); } -function isFunctionWithoutProps( - type: ts.Type, - checker: ts.TypeChecker, -): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isFunctionWithoutProps(t, checker)); - } - - return ( - type.getCallSignatures().length > 0 && type.getProperties().length === 0 - ); -} - -function isPromise( - program: ts.Program, - type: ts.Type, - checker: ts.TypeChecker, -): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isPromise(program, t, checker)); - } - - return isPromiseLike(program, type); +function isPromise(program: ts.Program, type: ts.Type): boolean { + return tsutils.typeParts(type).some(t => isPromiseLike(program, t)); } -function isClassInstance(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isClassInstance(t, checker)); - } - - return type.isClassOrInterface(); +function isClassInstance(type: ts.Type): boolean { + return tsutils.typeParts(type).some(t => t.isClassOrInterface()); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 48a0cee1bd50..e71457a52659 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -1,4 +1,4 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; +import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import rule from '../../src/rules/no-misused-spread'; import { getFixturesRootDir } from '../RuleTester'; @@ -83,6 +83,18 @@ ruleTester.run('no-misused-spread', rule, { const a = [...getIterable()]; `, + ` + declare const data: Uint8Array; + + const a = [...data]; + `, + + ` + declare const data: TypedArray; + + const a = [...data]; + `, + 'const o = { ...{ a: 1, b: 2 } };', 'const o = { ...({ a: 1, b: 2 } as const) };', @@ -229,6 +241,44 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...a }; `, }, + + ` + class A { + [Symbol.iterator]() { + return { + next() { + return { done: true, value: undefined }; + }, + }; + } + } + + const a = [...new A()]; + `, + + { + options: [{ allowClassInstances: true }], + code: ` + class A { + [Symbol.iterator]() { + return { + next() { + return { done: true, value: undefined }; + }, + }; + } + } + + const a = [...new A()]; + `, + }, + + { + options: [{ allowClassInstances: true }], + code: noFormat` + const a = { ...new (class A { static value = 1; })() }; + `, + }, ], invalid: [ @@ -529,7 +579,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 1, column: 13, @@ -547,7 +597,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -565,7 +615,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -583,7 +633,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -601,7 +651,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -619,7 +669,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -641,7 +691,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 3, column: 11, @@ -664,7 +714,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 7, column: 21, @@ -682,7 +732,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 3, column: 21, @@ -700,7 +750,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 3, column: 21, @@ -718,7 +768,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 3, column: 21, @@ -736,7 +786,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Map', + type: 'Iterable', }, line: 3, column: 21, @@ -754,7 +804,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Set', + type: 'Iterable', }, line: 3, column: 21, @@ -765,7 +815,7 @@ ruleTester.run('no-misused-spread', rule, { { code: ` - declare const promise: Promise; + const promise = new Promise(() => {}); const o = { ...promise }; `, errors: [ @@ -1045,13 +1095,41 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + options: [{ allowClassInstances: true }], + code: ` + class A { + [Symbol.iterator]() { + return { + next() { + return { done: true, value: undefined }; + }, + }; + } + } + + const a = { ...new A() }; + `, + errors: [ + { + messageId: 'noSpreadInObject', + data: { + type: 'Iterable', + }, + line: 12, + column: 21, + endColumn: 31, + }, + ], + }, + { code: ` const o = { ...new Date() }; `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 2, column: 21, endColumn: 34, @@ -1073,7 +1151,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 10, column: 21, endColumn: 31, @@ -1093,7 +1171,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 25, @@ -1113,7 +1191,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 25, @@ -1133,7 +1211,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 30, @@ -1153,7 +1231,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 30, @@ -1173,7 +1251,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 31, @@ -1193,7 +1271,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 25, @@ -1213,12 +1291,60 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 8, column: 21, endColumn: 25, }, ], }, + + { + code: ` + const a = { + ...class A { + static value = 1; + nonStatic = 2; + }, + }; + `, + errors: [ + { + messageId: 'noClassDeclarationSpreadInObject', + line: 3, + column: 11, + endLine: 6, + endColumn: 12, + }, + ], + }, + + { + code: noFormat` + const a = { ...(class A { static value = 1 }) } + `, + errors: [ + { + messageId: 'noClassDeclarationSpreadInObject', + line: 2, + column: 21, + endColumn: 54, + }, + ], + }, + + { + code: noFormat` + const a = { ...new (class A { static value = 1; })() }; + `, + errors: [ + { + messageId: 'noClassInstanceSpreadInObject', + line: 2, + column: 21, + endColumn: 61, + }, + ], + }, ], }); From e02d7a939a73e9fb34ee0c73ab5f347ae28e64e9 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 24 Apr 2024 16:33:00 +0300 Subject: [PATCH 11/37] don't flag strings spread in object --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 2 +- .../eslint-plugin/tests/rules/no-misused-spread.test.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 6ff1aefb4409..0f04dc4910d4 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -124,7 +124,7 @@ export default createRule({ return; } - if (isIterable(type, checker)) { + if (isIterable(type, checker) && !isString(type)) { context.report({ node, messageId: 'noSpreadInObject', diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index e71457a52659..56f37221ce70 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -162,6 +162,13 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...promiseLike }; `, + // This case is being flagged by TS already, but since we check in the code + // for `Iterable`s, it catches string as well, so this test exists to ensure + // we don't flag it. + ` + const o = { ...'test' }; + `, + { options: [{ allowClassInstances: true }], code: ` From 0921b80c8064ecf7cfd407d37479eee48bad13ef Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 20:58:44 +0300 Subject: [PATCH 12/37] wip --- ...isused-spread.md => no-misused-spread.mdx} | 22 ++++--- .../no-misused-spread.shot | 65 +++++++++++++++++++ 2 files changed, 78 insertions(+), 9 deletions(-) rename packages/eslint-plugin/docs/rules/{no-misused-spread.md => no-misused-spread.mdx} (90%) create mode 100644 packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.md b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx similarity index 90% rename from packages/eslint-plugin/docs/rules/no-misused-spread.md rename to packages/eslint-plugin/docs/rules/no-misused-spread.mdx index 8d2946509c96..f68ecc6cd74b 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.md +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -2,6 +2,9 @@ description: 'Disallow using the spread operator when it might cause unexpected behavior.' --- +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + > 🛑 This file is source code, not the primary documentation location! 🛑 > > See **https://typescript-eslint.io/rules/no-misused-spread** for documentation. @@ -10,9 +13,8 @@ The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Re ## Examples - - -### ❌ Incorrect + + ```ts declare const userName: string; @@ -31,7 +33,8 @@ declare function getObj(): { a: 1; b: 2 }; const getObjSpread = { ...getObj }; ``` -### ✅ Correct + + ```ts declare const userName: string; @@ -50,7 +53,8 @@ declare function getObj(): { a: 1; b: 2 }; const getObjSpread = { ...getObj() }; ``` - + + ## Options @@ -58,9 +62,8 @@ const getObjSpread = { ...getObj() }; By default, this rule disallows using the spread operator on instances of classes: - - -#### ❌ Incorrect + + ```ts option='{ "allowClassInstances": false }' class User { @@ -76,7 +79,8 @@ const user = new User('John'); const userSpread = { ...user }; ``` - + + If you want to allow this behavior, you can configure the rule with `"allowClassInstances": true`: diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot new file mode 100644 index 000000000000..2256df7773d9 --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot @@ -0,0 +1,65 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 1`] = ` +"Incorrect + +declare const userName: string; +const chars = [...userName]; + ~~~~~~~~~~~ Using the spread operator on a string can cause unexpected behavior. Prefer \`String.split('')\` instead. + +declare const arr: number[]; +const arrSpread = { ...arr }; + ~~~~~~ Using the spread operator on \`Array\` in an object can cause unexpected behavior. + +declare const set: Set; +const setSpread = { ...set }; + ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. + +declare const map: Map; +const mapSpread = { ...map }; + ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. + +declare function getObj(): { a: 1; b: 2 }; +const getObjSpread = { ...getObj }; + ~~~~~~~~~ Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function? +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 2`] = ` +"Correct + +declare const userName: string; +const chars = userName.split(''); + +declare const arr: number[]; +const arrSpread = [...arr]; + +declare const set: Set; +const setSpread = [...set]; + +declare const map: Map; +const mapSpread = [...map]; + +declare function getObj(): { a: 1; b: 2 }; +const getObjSpread = { ...getObj() }; +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 3`] = ` +"Incorrect +Options: { "allowClassInstances": false } + +class User { + name: string; + + constructor(name: string) { + this.name = name; + } +} + +const user = new User('John'); + +const userSpread = { ...user }; + ~~~~~~~ Using the spread operator on class instances without \`[Symbol.iterator]\` can cause unexpected behavior. +" +`; From b8d7e47de52aa9bfcec97ef76869d6d6ed83b5c2 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 21:39:49 +0300 Subject: [PATCH 13/37] wip --- .../src/rules/no-misused-spread.ts | 48 +++++++------------ .../tests/rules/no-misused-spread.test.ts | 33 +++++++++---- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 0f04dc4910d4..f984bc0b8560 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -91,18 +91,6 @@ export default createRule({ function checkObjectSpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - if (isArray(type, checker)) { - context.report({ - node, - messageId: 'noSpreadInObject', - data: { - type: 'Array', - }, - }); - - return; - } - if (isPromise(services.program, type)) { context.report({ node, @@ -162,14 +150,6 @@ export default createRule({ }, }); -function isArray(type: ts.Type, checker: ts.TypeChecker): boolean { - if (type.isUnion() || type.isIntersection()) { - return type.types.some(t => isArray(t, checker)); - } - - return checker.isArrayType(type) || checker.isTupleType(type); -} - function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { return tsutils .typeParts(type) @@ -179,23 +159,31 @@ function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { } function isString(type: ts.Type): boolean { - return tsutils - .typeParts(type) - .some(t => !!isTypeFlagSet(t, ts.TypeFlags.StringLike)); + return isTypeRecurser(type, t => isTypeFlagSet(t, ts.TypeFlags.StringLike)); } function isFunctionWithoutProps(type: ts.Type): boolean { - return tsutils - .typeParts(type) - .some( - t => t.getCallSignatures().length > 0 && t.getProperties().length === 0, - ); + return isTypeRecurser( + type, + t => t.getCallSignatures().length > 0 && t.getProperties().length === 0, + ); } function isPromise(program: ts.Program, type: ts.Type): boolean { - return tsutils.typeParts(type).some(t => isPromiseLike(program, t)); + return isTypeRecurser(type, t => isPromiseLike(program, t)); } function isClassInstance(type: ts.Type): boolean { - return tsutils.typeParts(type).some(t => t.isClassOrInterface()); + return isTypeRecurser(type, t => t.isClassOrInterface()); +} + +function isTypeRecurser( + type: ts.Type, + predicate: (t: ts.Type) => boolean, +): boolean { + if (type.isUnion() || type.isIntersection()) { + return type.types.some(t => isTypeRecurser(t, predicate)); + } + + return predicate(type); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 56f37221ce70..956015b044d6 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -376,6 +376,21 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const test: number | (boolean | (string & { __brand: true })); + const a = [...test]; + `, + errors: [ + { + messageId: 'noStringSpreadInArray', + line: 3, + column: 20, + endColumn: 27, + }, + ], + }, + { code: ` declare function getString(): string; @@ -427,7 +442,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 1, column: 13, @@ -445,7 +460,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -463,7 +478,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -481,7 +496,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -499,7 +514,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -517,7 +532,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -535,7 +550,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -553,7 +568,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, @@ -571,7 +586,7 @@ ruleTester.run('no-misused-spread', rule, { { messageId: 'noSpreadInObject', data: { - type: 'Array', + type: 'Iterable', }, line: 3, column: 21, From 3f9d545981e7a4cd58c9cc6292a6870c84e969ee Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 22:17:21 +0300 Subject: [PATCH 14/37] configs --- packages/typescript-eslint/src/configs/all.ts | 1 + packages/typescript-eslint/src/configs/disable-type-checked.ts | 1 + .../typescript-eslint/src/configs/strict-type-checked-only.ts | 1 + packages/typescript-eslint/src/configs/strict-type-checked.ts | 1 + 4 files changed, 4 insertions(+) diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index efe0d16fceb8..6f85f9e8b245 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -85,6 +85,7 @@ export default ( '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index 8d2f29220aba..21f5ace556b3 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -27,6 +27,7 @@ export default ( '@typescript-eslint/no-implied-eval': 'off', '@typescript-eslint/no-meaningless-void-operator': 'off', '@typescript-eslint/no-misused-promises': 'off', + '@typescript-eslint/no-misused-spread': 'off', '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-throw-literal': 'off', diff --git a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts index f17b5280ca49..fe345b0b966f 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts @@ -30,6 +30,7 @@ export default ( '@typescript-eslint/no-implied-eval': 'error', '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-redundant-type-constituents': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', diff --git a/packages/typescript-eslint/src/configs/strict-type-checked.ts b/packages/typescript-eslint/src/configs/strict-type-checked.ts index ad62ee749e25..d7f83631ce2a 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked.ts @@ -46,6 +46,7 @@ export default ( '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-misused-spread': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', From b1336986a2f8b34c279e238cde04ebb673ecbe46 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Sun, 19 May 2024 22:20:27 +0300 Subject: [PATCH 15/37] snapshot --- .../tests/docs-eslint-output-snapshots/no-misused-spread.shot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot index 2256df7773d9..8b084bbdb8ad 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot @@ -9,7 +9,7 @@ const chars = [...userName]; declare const arr: number[]; const arrSpread = { ...arr }; - ~~~~~~ Using the spread operator on \`Array\` in an object can cause unexpected behavior. + ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. declare const set: Set; const setSpread = { ...set }; From bb3031f0726fb1c35da70f3ab002d7416c13b6bd Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 18:29:28 +0300 Subject: [PATCH 16/37] add options --- .../src/rules/no-misused-spread.ts | 50 +++++++-- .../tests/rules/no-misused-spread.test.ts | 105 +++--------------- 2 files changed, 61 insertions(+), 94 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index f984bc0b8560..6473a0c58764 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -13,7 +13,11 @@ import { type Options = [ { - allowClassInstances: boolean; + allowStrings?: boolean; + allowFunctions?: boolean; + allowIterables?: boolean; + allowClassInstances?: boolean; + allowClassDeclarations?: boolean; }, ]; @@ -45,7 +49,7 @@ export default createRule({ 'Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function?', noClassInstanceSpreadInObject: - 'Using the spread operator on class instances without `[Symbol.iterator]` can cause unexpected behavior.', + 'Using the spread operator on class instances will lose their class prototype.', noClassDeclarationSpreadInObject: 'Using the spread operator on class declarations can cause unexpected behavior. Did you forget to instantiate the class?', @@ -54,11 +58,31 @@ export default createRule({ { type: 'object', properties: { + allowStrings: { + description: + 'Whether to allow spreading strings in arrays. Defaults to false.', + type: 'boolean', + }, + allowFunctions: { + description: + 'Whether to allow spreading functions without properties in objects. Defaults to false.', + type: 'boolean', + }, + allowIterables: { + description: + 'Whether to allow spreading iterables in objects. Defaults to false.', + type: 'boolean', + }, allowClassInstances: { description: 'Whether to allow spreading class instances in objects.', type: 'boolean', }, + allowClassDeclarations: { + description: + 'Whether to allow spreading class declarations in objects.', + type: 'boolean', + }, }, additionalProperties: false, }, @@ -67,7 +91,11 @@ export default createRule({ defaultOptions: [ { + allowStrings: false, + allowFunctions: false, + allowIterables: false, allowClassInstances: false, + allowClassDeclarations: false, }, ], @@ -78,7 +106,7 @@ export default createRule({ function checkArraySpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - if (isString(type)) { + if (!options.allowStrings && isString(type)) { context.report({ node, messageId: 'noStringSpreadInArray', @@ -91,6 +119,7 @@ export default createRule({ function checkObjectSpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); + // TODO: Remove? if (isPromise(services.program, type)) { context.report({ node, @@ -103,7 +132,7 @@ export default createRule({ return; } - if (isFunctionWithoutProps(type)) { + if (!options.allowFunctions && isFunctionWithoutProps(type)) { context.report({ node, messageId: 'noFunctionSpreadInObject', @@ -112,7 +141,11 @@ export default createRule({ return; } - if (isIterable(type, checker) && !isString(type)) { + if ( + !options.allowIterables && + isIterable(type, checker) && + !isString(type) + ) { context.report({ node, messageId: 'noSpreadInObject', @@ -133,7 +166,10 @@ export default createRule({ return; } - if (node.argument.type === AST_NODE_TYPES.ClassExpression) { + if ( + !options.allowClassDeclarations && + node.argument.type === AST_NODE_TYPES.ClassExpression + ) { context.report({ node, messageId: 'noClassDeclarationSpreadInObject', @@ -181,7 +217,7 @@ function isTypeRecurser( type: ts.Type, predicate: (t: ts.Type) => boolean, ): boolean { - if (type.isUnion() || type.isIntersection()) { + if (type.isUnionOrIntersection()) { return type.types.some(t => isTypeRecurser(t, predicate)); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 956015b044d6..2d9ab6c84950 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -170,69 +170,31 @@ ruleTester.run('no-misused-spread', rule, { `, { - options: [{ allowClassInstances: true }], - code: ` - class A { - a = 1; - public b = 2; - private c = 3; - protected d = 4; - static e = 5; - } - - const o = { ...new A() }; - `, - }, - - { - options: [{ allowClassInstances: true }], - code: ` - class A { - a = 1; - } - - const a = new A(); - - const o = { ...a }; - `, - }, - - { - options: [{ allowClassInstances: true }], - code: ` - class A { - a = 1; - } - - declare const a: A; - - const o = { ...a }; + options: [{ allowStrings: true }], + code: noFormat` + const a = [ ...'test' ] `, }, { - options: [{ allowClassInstances: true }], - code: ` - class A { - a = 1; - } - - class B extends A {} + options: [{ allowFunctions: true }], + code: noFormat` + function f() {} - const o = { ...new B() }; + const a = { ...f }; `, }, { - options: [{ allowClassInstances: true }], - code: ` - class A { - a = 1; - } - - declare const a: A | { b: string }; + options: [{ allowIterables: true }], + code: noFormat` + const iterator = { + *[Symbol.iterator]() { + yield 'test'; + }, + }; - const o = { ...a }; + const a = { ...iterator }; `, }, @@ -243,47 +205,16 @@ ruleTester.run('no-misused-spread', rule, { a = 1; } - declare const a: A & { b: string }; + const a = new A(); const o = { ...a }; `, }, - ` - class A { - [Symbol.iterator]() { - return { - next() { - return { done: true, value: undefined }; - }, - }; - } - } - - const a = [...new A()]; - `, - - { - options: [{ allowClassInstances: true }], - code: ` - class A { - [Symbol.iterator]() { - return { - next() { - return { done: true, value: undefined }; - }, - }; - } - } - - const a = [...new A()]; - `, - }, - { - options: [{ allowClassInstances: true }], + options: [{ allowClassDeclarations: true }], code: noFormat` - const a = { ...new (class A { static value = 1; })() }; + const a = { ...(class A { static value = 1 }) } `, }, ], From 99c67a0193571dbc199e606fba61a8ac1fbbd56e Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 18:36:18 +0300 Subject: [PATCH 17/37] better message --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 6473a0c58764..ec516e644e10 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -52,7 +52,7 @@ export default createRule({ 'Using the spread operator on class instances will lose their class prototype.', noClassDeclarationSpreadInObject: - 'Using the spread operator on class declarations can cause unexpected behavior. Did you forget to instantiate the class?', + 'Using the spread operator on class declarations will spread only their static properties, and will lose their class prototype.', }, schema: [ { From ef12015e1ef578f092bdf6f49b7c8a359236d46a Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 18:47:14 +0300 Subject: [PATCH 18/37] add map --- .../src/rules/no-misused-spread.ts | 33 ++++++++++++- .../tests/rules/no-misused-spread.test.ts | 47 ++++++++----------- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index ec516e644e10..0bb1a17cc16c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -7,6 +7,7 @@ import { createRule, getConstrainedTypeAtLocation, getParserServices, + isBuiltinSymbolLike, isPromiseLike, isTypeFlagSet, } from '../util'; @@ -15,6 +16,7 @@ type Options = [ { allowStrings?: boolean; allowFunctions?: boolean; + allowMaps?: boolean; allowIterables?: boolean; allowClassInstances?: boolean; allowClassDeclarations?: boolean; @@ -25,6 +27,7 @@ type MessageIds = | 'noStringSpreadInArray' | 'noSpreadInObject' | 'noFunctionSpreadInObject' + | 'noMapSpreadInObject' | 'noClassInstanceSpreadInObject' | 'noClassDeclarationSpreadInObject'; @@ -48,6 +51,9 @@ export default createRule({ noFunctionSpreadInObject: 'Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function?', + noMapSpreadInObject: + 'Using the spread operator on a Map in an object will result in an emtpy object. Did you mean to use `Object.fromEntries(map)` instead?', + noClassInstanceSpreadInObject: 'Using the spread operator on class instances will lose their class prototype.', @@ -68,6 +74,11 @@ export default createRule({ 'Whether to allow spreading functions without properties in objects. Defaults to false.', type: 'boolean', }, + allowMaps: { + description: + 'Whether to allow spreading maps in objects. Defaults to false.', + type: 'boolean', + }, allowIterables: { description: 'Whether to allow spreading iterables in objects. Defaults to false.', @@ -93,6 +104,7 @@ export default createRule({ { allowStrings: false, allowFunctions: false, + allowMaps: false, allowIterables: false, allowClassInstances: false, allowClassDeclarations: false, @@ -141,10 +153,20 @@ export default createRule({ return; } + if (!options.allowMaps && isMap(services.program, type)) { + context.report({ + node, + messageId: 'noMapSpreadInObject', + }); + + return; + } + if ( !options.allowIterables && isIterable(type, checker) && - !isString(type) + !isString(type) && + !isMap(services.program, type) ) { context.report({ node, @@ -213,6 +235,15 @@ function isClassInstance(type: ts.Type): boolean { return isTypeRecurser(type, t => t.isClassOrInterface()); } +function isMap(program: ts.Program, type: ts.Type): boolean { + return isTypeRecurser( + type, + t => + isBuiltinSymbolLike(program, t, 'Map') || + isBuiltinSymbolLike(program, t, 'ReadonlyMap'), + ); +} + function isTypeRecurser( type: ts.Type, predicate: (t: ts.Type) => boolean, diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 2d9ab6c84950..b176f05a9f86 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -185,6 +185,18 @@ ruleTester.run('no-misused-spread', rule, { `, }, + { + options: [{ allowMaps: true }], + code: noFormat` + const o = { + ...new Map([ + ['test-1', 1], + ['test-2', 2], + ]), + }; + `, + }, + { options: [{ allowIterables: true }], code: noFormat` @@ -642,10 +654,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 11, endLine: 6, @@ -665,10 +674,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 7, column: 21, endColumn: 27, @@ -683,10 +689,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -701,10 +704,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -719,10 +719,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -737,10 +734,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 21, endColumn: 32, @@ -755,10 +749,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noMapSpreadInObject', line: 3, column: 21, endColumn: 25, From c3fa340483b2a2ae1755b3a99aa58dd89c66e735 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 18:52:15 +0300 Subject: [PATCH 19/37] messages --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 0bb1a17cc16c..6a79aec62f0b 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -86,12 +86,12 @@ export default createRule({ }, allowClassInstances: { description: - 'Whether to allow spreading class instances in objects.', + 'Whether to allow spreading class instances in objects. Defaults to false.', type: 'boolean', }, allowClassDeclarations: { description: - 'Whether to allow spreading class declarations in objects.', + 'Whether to allow spreading class declarations in objects. Defaults to false.', type: 'boolean', }, }, From bc3bcf283029d02a30760c6d0524f9bcf3fd2b51 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 21:40:35 +0300 Subject: [PATCH 20/37] array --- .../src/rules/no-misused-spread.ts | 20 ++++++ .../tests/rules/no-misused-spread.test.ts | 61 ++++++++----------- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 6a79aec62f0b..23a11f653f6f 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -28,6 +28,7 @@ type MessageIds = | 'noSpreadInObject' | 'noFunctionSpreadInObject' | 'noMapSpreadInObject' + | 'noArraySpreadInObject' | 'noClassInstanceSpreadInObject' | 'noClassDeclarationSpreadInObject'; @@ -54,6 +55,9 @@ export default createRule({ noMapSpreadInObject: 'Using the spread operator on a Map in an object will result in an emtpy object. Did you mean to use `Object.fromEntries(map)` instead?', + noArraySpreadInObject: + 'Using the spread operator on an array in an object will result in a list of indices.', + noClassInstanceSpreadInObject: 'Using the spread operator on class instances will lose their class prototype.', @@ -162,6 +166,15 @@ export default createRule({ return; } + if (isArray(checker, type)) { + context.report({ + node, + messageId: 'noArraySpreadInObject', + }); + + return; + } + if ( !options.allowIterables && isIterable(type, checker) && @@ -216,6 +229,13 @@ function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { ); } +function isArray(checker: ts.TypeChecker, type: ts.Type): boolean { + return isTypeRecurser( + type, + t => checker.isArrayType(t) || checker.isTupleType(t), + ); +} + function isString(type: ts.Type): boolean { return isTypeRecurser(type, t => isTypeFlagSet(t, ts.TypeFlags.StringLike)); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index b176f05a9f86..479f8e02154e 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -383,10 +383,7 @@ ruleTester.run('no-misused-spread', rule, { code: 'const o = { ...[1, 2, 3] };', errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 1, column: 13, endColumn: 25, @@ -401,10 +398,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -419,10 +413,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -437,10 +428,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -455,10 +443,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -473,10 +458,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -491,10 +473,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 27, @@ -509,10 +488,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 34, @@ -527,10 +503,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noArraySpreadInObject', line: 3, column: 21, endColumn: 34, @@ -538,6 +511,22 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + const arr = [1, 2, 3]; + const o = { ...arr }; + `, + options: [{ allowIterables: true }], + errors: [ + { + messageId: 'noArraySpreadInObject', + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: 'const o = { ...new Set([1, 2, 3]) };', errors: [ From 137c371c19c902e5180c1e55eb44ba21e05309cb Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 21:49:38 +0300 Subject: [PATCH 21/37] promise + iterable --- .../src/rules/no-misused-spread.ts | 30 +++--- .../tests/rules/no-misused-spread.test.ts | 98 ++++++------------- 2 files changed, 46 insertions(+), 82 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 23a11f653f6f..8afcb34f6e57 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -15,6 +15,7 @@ import { type Options = [ { allowStrings?: boolean; + allowPromises?: boolean; allowFunctions?: boolean; allowMaps?: boolean; allowIterables?: boolean; @@ -25,7 +26,8 @@ type Options = [ type MessageIds = | 'noStringSpreadInArray' - | 'noSpreadInObject' + | 'noPromiseSpreadInObject' + | 'noIterableSpreadInObject' | 'noFunctionSpreadInObject' | 'noMapSpreadInObject' | 'noArraySpreadInObject' @@ -46,8 +48,11 @@ export default createRule({ noStringSpreadInArray: "Using the spread operator on a string can cause unexpected behavior. Prefer `String.split('')` instead.", - noSpreadInObject: - 'Using the spread operator on `{{type}}` in an object can cause unexpected behavior.', + noPromiseSpreadInObject: + 'Using the spread operator on Promise in an object can cause unexpected behavior. Did you forget to await the promise?', + + noIterableSpreadInObject: + 'Using the spread operator on an Iterable in an object can cause unexpected behavior.', noFunctionSpreadInObject: 'Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function?', @@ -73,6 +78,11 @@ export default createRule({ 'Whether to allow spreading strings in arrays. Defaults to false.', type: 'boolean', }, + allowPromises: { + description: + 'Whether to allow spreading promises in objects. Defaults to false.', + type: 'boolean', + }, allowFunctions: { description: 'Whether to allow spreading functions without properties in objects. Defaults to false.', @@ -107,6 +117,7 @@ export default createRule({ defaultOptions: [ { allowStrings: false, + allowPromises: false, allowFunctions: false, allowMaps: false, allowIterables: false, @@ -135,14 +146,10 @@ export default createRule({ function checkObjectSpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - // TODO: Remove? - if (isPromise(services.program, type)) { + if (!options.allowPromises && isPromise(services.program, type)) { context.report({ node, - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', }); return; @@ -183,10 +190,7 @@ export default createRule({ ) { context.report({ node, - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', }); return; diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 479f8e02154e..f6f3f4606a96 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -171,14 +171,22 @@ ruleTester.run('no-misused-spread', rule, { { options: [{ allowStrings: true }], - code: noFormat` - const a = [ ...'test' ] + code: ` + const a = [...'test']; + `, + }, + + { + options: [{ allowPromises: true }], + code: ` + const promise = new Promise(() => {}); + const o = { ...promise }; `, }, { options: [{ allowFunctions: true }], - code: noFormat` + code: ` function f() {} const a = { ...f }; @@ -187,7 +195,7 @@ ruleTester.run('no-misused-spread', rule, { { options: [{ allowMaps: true }], - code: noFormat` + code: ` const o = { ...new Map([ ['test-1', 1], @@ -199,7 +207,7 @@ ruleTester.run('no-misused-spread', rule, { { options: [{ allowIterables: true }], - code: noFormat` + code: ` const iterator = { *[Symbol.iterator]() { yield 'test'; @@ -531,10 +539,7 @@ ruleTester.run('no-misused-spread', rule, { code: 'const o = { ...new Set([1, 2, 3]) };', errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 1, column: 13, endColumn: 34, @@ -549,10 +554,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -567,10 +569,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -585,10 +584,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -603,10 +599,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 3, column: 21, endColumn: 27, @@ -621,10 +614,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 3, column: 21, endColumn: 32, @@ -753,10 +743,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', line: 3, column: 21, endColumn: 31, @@ -771,10 +758,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', line: 3, column: 21, endColumn: 36, @@ -789,10 +773,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', line: 3, column: 21, endColumn: 31, @@ -807,10 +788,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', line: 3, column: 21, endColumn: 36, @@ -825,10 +803,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Promise', - }, + messageId: 'noPromiseSpreadInObject', line: 3, column: 21, endColumn: 36, @@ -960,10 +935,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 8, column: 21, endColumn: 32, @@ -979,10 +951,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 4, column: 21, endColumn: 32, @@ -998,10 +967,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 4, column: 21, endColumn: 32, @@ -1017,10 +983,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 4, column: 21, endColumn: 37, @@ -1045,10 +1008,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noSpreadInObject', - data: { - type: 'Iterable', - }, + messageId: 'noIterableSpreadInObject', line: 12, column: 21, endColumn: 31, From db727269a8f9ec27d7cda21fb773f5d9952aab97 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 22:15:35 +0300 Subject: [PATCH 22/37] class declaration --- .../src/rules/no-misused-spread.ts | 21 ++++++- .../tests/rules/no-misused-spread.test.ts | 56 ++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 8afcb34f6e57..4e067246e308 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -207,7 +207,8 @@ export default createRule({ if ( !options.allowClassDeclarations && - node.argument.type === AST_NODE_TYPES.ClassExpression + isClassDeclaration(type) && + !isClassInstance(type) ) { context.report({ node, @@ -259,6 +260,24 @@ function isClassInstance(type: ts.Type): boolean { return isTypeRecurser(type, t => t.isClassOrInterface()); } +function isClassDeclaration(type: ts.Type): boolean { + return isTypeRecurser(type, t => { + if ( + tsutils.isObjectType(t) && + tsutils.isObjectFlagSet(t, ts.ObjectFlags.InstantiationExpressionType) + ) { + return true; + } + + const kind = t.getSymbol()?.valueDeclaration?.kind; + + return ( + kind === ts.SyntaxKind.ClassDeclaration || + kind === ts.SyntaxKind.ClassExpression + ); + }); +} + function isMap(program: ts.Program, type: ts.Type): boolean { return isTypeRecurser( type, diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index f6f3f4606a96..a8f722f4ddc7 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -233,8 +233,12 @@ ruleTester.run('no-misused-spread', rule, { { options: [{ allowClassDeclarations: true }], - code: noFormat` - const a = { ...(class A { static value = 1 }) } + code: ` + const a = { + ...class A { + static value = 1; + }, + }; `, }, ], @@ -1192,6 +1196,54 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + class A {} + + const o = { ...A }; + `, + errors: [ + { + messageId: 'noClassDeclarationSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + const A = class {}; + + const o = { ...A }; + `, + errors: [ + { + messageId: 'noClassDeclarationSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + + { + code: ` + const A = Set; + + const o = { ...A }; + `, + errors: [ + { + messageId: 'noClassDeclarationSpreadInObject', + line: 4, + column: 21, + endColumn: 25, + }, + ], + }, + { code: ` const a = { From 92140f22f62c910c492e0e63aa3bd48c345bbd01 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 22:37:49 +0300 Subject: [PATCH 23/37] class instance --- packages/eslint-plugin/src/rules/no-misused-spread.ts | 8 ++++++-- .../eslint-plugin/tests/rules/no-misused-spread.test.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 4e067246e308..911c47a17a36 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -1,5 +1,4 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; @@ -257,7 +256,12 @@ function isPromise(program: ts.Program, type: ts.Type): boolean { } function isClassInstance(type: ts.Type): boolean { - return isTypeRecurser(type, t => t.isClassOrInterface()); + return isTypeRecurser(type, t => { + return ( + t.isClassOrInterface() && + tsutils.isSymbolFlagSet(t.symbol, ts.SymbolFlags.Value) + ); + }); } function isClassDeclaration(type: ts.Type): boolean { diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index a8f722f4ddc7..5ebaa17fad31 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -162,6 +162,14 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...promiseLike }; `, + ` + interface A {} + + declare const a: A; + + const o = { ...a }; + `, + // This case is being flagged by TS already, but since we check in the code // for `Iterable`s, it catches string as well, so this test exists to ensure // we don't flag it. From 8a5e2dceb208b848b80f17ad275e51d8cde31d76 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Tue, 28 May 2024 22:38:03 +0300 Subject: [PATCH 24/37] lint --- packages/eslint-plugin/src/rules/lines-between-class-members.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/lines-between-class-members.ts b/packages/eslint-plugin/src/rules/lines-between-class-members.ts index 60da9308757a..9004c313bb7e 100644 --- a/packages/eslint-plugin/src/rules/lines-between-class-members.ts +++ b/packages/eslint-plugin/src/rules/lines-between-class-members.ts @@ -16,6 +16,7 @@ type MessageIds = InferMessageIdsTypeFromRule; const schema = Object.values( deepMerge( + // eslint-disable-next-line @typescript-eslint/no-misused-spread { ...baseRule.meta.schema }, { 1: { From d2fe8741450038c17e65cee7be65d1b25353c181 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 29 May 2024 16:12:40 +0300 Subject: [PATCH 25/37] wip --- .../src/rules/no-misused-spread.ts | 18 ++--------------- .../tests/rules/no-misused-spread.test.ts | 20 ------------------- 2 files changed, 2 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 911c47a17a36..d0b1b09dca63 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -14,9 +14,7 @@ import { type Options = [ { allowStrings?: boolean; - allowPromises?: boolean; allowFunctions?: boolean; - allowMaps?: boolean; allowIterables?: boolean; allowClassInstances?: boolean; allowClassDeclarations?: boolean; @@ -77,21 +75,11 @@ export default createRule({ 'Whether to allow spreading strings in arrays. Defaults to false.', type: 'boolean', }, - allowPromises: { - description: - 'Whether to allow spreading promises in objects. Defaults to false.', - type: 'boolean', - }, allowFunctions: { description: 'Whether to allow spreading functions without properties in objects. Defaults to false.', type: 'boolean', }, - allowMaps: { - description: - 'Whether to allow spreading maps in objects. Defaults to false.', - type: 'boolean', - }, allowIterables: { description: 'Whether to allow spreading iterables in objects. Defaults to false.', @@ -116,9 +104,7 @@ export default createRule({ defaultOptions: [ { allowStrings: false, - allowPromises: false, allowFunctions: false, - allowMaps: false, allowIterables: false, allowClassInstances: false, allowClassDeclarations: false, @@ -145,7 +131,7 @@ export default createRule({ function checkObjectSpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - if (!options.allowPromises && isPromise(services.program, type)) { + if (isPromise(services.program, type)) { context.report({ node, messageId: 'noPromiseSpreadInObject', @@ -163,7 +149,7 @@ export default createRule({ return; } - if (!options.allowMaps && isMap(services.program, type)) { + if (isMap(services.program, type)) { context.report({ node, messageId: 'noMapSpreadInObject', diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 5ebaa17fad31..ba847e08b880 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -184,14 +184,6 @@ ruleTester.run('no-misused-spread', rule, { `, }, - { - options: [{ allowPromises: true }], - code: ` - const promise = new Promise(() => {}); - const o = { ...promise }; - `, - }, - { options: [{ allowFunctions: true }], code: ` @@ -201,18 +193,6 @@ ruleTester.run('no-misused-spread', rule, { `, }, - { - options: [{ allowMaps: true }], - code: ` - const o = { - ...new Map([ - ['test-1', 1], - ['test-2', 2], - ]), - }; - `, - }, - { options: [{ allowIterables: true }], code: ` From 0b0ac03d004634fedd108731d94c4f0eed826604 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 29 May 2024 16:25:02 +0300 Subject: [PATCH 26/37] docs --- .../docs/rules/no-misused-spread.mdx | 127 +++++++++++++++++- .../no-misused-spread.shot | 58 +++++++- .../schema-snapshots/no-misused-spread.shot | 28 +++- 3 files changed, 201 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx index f68ecc6cd74b..336500bd052a 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -47,7 +47,7 @@ declare const set: Set; const setSpread = [...set]; declare const map: Map; -const mapSpread = [...map]; +const mapObject = Object.fromEntries(map); declare function getObj(): { a: 1; b: 2 }; const getObjSpread = { ...getObj() }; @@ -58,6 +58,92 @@ const getObjSpread = { ...getObj() }; ## Options +### `allowStrings` + +By default, this rule disallows using the spread operator on strings inside arrays: + + + + +```ts +const fruit = 'Apple'; +const chars = [...fruit]; +``` + + + + +If you want to allow this behavior, you can configure the rule with `"allowStrings": true`: + +```json +{ + "@typescript-eslint/no-misused-spread": [ + "error", + { + "allowStrings": true + } + ] +} +``` + +### `allowFunctions` + +By default, this rule disallows using the spread operator on functions without additional properties: + + + + +```ts +declare function doWork(); + +const spread = { ...doWork }; +``` + + + + +If you want to allow this behavior, you can configure the rule with `"allowFunctions": true`: + +```json +{ + "@typescript-eslint/no-misused-spread": [ + "error", + { + "allowFunctions": true + } + ] +} +``` + +### `allowIterables` + +By default, this rule disallows using the spread operator on iterables: + + + + +```ts +const set = new Set([1, 2, 3]); + +const spread = { ...set }; +``` + + + + +If you want to allow this behavior, you can configure the rule with `"allowIterables": true`: + +```json +{ + "@typescript-eslint/no-misused-spread": [ + "error", + { + "allowIterables": true + } + ] +} +``` + ### `allowClassInstances` By default, this rule disallows using the spread operator on instances of classes: @@ -65,7 +151,7 @@ By default, this rule disallows using the spread operator on instances of classe -```ts option='{ "allowClassInstances": false }' +```ts class User { name: string; @@ -76,7 +162,7 @@ class User { const user = new User('John'); -const userSpread = { ...user }; +const spread = { ...user }; ``` @@ -95,6 +181,41 @@ If you want to allow this behavior, you can configure the rule with `"allowClass } ``` +### `allowClassDeclarations` + +By default, this rule disallows using the spread operator on class declarations: + + + + +```ts +class User { + name: string; + + constructor(name: string) { + this.name = name; + } +} + +const spread = { ...User }; +``` + + + + +If you want to allow this behavior, you can configure the rule with `"allowClassDeclarations": true`: + +```json +{ + "@typescript-eslint/no-misused-spread": [ + "error", + { + "allowClassDeclarations": true + } + ] +} +``` + ## When Not To Use It If you intentionally want to use the spread operator in those cases, and expect diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot index 8b084bbdb8ad..b59e4720d81a 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot @@ -9,15 +9,15 @@ const chars = [...userName]; declare const arr: number[]; const arrSpread = { ...arr }; - ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. + ~~~~~~ Using the spread operator on an array in an object will result in a list of indices. declare const set: Set; const setSpread = { ...set }; - ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. + ~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. declare const map: Map; const mapSpread = { ...map }; - ~~~~~~ Using the spread operator on \`Iterable\` in an object can cause unexpected behavior. + ~~~~~~ Using the spread operator on a Map in an object will result in an emtpy object. Did you mean to use \`Object.fromEntries(map)\` instead? declare function getObj(): { a: 1; b: 2 }; const getObjSpread = { ...getObj }; @@ -38,7 +38,7 @@ declare const set: Set; const setSpread = [...set]; declare const map: Map; -const mapSpread = [...map]; +const mapObject = Object.fromEntries(map); declare function getObj(): { a: 1; b: 2 }; const getObjSpread = { ...getObj() }; @@ -47,7 +47,35 @@ const getObjSpread = { ...getObj() }; exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 3`] = ` "Incorrect -Options: { "allowClassInstances": false } + +const fruit = 'Apple'; +const chars = [...fruit]; + ~~~~~~~~ Using the spread operator on a string can cause unexpected behavior. Prefer \`String.split('')\` instead. +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 4`] = ` +"Incorrect + +declare function doWork(); + +const spread = { ...doWork }; + ~~~~~~~~~ Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function? +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 5`] = ` +"Incorrect + +const set = new Set([1, 2, 3]); + +const spread = { ...set }; + ~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 6`] = ` +"Incorrect class User { name: string; @@ -59,7 +87,23 @@ class User { const user = new User('John'); -const userSpread = { ...user }; - ~~~~~~~ Using the spread operator on class instances without \`[Symbol.iterator]\` can cause unexpected behavior. +const spread = { ...user }; + ~~~~~~~ Using the spread operator on class instances will lose their class prototype. +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 7`] = ` +"Incorrect + +class User { + name: string; + + constructor(name: string) { + this.name = name; + } +} + +const spread = { ...User }; + ~~~~~~~ Using the spread operator on class declarations will spread only their static properties, and will lose their class prototype. " `; diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot index fc9ba84d9b1f..93dae0523474 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot @@ -8,8 +8,24 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos { "additionalProperties": false, "properties": { + "allowClassDeclarations": { + "description": "Whether to allow spreading class declarations in objects. Defaults to false.", + "type": "boolean" + }, "allowClassInstances": { - "description": "Whether to allow spreading class instances in objects.", + "description": "Whether to allow spreading class instances in objects. Defaults to false.", + "type": "boolean" + }, + "allowFunctions": { + "description": "Whether to allow spreading functions without properties in objects. Defaults to false.", + "type": "boolean" + }, + "allowIterables": { + "description": "Whether to allow spreading iterables in objects. Defaults to false.", + "type": "boolean" + }, + "allowStrings": { + "description": "Whether to allow spreading strings in arrays. Defaults to false.", "type": "boolean" } }, @@ -22,8 +38,16 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos type Options = [ { - /** Whether to allow spreading class instances in objects. */ + /** Whether to allow spreading class declarations in objects. Defaults to false. */ + allowClassDeclarations?: boolean; + /** Whether to allow spreading class instances in objects. Defaults to false. */ allowClassInstances?: boolean; + /** Whether to allow spreading functions without properties in objects. Defaults to false. */ + allowFunctions?: boolean; + /** Whether to allow spreading iterables in objects. Defaults to false. */ + allowIterables?: boolean; + /** Whether to allow spreading strings in arrays. Defaults to false. */ + allowStrings?: boolean; }, ]; " From 02d7fd38f7dddfe9b1e654ad6e577e1b624fb2c1 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:02:28 +0300 Subject: [PATCH 27/37] wip --- .../src/rules/no-misused-spread.ts | 46 ++-- .../tests/rules/no-misused-spread.test.ts | 211 +++++++++++++++--- 2 files changed, 215 insertions(+), 42 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index d0b1b09dca63..e32d101d3809 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -9,15 +9,18 @@ import { isBuiltinSymbolLike, isPromiseLike, isTypeFlagSet, + readonlynessOptionsSchema, + typeMatchesSpecifier, + TypeOrValueSpecifier, } from '../util'; type Options = [ { allowStrings?: boolean; allowFunctions?: boolean; - allowIterables?: boolean; allowClassInstances?: boolean; allowClassDeclarations?: boolean; + allowForKnownSafeIterables?: TypeOrValueSpecifier[]; }, ]; @@ -70,6 +73,11 @@ export default createRule({ { type: 'object', properties: { + allowForKnownSafeIterables: { + ...readonlynessOptionsSchema.properties.allow, + description: + 'An array of iterables type specifiers that are known to be safe to spread in objects.', + }, allowStrings: { description: 'Whether to allow spreading strings in arrays. Defaults to false.', @@ -105,9 +113,9 @@ export default createRule({ { allowStrings: false, allowFunctions: false, - allowIterables: false, allowClassInstances: false, allowClassDeclarations: false, + allowForKnownSafeIterables: [], }, ], @@ -167,20 +175,6 @@ export default createRule({ return; } - if ( - !options.allowIterables && - isIterable(type, checker) && - !isString(type) && - !isMap(services.program, type) - ) { - context.report({ - node, - messageId: 'noIterableSpreadInObject', - }); - - return; - } - if (!options.allowClassInstances && isClassInstance(type)) { context.report({ node, @@ -202,6 +196,23 @@ export default createRule({ return; } + + const isTypeAllowed = () => { + const allowedTypes = options.allowForKnownSafeIterables ?? []; + + return allowedTypes.some(specifier => + typeMatchesSpecifier(type, specifier, services.program), + ); + }; + + if (isIterable(type, checker) && !isString(type) && !isTypeAllowed()) { + context.report({ + node, + messageId: 'noIterableSpreadInObject', + }); + + return; + } } return { @@ -273,7 +284,8 @@ function isMap(program: ts.Program, type: ts.Type): boolean { type, t => isBuiltinSymbolLike(program, t, 'Map') || - isBuiltinSymbolLike(program, t, 'ReadonlyMap'), + isBuiltinSymbolLike(program, t, 'ReadonlyMap') || + isBuiltinSymbolLike(program, t, 'WeakMap'), ); } diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index ba847e08b880..0f268aa0662c 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -6,10 +6,11 @@ import { getFixturesRootDir } from '../RuleTester'; const rootPath = getFixturesRootDir(); const ruleTester = new RuleTester({ - parser: '@typescript-eslint/parser', - parserOptions: { - tsconfigRootDir: rootPath, - project: './tsconfig.json', + languageOptions: { + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, }, }); @@ -194,14 +195,69 @@ ruleTester.run('no-misused-spread', rule, { }, { - options: [{ allowIterables: true }], + options: [ + { + allowForKnownSafeIterables: [{ from: 'lib', name: 'Iterable' }], + }, + ], code: ` - const iterator = { - *[Symbol.iterator]() { - yield 'test'; - }, + declare const iterator: Iterable; + + const a = { ...iterator }; + `, + }, + + { + options: [{ allowForKnownSafeIterables: ['CustomIterable'] }], + code: ` + type CustomIterable = { + [Symbol.iterator]: () => Generator; }; + declare const iterator: CustomIterable; + + const a = { ...iterator }; + `, + }, + + { + options: [ + { + allowForKnownSafeIterables: [ + { from: 'file', name: 'CustomIterable' }, + ], + }, + ], + code: ` + type CustomIterable = { + [Symbol.iterator]: () => string; + }; + + declare const iterator: CustomIterable; + + const a = { ...iterator }; + `, + }, + + { + options: [ + { + allowForKnownSafeIterables: [ + { from: 'package', package: 'module', name: 'CustomIterable' }, + ], + }, + ], + code: ` + declare module 'module' { + export type CustomIterable = { + [Symbol.iterator]: () => string; + }; + } + + import { CustomIterable } from 'module'; + + declare const iterator: CustomIterable; + const a = { ...iterator }; `, }, @@ -229,6 +285,12 @@ ruleTester.run('no-misused-spread', rule, { }; `, }, + + // WeakSet is not iterable + ` + declare const set: WeakSet; + const o = { ...set }; + `, ], invalid: [ @@ -511,22 +573,6 @@ ruleTester.run('no-misused-spread', rule, { ], }, - { - code: ` - const arr = [1, 2, 3]; - const o = { ...arr }; - `, - options: [{ allowIterables: true }], - errors: [ - { - messageId: 'noArraySpreadInObject', - line: 3, - column: 21, - endColumn: 27, - }, - ], - }, - { code: 'const o = { ...new Set([1, 2, 3]) };', errors: [ @@ -683,6 +729,21 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const map: WeakMap<{ a: number }, string>; + const o = { ...map }; + `, + errors: [ + { + messageId: 'noMapSpreadInObject', + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` declare const map: Map | { a: number }; @@ -728,6 +789,21 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const ref = new WeakRef<{ a: number }>; + const o = { ...ref }; + `, + errors: [ + { + messageId: 'noClassInstanceSpreadInObject', + line: 3, + column: 21, + endColumn: 27, + }, + ], + }, + { code: ` const promise = new Promise(() => {}); @@ -935,6 +1011,61 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + type CustomIterable = { + [Symbol.iterator]: () => Generator; + }; + + const iterator: CustomIterable = { + *[Symbol.iterator]() { + yield 'test'; + }, + }; + + const a = { ...iterator }; + `, + options: [{ allowForKnownSafeIterables: ['AnotherIterable'] }], + errors: [ + { + messageId: 'noIterableSpreadInObject', + line: 12, + column: 21, + endColumn: 32, + }, + ], + }, + { + code: ` + declare module 'module' { + export type CustomIterable = { + [Symbol.iterator]: () => string; + }; + } + + import { CustomIterable } from 'module'; + + declare const iterator: CustomIterable; + + const a = { ...iterator }; + `, + options: [ + { + allowForKnownSafeIterables: [ + { from: 'package', package: 'module', name: 'Nothing' }, + ], + }, + ], + errors: [ + { + messageId: 'noIterableSpreadInObject', + line: 12, + column: 21, + endColumn: 32, + }, + ], + }, + { code: ` declare const iterator: Iterable; @@ -1022,6 +1153,36 @@ ruleTester.run('no-misused-spread', rule, { ], }, + { + code: ` + declare const element: HTMLElement; + const o = { ...element }; + `, + errors: [ + { + messageId: 'noClassInstanceSpreadInObject', + line: 2, + column: 21, + endColumn: 34, + }, + ], + }, + + { + code: ` + declare const regex: RegExp; + const o = { ...regex }; + `, + errors: [ + { + messageId: 'noClassInstanceSpreadInObject', + line: 2, + column: 21, + endColumn: 34, + }, + ], + }, + { code: ` class A { From e90385a13732bd1c56f70b3cd0a8c120bd7dc2e4 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:51:45 +0300 Subject: [PATCH 28/37] wip --- .../docs/rules/no-misused-spread.mdx | 175 +++++++++++------- .../src/rules/no-misused-spread.ts | 19 +- .../no-misused-spread.shot | 41 +++- .../tests/rules/no-misused-spread.test.ts | 2 +- .../schema-snapshots/no-misused-spread.shot | 113 +++++++++++ 5 files changed, 259 insertions(+), 91 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx index 336500bd052a..9b1fa4100dc0 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -60,7 +60,8 @@ const getObjSpread = { ...getObj() }; ### `allowStrings` -By default, this rule disallows using the spread operator on strings inside arrays: +By default, this rule disallows using the spread operator on strings inside arrays. +Set `"allowStrings": true` to allow this behavior. @@ -71,24 +72,23 @@ const chars = [...fruit]; ``` - -If you want to allow this behavior, you can configure the rule with `"allowStrings": true`: + -```json -{ - "@typescript-eslint/no-misused-spread": [ - "error", - { - "allowStrings": true - } - ] -} +```ts option='{"allowStrings": true}' +// { "allowStrings": true } +const fruit = 'Apple'; +const chars = [...fruit]; +``` + + + ``` ### `allowFunctions` -By default, this rule disallows using the spread operator on functions without additional properties: +By default, this rule disallows using the spread operator on functions without additional properties. +Set `"allowFunctions": true` to allow this behavior. @@ -100,53 +100,23 @@ const spread = { ...doWork }; ``` - -If you want to allow this behavior, you can configure the rule with `"allowFunctions": true`: - -```json -{ - "@typescript-eslint/no-misused-spread": [ - "error", - { - "allowFunctions": true - } - ] -} -``` - -### `allowIterables` - -By default, this rule disallows using the spread operator on iterables: - - - + -```ts -const set = new Set([1, 2, 3]); +```ts option='{"allowFunctions": true}' +// { "allowFunctions": true } +declare function doWork(); -const spread = { ...set }; +const spread = { ...doWork }; ``` -If you want to allow this behavior, you can configure the rule with `"allowIterables": true`: - -```json -{ - "@typescript-eslint/no-misused-spread": [ - "error", - { - "allowIterables": true - } - ] -} -``` - ### `allowClassInstances` -By default, this rule disallows using the spread operator on instances of classes: +By default, this rule disallows using the spread operator on instances of classes. +Set `"allowClassInstances": true` to allow this behavior. @@ -166,24 +136,31 @@ const spread = { ...user }; ``` - -If you want to allow this behavior, you can configure the rule with `"allowClassInstances": true`: + -```json -{ - "@typescript-eslint/no-misused-spread": [ - "error", - { - "allowClassInstances": true - } - ] +```ts option='{"allowClassInstances": true}' +// { "allowClassInstances": true } +class User { + name: string; + + constructor(name: string) { + this.name = name; + } } + +const user = new User('John'); + +const spread = { ...user }; ``` + + + ### `allowClassDeclarations` -By default, this rule disallows using the spread operator on class declarations: +By default, this rule disallows using the spread operator on class declarations. +Set `"allowClassDeclarations": true` to allow this behavior. @@ -200,22 +177,80 @@ class User { const spread = { ...User }; ``` + + + + +```ts option='{"allowClassDeclarations": true}' +// { "allowClassDeclarations": true } +class User { + name: string; + + constructor(name: string) { + this.name = name; + } +} + +const spread = { ...User }; +``` + -If you want to allow this behavior, you can configure the rule with `"allowClassDeclarations": true`: +### `allowForKnownSafeIterables` + +By default, this rule disallows using the spread operator on iterables. This +option allows marking specific iterable types as "safe" to be spread. + +This option takes an array of type specifiers to consider safe. +Each item in the array must have one of the following forms: + +- A type defined in the current file, or globally available (`"SafeIterable"`) +- A type defined in a file (`{ from: "file", name: "SafeIterable", path: "src/safe-iterable.ts" }` with `path` being an optional path relative to the project root directory) +- A type from the default library (`{ from: "lib", name: "CustomIterable" }`) +- A type from a package (`{ from: "package", name: "AwesomeIterable", package: "iterable-lib" }`, this also works for types defined in a typings package). + +Examples of a configuration for this option: ```json -{ - "@typescript-eslint/no-misused-spread": [ - "error", - { - "allowClassDeclarations": true - } - ] -} +"@typescript-eslint/no-misused-spread": [ + "error", + { + "allowForKnownSafeIterables": [ + "SafeIterable", + { "from": "file", "name": "ProbablySafeIterable" }, + { "from": "lib", "name": "CustomIterable" }, + { "from": "package", "name": "AwesomeIterable", "package": "iterable-lib" } + ] + } +] +``` + + + + +```ts +type UnsafeIterable = Iterable; + +declare const iterable: UnsafeIterable; + +const spread = { ...iterable }; ``` + + + +```ts option='{"allowForKnownSafeIterables":["SafeIterable"]}' +type SafeIterable = Iterable; + +declare const iterable: SafeIterable; + +const spread = { ...iterable }; +``` + + + + ## When Not To Use It If you intentionally want to use the spread operator in those cases, and expect diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index e32d101d3809..f19ab5b49e37 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -2,6 +2,7 @@ import type { TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; +import type { TypeOrValueSpecifier } from '../util'; import { createRule, getConstrainedTypeAtLocation, @@ -11,7 +12,6 @@ import { isTypeFlagSet, readonlynessOptionsSchema, typeMatchesSpecifier, - TypeOrValueSpecifier, } from '../util'; type Options = [ @@ -73,11 +73,6 @@ export default createRule({ { type: 'object', properties: { - allowForKnownSafeIterables: { - ...readonlynessOptionsSchema.properties.allow, - description: - 'An array of iterables type specifiers that are known to be safe to spread in objects.', - }, allowStrings: { description: 'Whether to allow spreading strings in arrays. Defaults to false.', @@ -88,11 +83,6 @@ export default createRule({ 'Whether to allow spreading functions without properties in objects. Defaults to false.', type: 'boolean', }, - allowIterables: { - description: - 'Whether to allow spreading iterables in objects. Defaults to false.', - type: 'boolean', - }, allowClassInstances: { description: 'Whether to allow spreading class instances in objects. Defaults to false.', @@ -103,6 +93,11 @@ export default createRule({ 'Whether to allow spreading class declarations in objects. Defaults to false.', type: 'boolean', }, + allowForKnownSafeIterables: { + ...readonlynessOptionsSchema.properties.allow, + description: + 'An array of iterables type specifiers that are known to be safe to spread in objects.', + }, }, additionalProperties: false, }, @@ -197,7 +192,7 @@ export default createRule({ return; } - const isTypeAllowed = () => { + const isTypeAllowed = (): boolean => { const allowedTypes = options.allowForKnownSafeIterables ?? []; return allowedTypes.some(specifier => diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot index b59e4720d81a..777cf7136d8c 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot @@ -48,6 +48,7 @@ const getObjSpread = { ...getObj() }; exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 3`] = ` "Incorrect +// { "allowStrings": false } const fruit = 'Apple'; const chars = [...fruit]; ~~~~~~~~ Using the spread operator on a string can cause unexpected behavior. Prefer \`String.split('')\` instead. @@ -55,22 +56,22 @@ const chars = [...fruit]; `; exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 4`] = ` -"Incorrect - -declare function doWork(); +"Correct +Options: {"allowStrings": true} -const spread = { ...doWork }; - ~~~~~~~~~ Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function? +// { "allowStrings": true } +const fruit = 'Apple'; +const chars = [...fruit]; " `; exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 5`] = ` "Incorrect -const set = new Set([1, 2, 3]); +declare function doWork(); -const spread = { ...set }; - ~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. +const spread = { ...doWork }; + ~~~~~~~~~ Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function? " `; @@ -107,3 +108,27 @@ const spread = { ...User }; ~~~~~~~ Using the spread operator on class declarations will spread only their static properties, and will lose their class prototype. " `; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 8`] = ` +"Incorrect + +type UnsafeIterable = Iterable; + +declare const iterable: UnsafeIterable; + +const spread = { ...iterable }; + ~~~~~~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. +" +`; + +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 9`] = ` +"Correct +Options: {"allowForKnownSafeIterables":["SafeIterable"]} + +type SafeIterable = Iterable; + +declare const iterable: SafeIterable; + +const spread = { ...iterable }; +" +`; diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 0f268aa0662c..52542e4a2b62 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -791,7 +791,7 @@ ruleTester.run('no-misused-spread', rule, { { code: ` - declare const ref = new WeakRef<{ a: number }>; + declare const ref = new WeakRef<{ a: number }>(); const o = { ...ref }; `, errors: [ diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot index 93dae0523474..3d6b04f51c7e 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot @@ -16,6 +16,101 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos "description": "Whether to allow spreading class instances in objects. Defaults to false.", "type": "boolean" }, + "allowForKnownSafeIterables": { + "description": "An array of iterables type specifiers that are known to be safe to spread in objects.", + "items": { + "oneOf": [ + { + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["file"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + }, + "path": { + "type": "string" + } + }, + "required": ["from", "name"], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["lib"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + } + }, + "required": ["from", "name"], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["package"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + }, + "package": { + "type": "string" + } + }, + "required": ["from", "name", "package"], + "type": "object" + } + ] + }, + "type": "array" + }, "allowFunctions": { "description": "Whether to allow spreading functions without properties in objects. Defaults to false.", "type": "boolean" @@ -42,6 +137,24 @@ type Options = [ allowClassDeclarations?: boolean; /** Whether to allow spreading class instances in objects. Defaults to false. */ allowClassInstances?: boolean; + /** An array of iterables type specifiers that are known to be safe to spread in objects. */ + allowForKnownSafeIterables?: ( + | { + from: 'file'; + name: [string, ...string[]] | string; + path?: string; + } + | { + from: 'lib'; + name: [string, ...string[]] | string; + } + | { + from: 'package'; + name: [string, ...string[]] | string; + package: string; + } + | string + )[]; /** Whether to allow spreading functions without properties in objects. Defaults to false. */ allowFunctions?: boolean; /** Whether to allow spreading iterables in objects. Defaults to false. */ From fc57f137abc26410515909092c433b2fce92ec3e Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:53:22 +0300 Subject: [PATCH 29/37] wip --- docs/maintenance/Contributor_Tiers.mdx | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/maintenance/Contributor_Tiers.mdx b/docs/maintenance/Contributor_Tiers.mdx index fe554936d1cb..154ae4ee4fed 100644 --- a/docs/maintenance/Contributor_Tiers.mdx +++ b/docs/maintenance/Contributor_Tiers.mdx @@ -95,7 +95,10 @@ We treat both the creation and review of issues and PRs along the rough scale of #6976 - , #6992 + ,{' '} + + #6992 + @@ -106,7 +109,10 @@ We treat both the creation and review of issues and PRs along the rough scale of #6780 - , #6910 + ,{' '} + + #6910 + @@ -117,7 +123,10 @@ We treat both the creation and review of issues and PRs along the rough scale of #6107 - , #6907 + ,{' '} + + #6907 + @@ -128,7 +137,10 @@ We treat both the creation and review of issues and PRs along the rough scale of #6084 - , #6777 + ,{' '} + + #6777 + From a95cd5ffdf71c963bac632ea7b6aaf5e634deb87 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:55:37 +0300 Subject: [PATCH 30/37] wip? --- packages/eslint-plugin/tests/docs.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts index be0f0be1959c..5ccb7453ce1a 100644 --- a/packages/eslint-plugin/tests/docs.test.ts +++ b/packages/eslint-plugin/tests/docs.test.ts @@ -497,7 +497,6 @@ ${token.value}`, testCaption.push('Correct'); if (token.meta?.includes('skipValidation')) { assert.ok( - messages.length > 0, `Expected to contain at least 1 lint error (with skipValidation):\n${token.value}`, ); } else { From 094ae0234fbd504a40164fcf71dc80c1fb6086a1 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:56:43 +0300 Subject: [PATCH 31/37] wip --- .../src/rules/use-unknown-in-catch-callback-variable.ts | 4 +++- packages/eslint-plugin/tests/docs.test.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/use-unknown-in-catch-callback-variable.ts b/packages/eslint-plugin/src/rules/use-unknown-in-catch-callback-variable.ts index 4eec2d96b3c8..0c9cf63adf2b 100644 --- a/packages/eslint-plugin/src/rules/use-unknown-in-catch-callback-variable.ts +++ b/packages/eslint-plugin/src/rules/use-unknown-in-catch-callback-variable.ts @@ -52,7 +52,9 @@ export default createRule<[], MessageIds>({ messages: { useUnknown: useUnknownMessageBase, useUnknownArrayDestructuringPattern: `${useUnknownMessageBase} The thrown error may not be iterable.`, - useUnknownObjectDestructuringPattern: `${useUnknownMessageBase} The thrown error may be nullable, or may not have the expected shape.`, + useUnknownObjectDestructuringPattern: `${ + useUnknownMessageBase + } The thrown error may be nullable, or may not have the expected shape.`, addUnknownTypeAnnotationSuggestion: 'Add an explicit `: unknown` type annotation to the rejection callback variable.', addUnknownRestTypeAnnotationSuggestion: diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts index 5ccb7453ce1a..eb04e757a043 100644 --- a/packages/eslint-plugin/tests/docs.test.ts +++ b/packages/eslint-plugin/tests/docs.test.ts @@ -497,7 +497,9 @@ ${token.value}`, testCaption.push('Correct'); if (token.meta?.includes('skipValidation')) { assert.ok( - `Expected to contain at least 1 lint error (with skipValidation):\n${token.value}`, + `Expected to contain at least 1 lint error (with skipValidation):\n${ + token.value + }`, ); } else { assert.ok( From f9a1d18b28d0acfcc9ecb8b9957140d5511f4ae9 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 20 Aug 2024 21:57:22 +0300 Subject: [PATCH 32/37] FML --- packages/eslint-plugin/tests/docs.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts index eb04e757a043..ee518640efed 100644 --- a/packages/eslint-plugin/tests/docs.test.ts +++ b/packages/eslint-plugin/tests/docs.test.ts @@ -497,6 +497,7 @@ ${token.value}`, testCaption.push('Correct'); if (token.meta?.includes('skipValidation')) { assert.ok( + messages.length > 0, `Expected to contain at least 1 lint error (with skipValidation):\n${ token.value }`, From e20e49b3212779fad344ce49de974864f8eead17 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Wed, 21 Aug 2024 08:17:39 +0300 Subject: [PATCH 33/37] wip --- .../docs/rules/no-misused-spread.mdx | 180 +++--------------- .../src/rules/no-misused-spread.ts | 80 +++----- .../no-misused-spread.shot | 84 ++------ .../tests/rules/no-misused-spread.test.ts | 42 ++-- 4 files changed, 96 insertions(+), 290 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx index 9b1fa4100dc0..92d67033b754 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -58,157 +58,17 @@ const getObjSpread = { ...getObj() }; ## Options -### `allowStrings` +### `allow` -By default, this rule disallows using the spread operator on strings inside arrays. -Set `"allowStrings": true` to allow this behavior. +This option allows marking specific types as "safe" to be spread. It takes an +array of type specifiers to consider safe. - - - -```ts -const fruit = 'Apple'; -const chars = [...fruit]; -``` - - - - - -```ts option='{"allowStrings": true}' -// { "allowStrings": true } -const fruit = 'Apple'; -const chars = [...fruit]; -``` - - - -``` - -### `allowFunctions` - -By default, this rule disallows using the spread operator on functions without additional properties. -Set `"allowFunctions": true` to allow this behavior. - - - - -```ts -declare function doWork(); - -const spread = { ...doWork }; -``` - - - - - -```ts option='{"allowFunctions": true}' -// { "allowFunctions": true } -declare function doWork(); - -const spread = { ...doWork }; -``` - - - - -### `allowClassInstances` - -By default, this rule disallows using the spread operator on instances of classes. -Set `"allowClassInstances": true` to allow this behavior. - - - - -```ts -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} - -const user = new User('John'); - -const spread = { ...user }; -``` - - - - - -```ts option='{"allowClassInstances": true}' -// { "allowClassInstances": true } -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} - -const user = new User('John'); - -const spread = { ...user }; -``` - - - - -### `allowClassDeclarations` - -By default, this rule disallows using the spread operator on class declarations. -Set `"allowClassDeclarations": true` to allow this behavior. - - - - -```ts -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} - -const spread = { ...User }; -``` - - - - - -```ts option='{"allowClassDeclarations": true}' -// { "allowClassDeclarations": true } -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} - -const spread = { ...User }; -``` - - - - -### `allowForKnownSafeIterables` - -By default, this rule disallows using the spread operator on iterables. This -option allows marking specific iterable types as "safe" to be spread. - -This option takes an array of type specifiers to consider safe. Each item in the array must have one of the following forms: -- A type defined in the current file, or globally available (`"SafeIterable"`) -- A type defined in a file (`{ from: "file", name: "SafeIterable", path: "src/safe-iterable.ts" }` with `path` being an optional path relative to the project root directory) -- A type from the default library (`{ from: "lib", name: "CustomIterable" }`) -- A type from a package (`{ from: "package", name: "AwesomeIterable", package: "iterable-lib" }`, this also works for types defined in a typings package). +- A type defined in the current file, or globally available (`"SafeType"`) +- A type defined in a file (`{ from: "file", name: "SafeString", path: "src/safe-string.ts" }` with `path` being an optional path relative to the project root directory) +- A type from the default library (`{ from: "lib", name: "BrandedArray" }`) +- A type from a package (`{ from: "package", name: "ThisIsSafe", package: "safe-lib" }`, this also works for types defined in a typings package). Examples of a configuration for this option: @@ -217,10 +77,10 @@ Examples of a configuration for this option: "error", { "allowForKnownSafeIterables": [ - "SafeIterable", - { "from": "file", "name": "ProbablySafeIterable" }, - { "from": "lib", "name": "CustomIterable" }, - { "from": "package", "name": "AwesomeIterable", "package": "iterable-lib" } + "SafeType", + { "from": "file", "name": "SafeString", "path": "src/safe-string.ts" }, + { "from": "lib", "name": "BrandedArray" }, + { "from": "package", "name": "ThisIsSafe", "package": "safe-lib" } ] } ] @@ -234,18 +94,30 @@ type UnsafeIterable = Iterable; declare const iterable: UnsafeIterable; -const spread = { ...iterable }; +const spreadIterable = { ...iterable }; + +type UnsafeBrandedString = string & { __brand: 'unsafe' }; + +declare const brandedString: UnsafeBrandedString; + +const spreadBrandedString = { ...brandedString }; ``` -```ts option='{"allowForKnownSafeIterables":["SafeIterable"]}' +```ts option='{"allow":["SafeIterable", "BrandedString"]}' type SafeIterable = Iterable; declare const iterable: SafeIterable; -const spread = { ...iterable }; +const spreadIterable = { ...iterable }; + +type BrandedString = string & { __brand: 'safe' }; + +declare const brandedString: BrandedString; + +const spreadBrandedString = { ...brandedString }; ``` diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index f19ab5b49e37..33129c820898 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -16,11 +16,7 @@ import { type Options = [ { - allowStrings?: boolean; - allowFunctions?: boolean; - allowClassInstances?: boolean; - allowClassDeclarations?: boolean; - allowForKnownSafeIterables?: TypeOrValueSpecifier[]; + allow?: TypeOrValueSpecifier[]; }, ]; @@ -73,30 +69,10 @@ export default createRule({ { type: 'object', properties: { - allowStrings: { - description: - 'Whether to allow spreading strings in arrays. Defaults to false.', - type: 'boolean', - }, - allowFunctions: { - description: - 'Whether to allow spreading functions without properties in objects. Defaults to false.', - type: 'boolean', - }, - allowClassInstances: { - description: - 'Whether to allow spreading class instances in objects. Defaults to false.', - type: 'boolean', - }, - allowClassDeclarations: { - description: - 'Whether to allow spreading class declarations in objects. Defaults to false.', - type: 'boolean', - }, - allowForKnownSafeIterables: { + allow: { ...readonlynessOptionsSchema.properties.allow, description: - 'An array of iterables type specifiers that are known to be safe to spread in objects.', + 'An array of type specifiers that are known to be safe to spread.', }, }, additionalProperties: false, @@ -106,11 +82,7 @@ export default createRule({ defaultOptions: [ { - allowStrings: false, - allowFunctions: false, - allowClassInstances: false, - allowClassDeclarations: false, - allowForKnownSafeIterables: [], + allow: [], }, ], @@ -121,7 +93,11 @@ export default createRule({ function checkArraySpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); - if (!options.allowStrings && isString(type)) { + if (isTypeAllowed(type)) { + return; + } + + if (isString(type)) { context.report({ node, messageId: 'noStringSpreadInArray', @@ -134,6 +110,10 @@ export default createRule({ function checkObjectSpread(node: TSESTree.SpreadElement): void { const type = getConstrainedTypeAtLocation(services, node.argument); + if (isTypeAllowed(type)) { + return; + } + if (isPromise(services.program, type)) { context.report({ node, @@ -143,7 +123,7 @@ export default createRule({ return; } - if (!options.allowFunctions && isFunctionWithoutProps(type)) { + if (isFunctionWithoutProps(type)) { context.report({ node, messageId: 'noFunctionSpreadInObject', @@ -170,7 +150,7 @@ export default createRule({ return; } - if (!options.allowClassInstances && isClassInstance(type)) { + if (isClassInstance(type)) { context.report({ node, messageId: 'noClassInstanceSpreadInObject', @@ -179,11 +159,7 @@ export default createRule({ return; } - if ( - !options.allowClassDeclarations && - isClassDeclaration(type) && - !isClassInstance(type) - ) { + if (isClassDeclaration(type) && !isClassInstance(type)) { context.report({ node, messageId: 'noClassDeclarationSpreadInObject', @@ -192,15 +168,11 @@ export default createRule({ return; } - const isTypeAllowed = (): boolean => { - const allowedTypes = options.allowForKnownSafeIterables ?? []; - - return allowedTypes.some(specifier => - typeMatchesSpecifier(type, specifier, services.program), - ); - }; - - if (isIterable(type, checker) && !isString(type) && !isTypeAllowed()) { + if ( + isIterable(type, checker) && + // Don't report when the type is string, since TS will flag it already + !isString(type) + ) { context.report({ node, messageId: 'noIterableSpreadInObject', @@ -210,6 +182,16 @@ export default createRule({ } } + function isTypeAllowed(type: ts.Type): boolean { + if (!options.allow) { + return false; + } + + return options.allow.some(specifier => + typeMatchesSpecifier(type, specifier, services.program), + ); + } + return { 'ArrayExpression > SpreadElement': checkArraySpread, 'ObjectExpression > SpreadElement': checkObjectSpread, diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot index 777cf7136d8c..f18a788fffbf 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-spread.shot @@ -48,87 +48,35 @@ const getObjSpread = { ...getObj() }; exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 3`] = ` "Incorrect -// { "allowStrings": false } -const fruit = 'Apple'; -const chars = [...fruit]; - ~~~~~~~~ Using the spread operator on a string can cause unexpected behavior. Prefer \`String.split('')\` instead. -" -`; - -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 4`] = ` -"Correct -Options: {"allowStrings": true} - -// { "allowStrings": true } -const fruit = 'Apple'; -const chars = [...fruit]; -" -`; - -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 5`] = ` -"Incorrect - -declare function doWork(); - -const spread = { ...doWork }; - ~~~~~~~~~ Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function? -" -`; - -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 6`] = ` -"Incorrect - -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} - -const user = new User('John'); - -const spread = { ...user }; - ~~~~~~~ Using the spread operator on class instances will lose their class prototype. -" -`; - -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 7`] = ` -"Incorrect - -class User { - name: string; - - constructor(name: string) { - this.name = name; - } -} +type UnsafeIterable = Iterable; -const spread = { ...User }; - ~~~~~~~ Using the spread operator on class declarations will spread only their static properties, and will lose their class prototype. -" -`; +declare const iterable: UnsafeIterable; -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 8`] = ` -"Incorrect +const spreadIterable = { ...iterable }; + ~~~~~~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. -type UnsafeIterable = Iterable; +type UnsafeBrandedString = string & { __brand: 'unsafe' }; -declare const iterable: UnsafeIterable; +declare const brandedString: UnsafeBrandedString; -const spread = { ...iterable }; - ~~~~~~~~~~~ Using the spread operator on an Iterable in an object can cause unexpected behavior. +const spreadBrandedString = { ...brandedString }; " `; -exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 9`] = ` +exports[`Validating rule docs no-misused-spread.mdx code examples ESLint output 4`] = ` "Correct -Options: {"allowForKnownSafeIterables":["SafeIterable"]} +Options: {"allow":["SafeIterable", "BrandedString"]} type SafeIterable = Iterable; declare const iterable: SafeIterable; -const spread = { ...iterable }; +const spreadIterable = { ...iterable }; + +type BrandedString = string & { __brand: 'safe' }; + +declare const brandedString: BrandedString; + +const spreadBrandedString = { ...brandedString }; " `; diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 52542e4a2b62..2f60e4038b8b 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -163,6 +163,14 @@ ruleTester.run('no-misused-spread', rule, { const o = { ...promiseLike }; `, + { + options: [{ allow: ['Promise'] }], + code: ` + const promise = new Promise(() => {}); + const o = { ...promise }; + `, + }, + ` interface A {} @@ -179,14 +187,15 @@ ruleTester.run('no-misused-spread', rule, { `, { - options: [{ allowStrings: true }], + options: [{ allow: ['string'] }], code: ` - const a = [...'test']; + const str: string = 'test'; + const a = [...str]; `, }, { - options: [{ allowFunctions: true }], + options: [{ allow: ['f'] }], code: ` function f() {} @@ -197,7 +206,7 @@ ruleTester.run('no-misused-spread', rule, { { options: [ { - allowForKnownSafeIterables: [{ from: 'lib', name: 'Iterable' }], + allow: [{ from: 'lib', name: 'Iterable' }], }, ], code: ` @@ -208,7 +217,7 @@ ruleTester.run('no-misused-spread', rule, { }, { - options: [{ allowForKnownSafeIterables: ['CustomIterable'] }], + options: [{ allow: ['CustomIterable'] }], code: ` type CustomIterable = { [Symbol.iterator]: () => Generator; @@ -223,9 +232,7 @@ ruleTester.run('no-misused-spread', rule, { { options: [ { - allowForKnownSafeIterables: [ - { from: 'file', name: 'CustomIterable' }, - ], + allow: [{ from: 'file', name: 'CustomIterable' }], }, ], code: ` @@ -242,7 +249,7 @@ ruleTester.run('no-misused-spread', rule, { { options: [ { - allowForKnownSafeIterables: [ + allow: [ { from: 'package', package: 'module', name: 'CustomIterable' }, ], }, @@ -263,7 +270,7 @@ ruleTester.run('no-misused-spread', rule, { }, { - options: [{ allowClassInstances: true }], + options: [{ allow: ['A'] }], code: ` class A { a = 1; @@ -276,7 +283,7 @@ ruleTester.run('no-misused-spread', rule, { }, { - options: [{ allowClassDeclarations: true }], + options: [{ allow: ['A'] }], code: ` const a = { ...class A { @@ -1025,7 +1032,7 @@ ruleTester.run('no-misused-spread', rule, { const a = { ...iterator }; `, - options: [{ allowForKnownSafeIterables: ['AnotherIterable'] }], + options: [{ allow: ['AnotherIterable'] }], errors: [ { messageId: 'noIterableSpreadInObject', @@ -1051,9 +1058,7 @@ ruleTester.run('no-misused-spread', rule, { `, options: [ { - allowForKnownSafeIterables: [ - { from: 'package', package: 'module', name: 'Nothing' }, - ], + allow: [{ from: 'package', package: 'module', name: 'Nothing' }], }, ], errors: [ @@ -1115,7 +1120,6 @@ ruleTester.run('no-misused-spread', rule, { }, { - options: [{ allowClassInstances: true }], code: ` class A { [Symbol.iterator]() { @@ -1131,7 +1135,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noIterableSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', line: 12, column: 21, endColumn: 31, @@ -1176,9 +1180,9 @@ ruleTester.run('no-misused-spread', rule, { errors: [ { messageId: 'noClassInstanceSpreadInObject', - line: 2, + line: 3, column: 21, - endColumn: 34, + endColumn: 29, }, ], }, From af82cbb37a028d78723232e5d79cb9784ab67c52 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Wed, 21 Aug 2024 08:21:42 +0300 Subject: [PATCH 34/37] update snapshot --- .../schema-snapshots/no-misused-spread.shot | 38 ++----------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot index 3d6b04f51c7e..09a7e195ef81 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-spread.shot @@ -8,16 +8,8 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos { "additionalProperties": false, "properties": { - "allowClassDeclarations": { - "description": "Whether to allow spreading class declarations in objects. Defaults to false.", - "type": "boolean" - }, - "allowClassInstances": { - "description": "Whether to allow spreading class instances in objects. Defaults to false.", - "type": "boolean" - }, - "allowForKnownSafeIterables": { - "description": "An array of iterables type specifiers that are known to be safe to spread in objects.", + "allow": { + "description": "An array of type specifiers that are known to be safe to spread.", "items": { "oneOf": [ { @@ -110,18 +102,6 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos ] }, "type": "array" - }, - "allowFunctions": { - "description": "Whether to allow spreading functions without properties in objects. Defaults to false.", - "type": "boolean" - }, - "allowIterables": { - "description": "Whether to allow spreading iterables in objects. Defaults to false.", - "type": "boolean" - }, - "allowStrings": { - "description": "Whether to allow spreading strings in arrays. Defaults to false.", - "type": "boolean" } }, "type": "object" @@ -133,12 +113,8 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos type Options = [ { - /** Whether to allow spreading class declarations in objects. Defaults to false. */ - allowClassDeclarations?: boolean; - /** Whether to allow spreading class instances in objects. Defaults to false. */ - allowClassInstances?: boolean; - /** An array of iterables type specifiers that are known to be safe to spread in objects. */ - allowForKnownSafeIterables?: ( + /** An array of type specifiers that are known to be safe to spread. */ + allow?: ( | { from: 'file'; name: [string, ...string[]] | string; @@ -155,12 +131,6 @@ type Options = [ } | string )[]; - /** Whether to allow spreading functions without properties in objects. Defaults to false. */ - allowFunctions?: boolean; - /** Whether to allow spreading iterables in objects. Defaults to false. */ - allowIterables?: boolean; - /** Whether to allow spreading strings in arrays. Defaults to false. */ - allowStrings?: boolean; }, ]; " From 9ff1ba1cbbfe3eab406397a4c16120fa6366c71b Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Thu, 22 Aug 2024 20:30:30 +0300 Subject: [PATCH 35/37] docs --- packages/eslint-plugin/docs/rules/no-misused-spread.mdx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx index 92d67033b754..a383879b0bb7 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -11,6 +11,14 @@ import TabItem from '@theme/TabItem'; The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript that can be misused in ways not always detectable by TypeScript. This rule disallows using the spread operator on types where spreading can lead to unexpected behavior. +This rule disallows the spread operator in the following cases: + +- Spreading a string into an array +- Spreading a `Promise` into an object +- Spreading a function without properties into an object +- Spreading a class instance or declaration into an object +- Spreading an iterable (`Map`, `Array`, etc.) into an object + ## Examples From 9405682e81e2bccdd1d14e4c22088de8480d83cf Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 10 Sep 2024 21:40:50 +0300 Subject: [PATCH 36/37] wip --- .../docs/rules/no-misused-spread.mdx | 15 +++++----- .../src/rules/no-misused-spread.ts | 28 ++++++++----------- .../tests/fixtures/tsconfig.json | 2 +- .../tests/rules/no-misused-spread.test.ts | 8 +++--- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx index a383879b0bb7..17ba58cc6aff 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-spread.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-spread.mdx @@ -9,7 +9,9 @@ import TabItem from '@theme/TabItem'; > > See **https://typescript-eslint.io/rules/no-misused-spread** for documentation. -The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a powerful feature in JavaScript that can be misused in ways not always detectable by TypeScript. This rule disallows using the spread operator on types where spreading can lead to unexpected behavior. +The [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) (`...`) is a JavaScript +feature that can be misused in ways not always reported by TypeScript. This rule disallows using the spread operator on types where +spreading can lead to unexpected behavior. This rule disallows the spread operator in the following cases: @@ -71,12 +73,7 @@ const getObjSpread = { ...getObj() }; This option allows marking specific types as "safe" to be spread. It takes an array of type specifiers to consider safe. -Each item in the array must have one of the following forms: - -- A type defined in the current file, or globally available (`"SafeType"`) -- A type defined in a file (`{ from: "file", name: "SafeString", path: "src/safe-string.ts" }` with `path` being an optional path relative to the project root directory) -- A type from the default library (`{ from: "lib", name: "BrandedArray" }`) -- A type from a package (`{ from: "package", name: "ThisIsSafe", package: "safe-lib" }`, this also works for types defined in a typings package). +This option takes the shared [`TypeOrValueSpecifier` format](/packages/type-utils/type-or-value-specifier). Examples of a configuration for this option: @@ -138,3 +135,7 @@ the specific behavior that comes with it, you might not want this rule. For example, when you want to spread an array into an object and expect the result to be an object with the array elements as values and the array indices as keys. + +If your use cases for unusual spreads only involve a few types, you might consider using +[ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) +and/or the [`allow` option](#allow) instead of completely disabling this rule. diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 33129c820898..38b232743d29 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -150,32 +150,32 @@ export default createRule({ return; } - if (isClassInstance(type)) { + if ( + isIterable(type, checker) && + // Don't report when the type is string, since TS will flag it already + !isString(type) + ) { context.report({ node, - messageId: 'noClassInstanceSpreadInObject', + messageId: 'noIterableSpreadInObject', }); return; } - if (isClassDeclaration(type) && !isClassInstance(type)) { + if (isClassInstance(type)) { context.report({ node, - messageId: 'noClassDeclarationSpreadInObject', + messageId: 'noClassInstanceSpreadInObject', }); return; } - if ( - isIterable(type, checker) && - // Don't report when the type is string, since TS will flag it already - !isString(type) - ) { + if (isClassDeclaration(type)) { context.report({ node, - messageId: 'noIterableSpreadInObject', + messageId: 'noClassDeclarationSpreadInObject', }); return; @@ -257,12 +257,8 @@ function isClassDeclaration(type: ts.Type): boolean { } function isMap(program: ts.Program, type: ts.Type): boolean { - return isTypeRecurser( - type, - t => - isBuiltinSymbolLike(program, t, 'Map') || - isBuiltinSymbolLike(program, t, 'ReadonlyMap') || - isBuiltinSymbolLike(program, t, 'WeakMap'), + return isTypeRecurser(type, t => + isBuiltinSymbolLike(program, t, ['Map', 'ReadonlyMap', 'WeakMap']), ); } diff --git a/packages/eslint-plugin/tests/fixtures/tsconfig.json b/packages/eslint-plugin/tests/fixtures/tsconfig.json index c16815aaf1ac..a69fbcb40e40 100644 --- a/packages/eslint-plugin/tests/fixtures/tsconfig.json +++ b/packages/eslint-plugin/tests/fixtures/tsconfig.json @@ -5,7 +5,7 @@ "module": "commonjs", "strict": true, "esModuleInterop": true, - "lib": ["es2015", "es2017", "esnext"], + "lib": ["es2015", "es2017", "esnext", "DOM"], "experimentalDecorators": true }, "include": [ diff --git a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts index 2f60e4038b8b..2ca7c6df93f4 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-spread.test.ts @@ -798,7 +798,7 @@ ruleTester.run('no-misused-spread', rule, { { code: ` - declare const ref = new WeakRef<{ a: number }>(); + const ref = new WeakRef({ a: 1 }); const o = { ...ref }; `, errors: [ @@ -1135,7 +1135,7 @@ ruleTester.run('no-misused-spread', rule, { `, errors: [ { - messageId: 'noClassInstanceSpreadInObject', + messageId: 'noIterableSpreadInObject', line: 12, column: 21, endColumn: 31, @@ -1165,9 +1165,9 @@ ruleTester.run('no-misused-spread', rule, { errors: [ { messageId: 'noClassInstanceSpreadInObject', - line: 2, + line: 3, column: 21, - endColumn: 34, + endColumn: 31, }, ], }, From fc0e1141f7f3f327c8a1d185313c33ade384fa97 Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 24 Sep 2024 20:04:30 +0300 Subject: [PATCH 37/37] fix tests --- .../src/rules/no-misused-spread.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-spread.ts b/packages/eslint-plugin/src/rules/no-misused-spread.ts index 38b232743d29..5c4ffa8a409f 100644 --- a/packages/eslint-plugin/src/rules/no-misused-spread.ts +++ b/packages/eslint-plugin/src/rules/no-misused-spread.ts @@ -163,7 +163,7 @@ export default createRule({ return; } - if (isClassInstance(type)) { + if (isClassInstance(services.program, type)) { context.report({ node, messageId: 'noClassInstanceSpreadInObject', @@ -229,8 +229,26 @@ function isPromise(program: ts.Program, type: ts.Type): boolean { return isTypeRecurser(type, t => isPromiseLike(program, t)); } -function isClassInstance(type: ts.Type): boolean { +// Builtin classes that are known to be problematic when spread, +// but can't be detected in a reliable way. +const BUILTIN_CLASSES = ['WeakRef']; + +function isClassInstance(program: ts.Program, type: ts.Type): boolean { return isTypeRecurser(type, t => { + const symbol = t.getSymbol(); + + if (!symbol) { + return false; + } + + const isBuiltinProblematic = BUILTIN_CLASSES.some(name => + isBuiltinSymbolLike(program, t, name), + ); + + if (isBuiltinProblematic) { + return true; + } + return ( t.isClassOrInterface() && tsutils.isSymbolFlagSet(t.symbol, ts.SymbolFlags.Value)