From 1f024a238c40d672c137a6948bf4d3f2803ab9df Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Thu, 19 Sep 2024 07:18:57 -0500 Subject: [PATCH 01/14] bring previous PR --- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-misused-object-likes.ts | 149 +++++++++++++++ .../rules/no-misused-object-likes.test.ts | 174 ++++++++++++++++++ 3 files changed, 325 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/no-misused-object-likes.ts create mode 100644 packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 49d8bd67c5cb..003d50e0dbe6 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -52,6 +52,7 @@ import noLossOfPrecision from './no-loss-of-precision'; import noMagicNumbers from './no-magic-numbers'; import noMeaninglessVoidOperator from './no-meaningless-void-operator'; import noMisusedNew from './no-misused-new'; +import noMisusedObjectLikes from './no-misused-object-likes'; import noMisusedPromises from './no-misused-promises'; import noMixedEnums from './no-mixed-enums'; import noNamespace from './no-namespace'; @@ -181,6 +182,7 @@ export default { 'no-magic-numbers': noMagicNumbers, 'no-meaningless-void-operator': noMeaninglessVoidOperator, 'no-misused-new': noMisusedNew, + 'no-misused-object-likes': noMisusedObjectLikes, 'no-misused-promises': noMisusedPromises, 'no-mixed-enums': noMixedEnums, 'no-namespace': noNamespace, diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts new file mode 100644 index 000000000000..e0ccbdd0c597 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -0,0 +1,149 @@ +import type { Identifier, MemberExpression } from '@typescript-eslint/ast-spec'; +import type { TSESTree } from '@typescript-eslint/utils'; + +import { createRule, getParserServices } from '../util'; + +export type Options = [ + { + checkObjectKeysForMap?: boolean; + checkObjectValuesForMap?: boolean; + checkObjectEntriesForMap?: boolean; + checkObjectKeysForSet?: boolean; + checkObjectValuesForSet?: boolean; + checkObjectEntriesForSet?: boolean; + }, +]; +export type MessageIds = + | 'objectKeysForMap' + | 'objectValuesForMap' + | 'objectEntriesForMap' + | 'objectKeysForSet' + | 'objectValuesForSet' + | 'objectEntriesForSet'; + +export default createRule({ + name: 'no-misused-object-likes', + defaultOptions: [ + { + checkObjectKeysForMap: true, + checkObjectValuesForMap: true, + checkObjectEntriesForMap: true, + checkObjectKeysForSet: true, + checkObjectValuesForSet: true, + checkObjectEntriesForSet: true, + }, + ], + + meta: { + type: 'problem', + docs: { + description: + 'Enforce check `Object.values(...)`, `Object.keys(...)`, `Object.entries(...)` usage with Map/Set objects', + requiresTypeChecking: false, + }, + messages: { + objectKeysForMap: "Don't use `Object.keys()` for Map objects", + objectValuesForMap: "Don't use `Object.values()` for Map objects", + objectEntriesForMap: "Don't use `Object.entries()` for Map objects", + objectKeysForSet: "Don't use `Object.keys()` for Set", + objectValuesForSet: "Don't use `Object.values()` for Set", + objectEntriesForSet: "Don't use `Object.entries()` for Set", + }, + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + checkObjectKeysForMap: { + description: 'Check usage Object.keys for Map object', + type: 'boolean', + }, + checkObjectValuesForMap: { + description: 'Check usage Object.values for Map object', + type: 'boolean', + }, + checkObjectEntriesForMap: { + description: 'Check usage Object.entries for Map object', + type: 'boolean', + }, + checkObjectKeysForSet: { + description: 'Check usage Object.keys for Set object', + type: 'boolean', + }, + checkObjectValuesForSet: { + description: 'Check usage Object.values for Set object', + type: 'boolean', + }, + checkObjectEntriesForSet: { + description: 'Check usage Object.entries for Set object', + type: 'boolean', + }, + }, + }, + ], + }, + + create(context, [options]) { + const services = getParserServices(context); + + function checkObjectMethodCall( + callExpression: TSESTree.CallExpression, + ): void { + const argument = callExpression.arguments[0]; + const type = services.getTypeAtLocation(argument); + const argumentTypeName = type.getSymbol()?.name; + const callee = callExpression.callee as MemberExpression; + const objectMethod = (callee.property as Identifier).name; + + if (argumentTypeName === 'Map') { + if (objectMethod === 'keys' && options.checkObjectKeysForMap) { + context.report({ + node: callExpression, + messageId: 'objectKeysForMap', + }); + } + if (objectMethod === 'values' && options.checkObjectValuesForMap) { + context.report({ + node: callExpression, + messageId: 'objectValuesForMap', + }); + } + if (objectMethod === 'entries' && options.checkObjectEntriesForMap) { + context.report({ + node: callExpression, + messageId: 'objectEntriesForMap', + }); + } + } + if (argumentTypeName === 'Set') { + if (objectMethod === 'keys' && options.checkObjectKeysForSet) { + context.report({ + node: callExpression, + messageId: 'objectKeysForSet', + }); + } + if (objectMethod === 'values' && options.checkObjectValuesForSet) { + context.report({ + node: callExpression, + messageId: 'objectValuesForSet', + }); + } + if (objectMethod === 'entries' && options.checkObjectEntriesForSet) { + context.report({ + node: callExpression, + messageId: 'objectEntriesForSet', + }); + } + } + } + + return { + 'CallExpression[callee.object.name=Object][callee.property.name=keys][arguments.length=1]': + checkObjectMethodCall, + 'CallExpression[callee.object.name=Object][callee.property.name=values][arguments.length=1]': + checkObjectMethodCall, + 'CallExpression[callee.object.name=Object][callee.property.name=entries][arguments.length=1]': + checkObjectMethodCall, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts new file mode 100644 index 000000000000..e4fe49033601 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -0,0 +1,174 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/no-misused-object-likes'; +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-object-likes', rule, { + valid: [ + { + code: ` + class ExMap extends Map {} + const map = new ExMap(); + Object.keys(map); + `, + }, + { + code: ` + class ExMap extends Map {} + const map = new ExMap(); + Object.values(map); + `, + }, + { + code: ` + class ExMap extends Map {} + const map = new ExMap(); + Object.entries(map); + `, + }, + { + code: ` + const test = {}; + Object.entries(test); + `, + }, + { + code: ` + const test = {}; + Object.keys(test); + `, + }, + { + code: ` + const test = {}; + Object.values(test); + `, + }, + { + code: ` + const test = []; + Object.keys(test); + `, + }, + { + code: ` + const test = []; + Object.values(test); + `, + }, + { + code: ` + const test = []; + Object.entries(test); + `, + }, + { + options: [{ checkObjectKeysForMap: false }], + code: ` + const map = new Map(); + const result = Object.keys(map); + `, + }, + { + options: [{ checkObjectEntriesForMap: false }], + code: ` + const map = new Map(); + const result = Object.entries(map); + `, + }, + { + options: [{ checkObjectValuesForMap: false }], + code: ` + const map = new Map(); + const result = Object.values(map); + `, + }, + { + options: [{ checkObjectKeysForSet: false }], + code: ` + const set = new Set(); + const result = Object.keys(set); + `, + }, + { + options: [{ checkObjectEntriesForSet: false }], + code: ` + const set = new Set(); + const result = Object.entries(set); + `, + }, + { + options: [{ checkObjectValuesForSet: false }], + code: ` + const set = new Set(); + const result = Object.values(set); + `, + }, + { + code: ` + const test = 123; + Object.keys(test); + `, + }, + { + code: ` + const test = new WeakMap(); + Object.keys(test); + `, + }, + ], + invalid: [ + { + code: ` + const map = new Map(); + const result = Object.keys(map); + `, + errors: [{ messageId: 'objectKeysForMap' }], + }, + { + code: ` + const map = new Map(); + const result = Object.entries(map); + `, + errors: [{ messageId: 'objectEntriesForMap' }], + }, + { + code: ` + const map = new Map(); + const result = Object.values(map); + `, + errors: [{ messageId: 'objectValuesForMap' }], + }, + { + code: ` + const set = new Set(); + const result = Object.keys(set); + `, + errors: [{ messageId: 'objectKeysForSet' }], + }, + { + code: ` + const set = new Set(); + const result = Object.entries(set); + `, + errors: [{ messageId: 'objectEntriesForSet' }], + }, + { + code: ` + const set = new Set(); + const result = Object.values(set); + `, + errors: [{ messageId: 'objectValuesForSet' }], + }, + ], +}); From 0e39ae1f29598aab58e955fe6fab8648d3e70352 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Thu, 19 Sep 2024 07:49:02 -0500 Subject: [PATCH 02/14] cleanup --- .../src/rules/no-misused-object-likes.ts | 174 +++++------------- .../rules/no-misused-object-likes.test.ts | 174 ++++++------------ 2 files changed, 99 insertions(+), 249 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index e0ccbdd0c597..b825b243be24 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,149 +1,59 @@ -import type { Identifier, MemberExpression } from '@typescript-eslint/ast-spec'; -import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule, getParserServices } from '../util'; +import { + createRule, + getParserServices, + getStaticMemberAccessValue, +} from '../util'; -export type Options = [ - { - checkObjectKeysForMap?: boolean; - checkObjectValuesForMap?: boolean; - checkObjectEntriesForMap?: boolean; - checkObjectKeysForSet?: boolean; - checkObjectValuesForSet?: boolean; - checkObjectEntriesForSet?: boolean; - }, -]; -export type MessageIds = - | 'objectKeysForMap' - | 'objectValuesForMap' - | 'objectEntriesForMap' - | 'objectKeysForSet' - | 'objectValuesForSet' - | 'objectEntriesForSet'; - -export default createRule({ +export default createRule({ name: 'no-misused-object-likes', - defaultOptions: [ - { - checkObjectKeysForMap: true, - checkObjectValuesForMap: true, - checkObjectEntriesForMap: true, - checkObjectKeysForSet: true, - checkObjectValuesForSet: true, - checkObjectEntriesForSet: true, - }, - ], + defaultOptions: [], meta: { type: 'problem', docs: { description: 'Enforce check `Object.values(...)`, `Object.keys(...)`, `Object.entries(...)` usage with Map/Set objects', - requiresTypeChecking: false, + requiresTypeChecking: true, }, messages: { - objectKeysForMap: "Don't use `Object.keys()` for Map objects", - objectValuesForMap: "Don't use `Object.values()` for Map objects", - objectEntriesForMap: "Don't use `Object.entries()` for Map objects", - objectKeysForSet: "Don't use `Object.keys()` for Set", - objectValuesForSet: "Don't use `Object.values()` for Set", - objectEntriesForSet: "Don't use `Object.entries()` for Set", + misusedObjectLike: + "Don't use `Object.{{method}}()` on {{objectClass}} objects — it will not properly check the contents.", }, - schema: [ - { - type: 'object', - additionalProperties: false, - properties: { - checkObjectKeysForMap: { - description: 'Check usage Object.keys for Map object', - type: 'boolean', - }, - checkObjectValuesForMap: { - description: 'Check usage Object.values for Map object', - type: 'boolean', - }, - checkObjectEntriesForMap: { - description: 'Check usage Object.entries for Map object', - type: 'boolean', - }, - checkObjectKeysForSet: { - description: 'Check usage Object.keys for Set object', - type: 'boolean', - }, - checkObjectValuesForSet: { - description: 'Check usage Object.values for Set object', - type: 'boolean', - }, - checkObjectEntriesForSet: { - description: 'Check usage Object.entries for Set object', - type: 'boolean', - }, - }, - }, - ], + schema: [], }, - create(context, [options]) { - const services = getParserServices(context); - - function checkObjectMethodCall( - callExpression: TSESTree.CallExpression, - ): void { - const argument = callExpression.arguments[0]; - const type = services.getTypeAtLocation(argument); - const argumentTypeName = type.getSymbol()?.name; - const callee = callExpression.callee as MemberExpression; - const objectMethod = (callee.property as Identifier).name; - - if (argumentTypeName === 'Map') { - if (objectMethod === 'keys' && options.checkObjectKeysForMap) { - context.report({ - node: callExpression, - messageId: 'objectKeysForMap', - }); - } - if (objectMethod === 'values' && options.checkObjectValuesForMap) { - context.report({ - node: callExpression, - messageId: 'objectValuesForMap', - }); - } - if (objectMethod === 'entries' && options.checkObjectEntriesForMap) { - context.report({ - node: callExpression, - messageId: 'objectEntriesForMap', - }); - } + create: context => ({ + CallExpression(node): void { + const { arguments: args, callee } = node; + if ( + args.length !== 1 || + callee.type !== AST_NODE_TYPES.MemberExpression + ) { + return; } - if (argumentTypeName === 'Set') { - if (objectMethod === 'keys' && options.checkObjectKeysForSet) { - context.report({ - node: callExpression, - messageId: 'objectKeysForSet', - }); - } - if (objectMethod === 'values' && options.checkObjectValuesForSet) { - context.report({ - node: callExpression, - messageId: 'objectValuesForSet', - }); - } - if (objectMethod === 'entries' && options.checkObjectEntriesForSet) { - context.report({ - node: callExpression, - messageId: 'objectEntriesForSet', - }); - } + const { object } = callee; + if ( + object.type !== AST_NODE_TYPES.Identifier || + object.name !== 'Object' + ) { + return; } - } - - return { - 'CallExpression[callee.object.name=Object][callee.property.name=keys][arguments.length=1]': - checkObjectMethodCall, - 'CallExpression[callee.object.name=Object][callee.property.name=values][arguments.length=1]': - checkObjectMethodCall, - 'CallExpression[callee.object.name=Object][callee.property.name=entries][arguments.length=1]': - checkObjectMethodCall, - }; - }, + const method = getStaticMemberAccessValue(callee, context); + if (method && !['keys', 'values', 'entries'].includes(method)) { + return; + } + const objectClass = getParserServices(context) + .getTypeAtLocation(args[0]) + .getSymbol()?.name; + if (objectClass && /^(Weak)?(Map|Set)$/.test(objectClass)) { + context.report({ + node, + messageId: 'misusedObjectLike', + data: { method, objectClass }, + }); + } + }, + }), }); diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index e4fe49033601..19331fef00db 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -6,169 +6,109 @@ 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', + }, }, }); ruleTester.run('no-misused-object-likes', rule, { valid: [ + ` + class ExMap extends Map {} + const map = new ExMap(); + Object.keys(map); + `, + ` + class ExMap extends Map {} + const map = new ExMap(); + Object.values(map); + `, + ` + class ExMap extends Map {} + const map = new ExMap(); + Object.entries(map); + `, + ` + const test = {}; + Object.entries(test); + `, + ` + const test = {}; + Object.keys(test); + `, + ` + const test = {}; + Object.values(test); + `, + ` + const test = []; + Object.keys(test); + `, + ` + const test = []; + Object.values(test); + `, + ` + const test = []; + Object.entries(test); + `, + ` + const test = 123; + Object.keys(test); + `, + ], + invalid: [ { - code: ` - class ExMap extends Map {} - const map = new ExMap(); - Object.keys(map); - `, - }, - { - code: ` - class ExMap extends Map {} - const map = new ExMap(); - Object.values(map); - `, - }, - { - code: ` - class ExMap extends Map {} - const map = new ExMap(); - Object.entries(map); - `, - }, - { - code: ` - const test = {}; - Object.entries(test); - `, - }, - { - code: ` - const test = {}; - Object.keys(test); - `, - }, - { - code: ` - const test = {}; - Object.values(test); - `, - }, - { - code: ` - const test = []; - Object.keys(test); - `, - }, - { - code: ` - const test = []; - Object.values(test); - `, - }, - { - code: ` - const test = []; - Object.entries(test); - `, - }, - { - options: [{ checkObjectKeysForMap: false }], code: ` const map = new Map(); const result = Object.keys(map); `, + errors: [{ messageId: 'misusedObjectLike' }], }, { - options: [{ checkObjectEntriesForMap: false }], code: ` const map = new Map(); const result = Object.entries(map); `, + errors: [{ messageId: 'misusedObjectLike' }], }, { - options: [{ checkObjectValuesForMap: false }], code: ` const map = new Map(); const result = Object.values(map); `, + errors: [{ messageId: 'misusedObjectLike' }], }, { - options: [{ checkObjectKeysForSet: false }], code: ` const set = new Set(); const result = Object.keys(set); `, + errors: [{ messageId: 'misusedObjectLike' }], }, { - options: [{ checkObjectEntriesForSet: false }], code: ` const set = new Set(); const result = Object.entries(set); `, + errors: [{ messageId: 'misusedObjectLike' }], }, { - options: [{ checkObjectValuesForSet: false }], code: ` const set = new Set(); const result = Object.values(set); `, - }, - { - code: ` - const test = 123; - Object.keys(test); - `, + errors: [{ messageId: 'misusedObjectLike' }], }, { code: ` const test = new WeakMap(); Object.keys(test); `, - }, - ], - invalid: [ - { - code: ` - const map = new Map(); - const result = Object.keys(map); - `, - errors: [{ messageId: 'objectKeysForMap' }], - }, - { - code: ` - const map = new Map(); - const result = Object.entries(map); - `, - errors: [{ messageId: 'objectEntriesForMap' }], - }, - { - code: ` - const map = new Map(); - const result = Object.values(map); - `, - errors: [{ messageId: 'objectValuesForMap' }], - }, - { - code: ` - const set = new Set(); - const result = Object.keys(set); - `, - errors: [{ messageId: 'objectKeysForSet' }], - }, - { - code: ` - const set = new Set(); - const result = Object.entries(set); - `, - errors: [{ messageId: 'objectEntriesForSet' }], - }, - { - code: ` - const set = new Set(); - const result = Object.values(set); - `, - errors: [{ messageId: 'objectValuesForSet' }], + errors: [{ messageId: 'misusedObjectLike' }], }, ], }); From 54cd2418ee1fd8b4581ae9b291d6f1757fa126a1 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Fri, 4 Oct 2024 09:52:25 -0500 Subject: [PATCH 03/14] fix TS error and support readonly --- .../eslint-plugin/src/rules/no-misused-object-likes.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index b825b243be24..a9dce2708b2c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -41,13 +41,16 @@ export default createRule({ return; } const method = getStaticMemberAccessValue(callee, context); - if (method && !['keys', 'values', 'entries'].includes(method)) { + if ( + typeof method !== 'string' || + !['keys', 'values', 'entries'].includes(method) + ) { return; } const objectClass = getParserServices(context) .getTypeAtLocation(args[0]) .getSymbol()?.name; - if (objectClass && /^(Weak)?(Map|Set)$/.test(objectClass)) { + if (objectClass && /^(Readonly|Weak)?(Map|Set)$/.test(objectClass)) { context.report({ node, messageId: 'misusedObjectLike', From 813dd7df32606e3362d941a5b19f41408ce6a1d6 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Fri, 4 Oct 2024 10:38:50 -0500 Subject: [PATCH 04/14] update description --- .../eslint-plugin/src/rules/no-misused-object-likes.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index a9dce2708b2c..c4539ec9b3d3 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -6,6 +6,8 @@ import { getStaticMemberAccessValue, } from '../util'; +const METHODS = ['assign', 'entries', 'hasOwn', 'keys', 'values']; + export default createRule({ name: 'no-misused-object-likes', defaultOptions: [], @@ -13,8 +15,7 @@ export default createRule({ meta: { type: 'problem', docs: { - description: - 'Enforce check `Object.values(...)`, `Object.keys(...)`, `Object.entries(...)` usage with Map/Set objects', + description: `Disallow using \`Object.${METHODS.join(`|`)}(...)\` on Map/Set objects`, requiresTypeChecking: true, }, messages: { @@ -41,10 +42,7 @@ export default createRule({ return; } const method = getStaticMemberAccessValue(callee, context); - if ( - typeof method !== 'string' || - !['keys', 'values', 'entries'].includes(method) - ) { + if (typeof method !== 'string' || !METHODS.includes(method)) { return; } const objectClass = getParserServices(context) From ad28a4af3ef71afea0f351a6ad8c636c3bf2b157 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Sun, 6 Oct 2024 15:10:42 -0500 Subject: [PATCH 05/14] generate-configs --- packages/eslint-plugin/src/configs/all.ts | 1 + packages/eslint-plugin/src/configs/disable-type-checked.ts | 1 + packages/typescript-eslint/src/configs/all.ts | 1 + packages/typescript-eslint/src/configs/disable-type-checked.ts | 1 + 4 files changed, 4 insertions(+) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 107f369260ec..a5ce4b02ad79 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -73,6 +73,7 @@ export = { '@typescript-eslint/no-magic-numbers': 'error', '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', + '@typescript-eslint/no-misused-object-likes': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 7cf867b382f2..a36c74de433c 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -24,6 +24,7 @@ export = { '@typescript-eslint/no-for-in-array': 'off', '@typescript-eslint/no-implied-eval': 'off', '@typescript-eslint/no-meaningless-void-operator': 'off', + '@typescript-eslint/no-misused-object-likes': 'off', '@typescript-eslint/no-misused-promises': 'off', '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 1c177f1943bf..55743ebf875b 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -86,6 +86,7 @@ export default ( '@typescript-eslint/no-magic-numbers': 'error', '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', + '@typescript-eslint/no-misused-object-likes': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index b4c2afd20ec8..70075688ab36 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -31,6 +31,7 @@ export default ( '@typescript-eslint/no-for-in-array': 'off', '@typescript-eslint/no-implied-eval': 'off', '@typescript-eslint/no-meaningless-void-operator': 'off', + '@typescript-eslint/no-misused-object-likes': 'off', '@typescript-eslint/no-misused-promises': 'off', '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', From c0ba7044bffe2a781f315b8427194b4d6ce41035 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 7 Oct 2024 07:13:07 -0500 Subject: [PATCH 06/14] add docs page --- .../docs/rules/no-misused-object-likes.mdx | 29 +++++++++++++++++++ .../src/rules/no-misused-object-likes.ts | 1 - .../no-misused-object-likes.shot | 14 +++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx create mode 100644 packages/eslint-plugin/tests/schema-snapshots/no-misused-object-likes.shot diff --git a/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx new file mode 100644 index 000000000000..cbb0afaf5f6a --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx @@ -0,0 +1,29 @@ +--- +description: 'Disallow using Object methods on Map/Set objects' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-misused-object-likes** for documentation. + +Methods like `Object.assign()`, `Object.entries()`, `Object.hasOwn()`, `Object.keys()`, and `Object.values()` can be +used work with collections of data stored in objects. However, when working with `Map` or `Set` objects, even though +they are collections, using these methods are a mistake because they do not properly write to (in the case of +`Object.assign()`) or read from the object. + +This rule prevents such methods from being used on `Map` and `Set` objects. + + + + ```ts + console.log(Object.values(new Set('abc'))); + Object.assign(new Map(), { k: 'v' }); + ``` + + + ```ts + console.log([...new Set('abc').values()]); + new Map().set('k', 'v'); + ``` + + diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index c4539ec9b3d3..8723beece391 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -11,7 +11,6 @@ const METHODS = ['assign', 'entries', 'hasOwn', 'keys', 'values']; export default createRule({ name: 'no-misused-object-likes', defaultOptions: [], - meta: { type: 'problem', docs: { diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-misused-object-likes.shot b/packages/eslint-plugin/tests/schema-snapshots/no-misused-object-likes.shot new file mode 100644 index 000000000000..6c5b84e1e34e --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-misused-object-likes.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-object-likes 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; From 139d0e7dcd09e5bff81a51a09a325ef6eba7e25d Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Tue, 8 Oct 2024 06:57:59 -0500 Subject: [PATCH 07/14] WIP --- .../docs/rules/no-misused-object-likes.mdx | 7 ++- .../src/rules/no-misused-object-likes.ts | 63 ++++++++++++------- .../no-misused-object-likes.shot | 18 ++++++ .../rules/no-misused-object-likes.test.ts | 1 + 4 files changed, 64 insertions(+), 25 deletions(-) create mode 100644 packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot diff --git a/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx index cbb0afaf5f6a..5f27f8e41915 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx @@ -1,7 +1,10 @@ --- -description: 'Disallow using Object methods on Map/Set objects' +description: 'Disallow using `Object.{assign|entries|hasOwn|keys|values}(...)` and the `in` operator on Map/Set objects.' --- +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-object-likes** for documentation. @@ -27,3 +30,5 @@ This rule prevents such methods from being used on `Map` and `Set` objects. ``` + +{/* Intentionally Omitted: When Not To Use It */} diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 8723beece391..6798b1df1386 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,3 +1,6 @@ +import console from 'node:console'; + +import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { @@ -14,7 +17,7 @@ export default createRule({ meta: { type: 'problem', docs: { - description: `Disallow using \`Object.${METHODS.join(`|`)}(...)\` on Map/Set objects`, + description: `Disallow using \`Object.{${METHODS.join(`|`)}}(...)\` and the \`in\` operator on Map/Set objects`, requiresTypeChecking: true, }, messages: { @@ -24,28 +27,10 @@ export default createRule({ schema: [], }, - create: context => ({ - CallExpression(node): void { - const { arguments: args, callee } = node; - if ( - args.length !== 1 || - callee.type !== AST_NODE_TYPES.MemberExpression - ) { - return; - } - const { object } = callee; - if ( - object.type !== AST_NODE_TYPES.Identifier || - object.name !== 'Object' - ) { - return; - } - const method = getStaticMemberAccessValue(callee, context); - if (typeof method !== 'string' || !METHODS.includes(method)) { - return; - } + create(context) { + const checkClassAndReport = (node: TSESTree.Node) => { const objectClass = getParserServices(context) - .getTypeAtLocation(args[0]) + .getTypeAtLocation(node) .getSymbol()?.name; if (objectClass && /^(Readonly|Weak)?(Map|Set)$/.test(objectClass)) { context.report({ @@ -54,6 +39,36 @@ export default createRule({ data: { method, objectClass }, }); } - }, - }), + }; + return { + BinaryExpression(node): void { + if (node.operator === 'in') + console.log( + getParserServices(context).getTypeAtLocation(node.right).getSymbol() + ?.name, + ); + }, + CallExpression(node): void { + const { arguments: args, callee } = node; + if ( + args.length !== 1 || + callee.type !== AST_NODE_TYPES.MemberExpression + ) { + return; + } + const { object } = callee; + if ( + object.type !== AST_NODE_TYPES.Identifier || + object.name !== 'Object' + ) { + return; + } + const method = getStaticMemberAccessValue(callee, context); + if (typeof method !== 'string' || !METHODS.includes(method)) { + return; + } + checkClassAndReport(args[0]); + }, + }; + }, }); diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot new file mode 100644 index 000000000000..8f5e463f8210 --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot @@ -0,0 +1,18 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs no-misused-object-likes.mdx code examples ESLint output 1`] = ` +"Incorrect + +console.log(Object.values(new Set('abc'))); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Don't use \`Object.values()\` on Set objects — it will not properly check the contents. + Object.assign(new Map(), { k: 'v' }); +" +`; + +exports[`Validating rule docs no-misused-object-likes.mdx code examples ESLint output 2`] = ` +"Correct + +console.log([...new Set('abc').values()]); + new Map().set('k', 'v'); +" +`; diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index 19331fef00db..30ead008233b 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -110,5 +110,6 @@ ruleTester.run('no-misused-object-likes', rule, { `, errors: [{ messageId: 'misusedObjectLike' }], }, + { code: '4 in new Set();', errors: [{ messageId: 'misusedObjectLike' }] }, ], }); From c1ebe0761c64989f60f7abc8bd2762dc9bb0bf5e Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Tue, 8 Oct 2024 07:11:59 -0500 Subject: [PATCH 08/14] implement 'in' operator --- .../src/rules/no-misused-object-likes.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 6798b1df1386..f47411bb1ca4 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -22,13 +22,13 @@ export default createRule({ }, messages: { misusedObjectLike: - "Don't use `Object.{{method}}()` on {{objectClass}} objects — it will not properly check the contents.", + "Don't use {{used}} on {{objectClass}} objects — it will not properly check the contents.", }, schema: [], }, create(context) { - const checkClassAndReport = (node: TSESTree.Node) => { + const checkClassAndReport = (node: TSESTree.Node, used: string): void => { const objectClass = getParserServices(context) .getTypeAtLocation(node) .getSymbol()?.name; @@ -36,17 +36,15 @@ export default createRule({ context.report({ node, messageId: 'misusedObjectLike', - data: { method, objectClass }, + data: { used, objectClass }, }); } }; return { BinaryExpression(node): void { - if (node.operator === 'in') - console.log( - getParserServices(context).getTypeAtLocation(node.right).getSymbol() - ?.name, - ); + if (node.operator === 'in') { + checkClassAndReport(node.right, 'the `in` operator'); + } }, CallExpression(node): void { const { arguments: args, callee } = node; @@ -64,10 +62,9 @@ export default createRule({ return; } const method = getStaticMemberAccessValue(callee, context); - if (typeof method !== 'string' || !METHODS.includes(method)) { - return; + if (typeof method === 'string' && METHODS.includes(method)) { + checkClassAndReport(args[0], `\`Object.${method}()\``); } - checkClassAndReport(args[0]); }, }; }, From 605c8e271d5e22d297b8e5c8dc707a4d15a6c7b9 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Tue, 8 Oct 2024 07:12:27 -0500 Subject: [PATCH 09/14] lint --- packages/eslint-plugin/src/rules/no-misused-object-likes.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index f47411bb1ca4..e85483823489 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,5 +1,3 @@ -import console from 'node:console'; - import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; From 0ccee09322bd8a69c4205624b79902e166e70250 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Tue, 8 Oct 2024 07:29:06 -0500 Subject: [PATCH 10/14] check whether symbol is from default library --- .../eslint-plugin/src/rules/no-misused-object-likes.ts | 10 +++++++--- .../tests/rules/no-misused-object-likes.test.ts | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index e85483823489..29cd75e4b36e 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,3 +1,4 @@ +import { isSymbolFromDefaultLibrary } from '@typescript-eslint/type-utils'; import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; @@ -27,9 +28,12 @@ export default createRule({ create(context) { const checkClassAndReport = (node: TSESTree.Node, used: string): void => { - const objectClass = getParserServices(context) - .getTypeAtLocation(node) - .getSymbol()?.name; + const services = getParserServices(context); + const symbol = services.getTypeAtLocation(node).getSymbol(); + if (!isSymbolFromDefaultLibrary(services.program, symbol)) { + return; + } + const objectClass = symbol?.name; if (objectClass && /^(Readonly|Weak)?(Map|Set)$/.test(objectClass)) { context.report({ node, diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index 30ead008233b..3e519d9d085b 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -59,6 +59,7 @@ ruleTester.run('no-misused-object-likes', rule, { const test = 123; Object.keys(test); `, + 'Object.values(new (class Map {})());', ], invalid: [ { From a5658fd3617f7986985f34cd86bb31d7b58ddf29 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Thu, 10 Oct 2024 06:53:01 -0500 Subject: [PATCH 11/14] WIP --- .../src/rules/no-misused-object-likes.ts | 15 +++++++++------ .../tests/rules/no-misused-object-likes.test.ts | 4 ++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 29cd75e4b36e..9b26e124c24f 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -27,13 +27,15 @@ export default createRule({ }, create(context) { - const checkClassAndReport = (node: TSESTree.Node, used: string): void => { + const getSymbolIfFromDefaultLibrary = (node: TSESTree.Node) => { const services = getParserServices(context); const symbol = services.getTypeAtLocation(node).getSymbol(); - if (!isSymbolFromDefaultLibrary(services.program, symbol)) { - return; - } - const objectClass = symbol?.name; + return isSymbolFromDefaultLibrary(services.program, symbol) + ? symbol + : undefined; + }; + const checkClassAndReport = (node: TSESTree.Node, used: string): void => { + const objectClass = getSymbolIfFromDefaultLibrary(node)?.name; if (objectClass && /^(Readonly|Weak)?(Map|Set)$/.test(objectClass)) { context.report({ node, @@ -59,7 +61,8 @@ export default createRule({ const { object } = callee; if ( object.type !== AST_NODE_TYPES.Identifier || - object.name !== 'Object' + object.name !== 'Object' || + getSymbolIfFromDefaultLibrary(object) ) { return; } diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index 3e519d9d085b..cd6ecd67530d 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -60,6 +60,10 @@ ruleTester.run('no-misused-object-likes', rule, { Object.keys(test); `, 'Object.values(new (class Map {})());', + ` + const Object = { keys: () => {} }; + Object.keys(new Map()); + `, ], invalid: [ { From 8dec2824d2326009556cf41f239292b15c962bfb Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Thu, 17 Oct 2024 07:21:28 -0500 Subject: [PATCH 12/14] lint --- .../src/rules/no-misused-object-likes.ts | 12 ++++++++---- .../tests/rules/no-misused-object-likes.test.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 9b26e124c24f..9c732408a688 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,5 +1,7 @@ -import { isSymbolFromDefaultLibrary } from '@typescript-eslint/type-utils'; import type { TSESTree } from '@typescript-eslint/utils'; +import type * as ts from 'typescript'; + +import { isSymbolFromDefaultLibrary } from '@typescript-eslint/type-utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { @@ -12,7 +14,6 @@ const METHODS = ['assign', 'entries', 'hasOwn', 'keys', 'values']; export default createRule({ name: 'no-misused-object-likes', - defaultOptions: [], meta: { type: 'problem', docs: { @@ -25,9 +26,12 @@ export default createRule({ }, schema: [], }, + defaultOptions: [], create(context) { - const getSymbolIfFromDefaultLibrary = (node: TSESTree.Node) => { + const getSymbolIfFromDefaultLibrary = ( + node: TSESTree.Node, + ): ts.Symbol | undefined => { const services = getParserServices(context); const symbol = services.getTypeAtLocation(node).getSymbol(); return isSymbolFromDefaultLibrary(services.program, symbol) @@ -40,7 +44,7 @@ export default createRule({ context.report({ node, messageId: 'misusedObjectLike', - data: { used, objectClass }, + data: { objectClass, used }, }); } }; diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index cd6ecd67530d..6ddf1d9ec5e0 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -8,8 +8,8 @@ const rootPath = getFixturesRootDir(); const ruleTester = new RuleTester({ languageOptions: { parserOptions: { - tsconfigRootDir: rootPath, project: './tsconfig.json', + tsconfigRootDir: rootPath, }, }, }); From 19fea5521dd5364abd7d8ee8a5b4c2d5d029457b Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 30 Dec 2024 22:58:45 +0200 Subject: [PATCH 13/14] initial (rough) implementation --- .../docs/rules/no-misused-object-likes.mdx | 39 ++- .../src/rules/no-misused-object-likes.ts | 185 ++++++++--- .../no-misused-object-likes.shot | 17 +- .../rules/no-misused-object-likes.test.ts | 301 +++++++++++++++--- 4 files changed, 428 insertions(+), 114 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx index 5f27f8e41915..f81191eb523a 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx +++ b/packages/eslint-plugin/docs/rules/no-misused-object-likes.mdx @@ -1,5 +1,5 @@ --- -description: 'Disallow using `Object.{assign|entries|hasOwn|keys|values}(...)` and the `in` operator on Map/Set objects.' +description: 'Disallow using `Object` methods on `Map` or `Set` when it might cause unexpected behavior.' --- import Tabs from '@theme/Tabs'; @@ -9,26 +9,35 @@ import TabItem from '@theme/TabItem'; > > See **https://typescript-eslint.io/rules/no-misused-object-likes** for documentation. -Methods like `Object.assign()`, `Object.entries()`, `Object.hasOwn()`, `Object.keys()`, and `Object.values()` can be -used work with collections of data stored in objects. However, when working with `Map` or `Set` objects, even though -they are collections, using these methods are a mistake because they do not properly write to (in the case of -`Object.assign()`) or read from the object. +Object methods like `Object.keys` and `Object.values` are intended for plain objects. Using them on `Map` or `Set` is a common mistake that may lead to unexpected behavior. -This rule prevents such methods from being used on `Map` and `Set` objects. +This rule disallows such usage, encouraging correct alternatives. ```ts - console.log(Object.values(new Set('abc'))); - Object.assign(new Map(), { k: 'v' }); - ``` - + const mySet = new Set(['foo', 'bar']); + Object.values(mySet); + + const myMap = new Map([['foo', 'bar'], ['hello', 'world']]); + Object.entries(myMap); + ``` + + ```ts - console.log([...new Set('abc').values()]); - new Map().set('k', 'v'); - ``` - + const mySet = new Set(['foo', 'bar']); + mySet.keys(); + + const myMap = new Map([['foo', 'bar'], ['hello', 'world']]); + myMap.entries(); + ``` + + -{/* Intentionally Omitted: When Not To Use It */} +## When Not To Use It + +If your application extends the `Map` or `Set` types in unusual ways or manipulating their class prototype chains, you might not want this rule. + +You might consider using [ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) for those specific situations instead of completely disabling this rule. diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 9c732408a688..779272ed4297 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -1,7 +1,14 @@ -import type { TSESTree } from '@typescript-eslint/utils'; +import type { + ParserServicesWithTypeInformation, + TSESTree, +} from '@typescript-eslint/utils'; +import type { RuleContext } from '@typescript-eslint/utils/ts-eslint'; import type * as ts from 'typescript'; -import { isSymbolFromDefaultLibrary } from '@typescript-eslint/type-utils'; +import { + getConstrainedTypeAtLocation, + isBuiltinSymbolLike, +} from '@typescript-eslint/type-utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { @@ -10,71 +17,157 @@ import { getStaticMemberAccessValue, } from '../util'; -const METHODS = ['assign', 'entries', 'hasOwn', 'keys', 'values']; +type Options = []; + +type MessageIds = + | 'misuedObjectEntries' + | 'misusedHasOwn' + | 'misusedHasOwnProperty' + | 'misusedObjectAssign' + | 'misusedObjectKeys' + | 'misusedObjectValues'; -export default createRule({ +const objectConstructorMethodsMap = new Map([ + ['assign', 'misusedObjectAssign'], + ['entries', 'misuedObjectEntries'], + ['hasOwn', 'misusedHasOwn'], + ['hasOwnProperty', 'misusedHasOwnProperty'], + ['keys', 'misusedObjectKeys'], + ['values', 'misusedObjectValues'], +]); + +export default createRule({ name: 'no-misused-object-likes', meta: { type: 'problem', docs: { - description: `Disallow using \`Object.{${METHODS.join(`|`)}}(...)\` and the \`in\` operator on Map/Set objects`, + description: + 'Disallow using `Object` methods on `Map` or `Set` when it might cause unexpected behavior', requiresTypeChecking: true, }, messages: { - misusedObjectLike: - "Don't use {{used}} on {{objectClass}} objects — it will not properly check the contents.", + misuedObjectEntries: + "Using 'Object.entries()' on a '{{type}}' will return an empty array. Consider using the 'entries()' method instead.", + misusedHasOwn: + "Using 'hasOwn()' on a '{{type}}' may lead to unexpected results. Consider using the 'has(key)' method instead.", + misusedHasOwnProperty: + "Using 'hasOwnProperty()' on a '{{type}}' may lead to unexno-misused-object-likes-rulepected results. Consider using the 'has(key)' method instead.", + misusedObjectAssign: + "Using 'Object.assign()' with a '{{type}}' may lead to unexpected results. Consider alternative approaches instead.", + misusedObjectKeys: + "Using 'Object.keys()' on a '{{type}}' will return an empty array. Consider using the 'keys()' method instead.", + misusedObjectValues: + "Using 'Object.values()' on a '{{type}}' will return an empty array. Consider using the 'values()' method instead.", }, schema: [], }, defaultOptions: [], - create(context) { - const getSymbolIfFromDefaultLibrary = ( - node: TSESTree.Node, - ): ts.Symbol | undefined => { - const services = getParserServices(context); - const symbol = services.getTypeAtLocation(node).getSymbol(); - return isSymbolFromDefaultLibrary(services.program, symbol) - ? symbol - : undefined; - }; - const checkClassAndReport = (node: TSESTree.Node, used: string): void => { - const objectClass = getSymbolIfFromDefaultLibrary(node)?.name; - if (objectClass && /^(Readonly|Weak)?(Map|Set)$/.test(objectClass)) { - context.report({ - node, - messageId: 'misusedObjectLike', - data: { objectClass, used }, - }); - } - }; + const services = getParserServices(context); + return { - BinaryExpression(node): void { - if (node.operator === 'in') { - checkClassAndReport(node.right, 'the `in` operator'); - } - }, CallExpression(node): void { - const { arguments: args, callee } = node; - if ( - args.length !== 1 || - callee.type !== AST_NODE_TYPES.MemberExpression - ) { + const objectMethodName = getPotentiallyMisusedObjectConstructorMethod( + context, + services, + node, + ); + + if (objectMethodName == null) { return; } - const { object } = callee; - if ( - object.type !== AST_NODE_TYPES.Identifier || - object.name !== 'Object' || - getSymbolIfFromDefaultLibrary(object) - ) { + + const objectArgument = node.arguments.at(0); + + if (!objectArgument) { return; } - const method = getStaticMemberAccessValue(callee, context); - if (typeof method === 'string' && METHODS.includes(method)) { - checkClassAndReport(args[0], `\`Object.${method}()\``); + + const typeName = getViolatingTypeName( + services, + getConstrainedTypeAtLocation(services, objectArgument), + ); + + if (typeName) { + const messageId = objectConstructorMethodsMap.get(objectMethodName); + + if (messageId) { + context.report({ + node, + messageId, + data: { + type: typeName, + }, + }); + } } }, }; }, }); + +function getViolatingTypeName( + services: ParserServicesWithTypeInformation, + type: ts.Type, +): string | null { + if (isSet(services.program, type)) { + return 'Set'; + } + + if (isMap(services.program, type)) { + return 'Map'; + } + + return null; +} + +function isMap(program: ts.Program, type: ts.Type): boolean { + return isTypeRecurser(type, t => + isBuiltinSymbolLike(program, t, ['Map', 'ReadonlyMap', 'WeakMap']), + ); +} + +function isSet(program: ts.Program, type: ts.Type): boolean { + return isTypeRecurser(type, t => + isBuiltinSymbolLike(program, t, ['Set', 'ReadonlySet', 'WeakSet']), + ); +} + +function isTypeRecurser( + type: ts.Type, + predicate: (t: ts.Type) => boolean, +): boolean { + if (type.isUnionOrIntersection()) { + return type.types.some(t => isTypeRecurser(t, predicate)); + } + + return predicate(type); +} + +function getPotentiallyMisusedObjectConstructorMethod( + context: RuleContext, + services: ParserServicesWithTypeInformation, + node: TSESTree.CallExpression, +): string | null { + if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { + return null; + } + + const objectType = getConstrainedTypeAtLocation(services, node.callee.object); + + if (!isBuiltinSymbolLike(services.program, objectType, 'ObjectConstructor')) { + return null; + } + + const staticAccessValue = getStaticMemberAccessValue(node.callee, context); + + if (typeof staticAccessValue !== 'string') { + return null; + } + + if (!objectConstructorMethodsMap.has(staticAccessValue)) { + return null; + } + + return staticAccessValue; +} diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot index 8f5e463f8210..07b8f3d2c420 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-misused-object-likes.shot @@ -3,16 +3,23 @@ exports[`Validating rule docs no-misused-object-likes.mdx code examples ESLint output 1`] = ` "Incorrect -console.log(Object.values(new Set('abc'))); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Don't use \`Object.values()\` on Set objects — it will not properly check the contents. - Object.assign(new Map(), { k: 'v' }); +const mySet = new Set(['foo', 'bar']); + Object.values(mySet); + ~~~~~~~~~~~~~~~~~~~~ Using 'Object.values()' on a 'Set' will return an empty array. Consider using the 'values()' method instead. + +const myMap = new Map([['foo', 'bar'], ['hello', 'world']]); + Object.entries(myMap); + ~~~~~~~~~~~~~~~~~~~~~ Using 'Object.entries()' on a 'Map' will return an empty array. Consider using the 'entries()' method instead. " `; exports[`Validating rule docs no-misused-object-likes.mdx code examples ESLint output 2`] = ` "Correct -console.log([...new Set('abc').values()]); - new Map().set('k', 'v'); +const mySet = new Set(['foo', 'bar']); + mySet.keys(); + +const myMap = new Map([['foo', 'bar'], ['hello', 'world']]); + myMap.entries(); " `; diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index 6ddf1d9ec5e0..9bfe34e16e4e 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -16,105 +16,310 @@ const ruleTester = new RuleTester({ ruleTester.run('no-misused-object-likes', rule, { valid: [ + "Object.keys({ a: 'a' });", + 'Object.keys([1, 2, 3]);', + 'Object.keys([1, 2, 3] as const);', ` - class ExMap extends Map {} - const map = new ExMap(); - Object.keys(map); +declare const data: unknown; +Object.keys(data); `, ` - class ExMap extends Map {} - const map = new ExMap(); - Object.values(map); +declare const data: any; +Object.keys(data); `, ` - class ExMap extends Map {} - const map = new ExMap(); - Object.entries(map); +declare const data: [1, 2, 3]; +Object.keys(data); `, ` - const test = {}; - Object.entries(test); +declare const data: Set; +data.keys(); `, ` - const test = {}; - Object.keys(test); +declare const data: Map; +data.keys(); `, ` - const test = {}; - Object.values(test); +declare const data: Set; +Object.keys(data); `, ` - const test = []; - Object.keys(test); +declare const data: Map; +Object.keys(data); `, ` - const test = []; - Object.values(test); +declare const data: Map; +Object.keys(data); `, ` - const test = []; - Object.entries(test); +function test(data: T) { + Object.keys(data); +} `, ` - const test = 123; - Object.keys(test); +function test(data: string[]) { + Object.keys(data); +} `, - 'Object.values(new (class Map {})());', ` - const Object = { keys: () => {} }; - Object.keys(new Map()); +function test(data: Record) { + Object.keys(data); +} + `, + ` +declare const data: Iterable; +Object.keys(data); + `, + // error type + ` +Object.keys(data); + `, + ` +declare const data: Set; +keys(data); + `, + ` +declare const data: Set; +Object.create(data); + `, + ` +declare const data: Set; +Object[Symbol.iterator](data); + `, + ` +Object.keys(); `, ], invalid: [ { code: ` - const map = new Map(); - const result = Object.keys(map); +declare const data: Set; +Object.keys(data); + `, + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +declare const data: Set; +Object.keys(data, 'extra-arg'); + `, + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +declare const data: Set | number; +Object.keys(data); + `, + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +declare const data: number | (boolean | (string & Set)); +Object.keys(data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const map = new Map(); - const result = Object.entries(map); +function test>(data: T) { + Object.keys(data); +} `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 3, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const map = new Map(); - const result = Object.values(map); +class ExtendedSet extends Set {} + +declare const data: ExtendedSet; +Object.keys(data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 5, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const set = new Set(); - const result = Object.keys(set); +declare const data: Set; +Object['keys'](data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Set', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const set = new Set(); - const result = Object.entries(set); +declare const data: Map; +Object.keys(data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const set = new Set(); - const result = Object.values(set); +declare const data: Map; +Object.keys(data, 'extra-arg'); + `, + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +declare const data: Map | string; +Object.keys(data); + `, + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +declare const data: number | (boolean | (string & Map)); +Object.keys(data); + `, + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +function test>(data: T) { + Object.keys(data); +} + `, + errors: [ + { + column: 3, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], + }, + { + code: ` +class ExtendedMap extends Map {} + +declare const data: ExtendedMap; +Object.keys(data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 5, + messageId: 'misusedObjectKeys', + }, + ], }, { code: ` - const test = new WeakMap(); - Object.keys(test); +declare const data: Map; +Object['keys'](data); `, - errors: [{ messageId: 'misusedObjectLike' }], + errors: [ + { + column: 1, + data: { + type: 'Map', + }, + line: 3, + messageId: 'misusedObjectKeys', + }, + ], }, - { code: '4 in new Set();', errors: [{ messageId: 'misusedObjectLike' }] }, ], }); From b335a78b697c5f163ee26dc516b9adc3c9ab291c Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Tue, 31 Dec 2024 01:09:23 +0200 Subject: [PATCH 14/14] refactor --- .../src/rules/no-misused-object-likes.ts | 42 ++++++------ .../rules/no-misused-object-likes.test.ts | 64 ++++++++++--------- 2 files changed, 55 insertions(+), 51 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts index 779272ed4297..3fdc40fb2241 100644 --- a/packages/eslint-plugin/src/rules/no-misused-object-likes.ts +++ b/packages/eslint-plugin/src/rules/no-misused-object-likes.ts @@ -20,20 +20,20 @@ import { type Options = []; type MessageIds = - | 'misuedObjectEntries' - | 'misusedHasOwn' - | 'misusedHasOwnProperty' - | 'misusedObjectAssign' - | 'misusedObjectKeys' - | 'misusedObjectValues'; + | 'noMapOrSetInObjectAssign' + | 'noMapOrSetInObjectEntries' + | 'noMapOrSetInObjectHasOwn' + | 'noMapOrSetInObjectHasOwnProperty' + | 'noMapOrSetInObjectKeys' + | 'noMapOrSetInObjectValues'; const objectConstructorMethodsMap = new Map([ - ['assign', 'misusedObjectAssign'], - ['entries', 'misuedObjectEntries'], - ['hasOwn', 'misusedHasOwn'], - ['hasOwnProperty', 'misusedHasOwnProperty'], - ['keys', 'misusedObjectKeys'], - ['values', 'misusedObjectValues'], + ['assign', 'noMapOrSetInObjectAssign'], + ['entries', 'noMapOrSetInObjectEntries'], + ['hasOwn', 'noMapOrSetInObjectHasOwn'], + ['hasOwnProperty', 'noMapOrSetInObjectHasOwnProperty'], + ['keys', 'noMapOrSetInObjectKeys'], + ['values', 'noMapOrSetInObjectValues'], ]); export default createRule({ @@ -46,17 +46,17 @@ export default createRule({ requiresTypeChecking: true, }, messages: { - misuedObjectEntries: - "Using 'Object.entries()' on a '{{type}}' will return an empty array. Consider using the 'entries()' method instead.", - misusedHasOwn: - "Using 'hasOwn()' on a '{{type}}' may lead to unexpected results. Consider using the 'has(key)' method instead.", - misusedHasOwnProperty: - "Using 'hasOwnProperty()' on a '{{type}}' may lead to unexno-misused-object-likes-rulepected results. Consider using the 'has(key)' method instead.", - misusedObjectAssign: + noMapOrSetInObjectAssign: "Using 'Object.assign()' with a '{{type}}' may lead to unexpected results. Consider alternative approaches instead.", - misusedObjectKeys: + noMapOrSetInObjectEntries: + "Using 'Object.entries()' on a '{{type}}' will return an empty array. Consider using the 'entries()' method instead.", + noMapOrSetInObjectHasOwn: + "Using 'Object.hasOwn()' on a '{{type}}' may lead to unexpected results. Consider using the 'has(key)' method instead.", + noMapOrSetInObjectHasOwnProperty: + "Using 'Object.hasOwnProperty()' on a '{{type}}' may lead to unexpected results. Consider using the 'has(key)' method instead.", + noMapOrSetInObjectKeys: "Using 'Object.keys()' on a '{{type}}' will return an empty array. Consider using the 'keys()' method instead.", - misusedObjectValues: + noMapOrSetInObjectValues: "Using 'Object.values()' on a '{{type}}' will return an empty array. Consider using the 'values()' method instead.", }, schema: [], diff --git a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts index 9bfe34e16e4e..3cc37818ead9 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-object-likes.test.ts @@ -103,14 +103,14 @@ Object.keys(data); type: 'Set', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectKeys', }, ], }, { code: ` declare const data: Set; -Object.keys(data, 'extra-arg'); +Object.values(data, 'extra-arg'); `, errors: [ { @@ -119,14 +119,14 @@ Object.keys(data, 'extra-arg'); type: 'Set', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectValues', }, ], }, { code: ` -declare const data: Set | number; -Object.keys(data); +declare const data: Set | { a: number }; +Object.assign(data); `, errors: [ { @@ -135,14 +135,16 @@ Object.keys(data); type: 'Set', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectAssign', }, ], }, { code: ` -declare const data: number | (boolean | (string & Set)); -Object.keys(data); +declare const data: + | { a: number } + | ({ b: boolean } | ({ c: string } & Set)); +Object.entries(data); `, errors: [ { @@ -150,15 +152,15 @@ Object.keys(data); data: { type: 'Set', }, - line: 3, - messageId: 'misusedObjectKeys', + line: 5, + messageId: 'noMapOrSetInObjectEntries', }, ], }, { code: ` function test>(data: T) { - Object.keys(data); + Object.hasOwn(data, 'key'); } `, errors: [ @@ -168,7 +170,7 @@ function test>(data: T) { type: 'Set', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectHasOwn', }, ], }, @@ -177,7 +179,7 @@ function test>(data: T) { class ExtendedSet extends Set {} declare const data: ExtendedSet; -Object.keys(data); +Object.hasOwnProperty(data, 'key'); `, errors: [ { @@ -186,14 +188,14 @@ Object.keys(data); type: 'Set', }, line: 5, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectHasOwnProperty', }, ], }, { code: ` declare const data: Set; -Object['keys'](data); +Object['values'](data); `, errors: [ { @@ -202,7 +204,7 @@ Object['keys'](data); type: 'Set', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectValues', }, ], }, @@ -218,14 +220,14 @@ Object.keys(data); type: 'Map', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectKeys', }, ], }, { code: ` declare const data: Map; -Object.keys(data, 'extra-arg'); +Object.assign(data, 'extra-arg'); `, errors: [ { @@ -234,14 +236,14 @@ Object.keys(data, 'extra-arg'); type: 'Map', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectAssign', }, ], }, { code: ` -declare const data: Map | string; -Object.keys(data); +declare const data: Map | { a: string }; +Object.entries(data); `, errors: [ { @@ -250,13 +252,15 @@ Object.keys(data); type: 'Map', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectEntries', }, ], }, { code: ` -declare const data: number | (boolean | (string & Map)); +declare const data: + | { a: number } + | ({ b: boolean } | ({ c: string } & Map)); Object.keys(data); `, errors: [ @@ -265,15 +269,15 @@ Object.keys(data); data: { type: 'Map', }, - line: 3, - messageId: 'misusedObjectKeys', + line: 5, + messageId: 'noMapOrSetInObjectKeys', }, ], }, { code: ` function test>(data: T) { - Object.keys(data); + Object.hasOwn(data, 'foo'); } `, errors: [ @@ -283,7 +287,7 @@ function test>(data: T) { type: 'Map', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectHasOwn', }, ], }, @@ -292,7 +296,7 @@ function test>(data: T) { class ExtendedMap extends Map {} declare const data: ExtendedMap; -Object.keys(data); +Object.values(data); `, errors: [ { @@ -301,7 +305,7 @@ Object.keys(data); type: 'Map', }, line: 5, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectValues', }, ], }, @@ -317,7 +321,7 @@ Object['keys'](data); type: 'Map', }, line: 3, - messageId: 'misusedObjectKeys', + messageId: 'noMapOrSetInObjectKeys', }, ], },