Skip to content

Commit ced7bcd

Browse files
sindresorhussindresorhus
authored
Add prefer-object-define-properties rule (#3189)
1 parent c8f2bf5 commit ced7bcd

7 files changed

Lines changed: 783 additions & 0 deletions

‎docs/rules/prefer-object-define-properties.md‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# prefer-object-define-properties
2+
3+
📝 Prefer `Object.defineProperties()` over multiple `Object.defineProperty()` calls.
4+
5+
💼 This rule is enabled in the following [configs](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config): ✅ `recommended`, ☑️ `unopinionated`.
6+
7+
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
8+
9+
<!-- end auto-generated rule header -->
10+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
11+
12+
Prefer [`Object.defineProperties()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperties) when defining multiple properties on the same object.
13+
14+
This rule only checks adjacent `Object.defineProperty()` expression statements with the same target. `Reflect.defineProperty()` is intentionally ignored because it returns a boolean instead of throwing and there is no `Reflect.defineProperties()` equivalent.
15+
16+
The autofix is skipped when comments would be removed or when duplicate property keys can be detected.
17+
18+
## Examples
19+
20+
```js
21+
// ❌
22+
Object.defineProperty(foo, 'bar', {
23+
value: 1,
24+
writable: true,
25+
});
26+
Object.defineProperty(foo, 'baz', {
27+
value: 2,
28+
writable: true,
29+
});
30+
31+
// ✅
32+
Object.defineProperties(foo, {
33+
bar: {
34+
value: 1,
35+
writable: true,
36+
},
37+
baz: {
38+
value: 2,
39+
writable: true,
40+
},
41+
});
42+
```

‎readme.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ export default [
238238
| [prefer-number-coercion](docs/rules/prefer-number-coercion.md) | Prefer `Number()` over `parseFloat()` and base-10 `parseInt()`. | ✅ ☑️ | | 💡 |
239239
| [prefer-number-is-safe-integer](docs/rules/prefer-number-is-safe-integer.md) | Prefer `Number.isSafeInteger()` over `Number.isInteger()`. | ✅ | | 💡 |
240240
| [prefer-number-properties](docs/rules/prefer-number-properties.md) | Prefer `Number` static methods over global functions and optionally static properties over global constants. | ✅ ☑️ | 🔧 | 💡 |
241+
| [prefer-object-define-properties](docs/rules/prefer-object-define-properties.md) | Prefer `Object.defineProperties()` over multiple `Object.defineProperty()` calls. | ✅ ☑️ | 🔧 | |
241242
| [prefer-object-from-entries](docs/rules/prefer-object-from-entries.md) | Prefer using `Object.fromEntries(…)` to transform a list of key-value pairs into an object. | ✅ ☑️ | 🔧 | |
242243
| [prefer-object-iterable-methods](docs/rules/prefer-object-iterable-methods.md) | Prefer the most specific `Object` iterable method. | ✅ ☑️ | 🔧 | |
243244
| [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md) | Prefer omitting the `catch` binding parameter. | ✅ ☑️ | 🔧 | |

‎rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ export {default as 'prefer-node-protocol'} from './prefer-node-protocol.js';
179179
export {default as 'prefer-number-coercion'} from './prefer-number-coercion.js';
180180
export {default as 'prefer-number-is-safe-integer'} from './prefer-number-is-safe-integer.js';
181181
export {default as 'prefer-number-properties'} from './prefer-number-properties.js';
182+
export {default as 'prefer-object-define-properties'} from './prefer-object-define-properties.js';
182183
export {default as 'prefer-object-from-entries'} from './prefer-object-from-entries.js';
183184
export {default as 'prefer-object-iterable-methods'} from './prefer-object-iterable-methods.js';
184185
export {default as 'prefer-optional-catch-binding'} from './prefer-optional-catch-binding.js';

‎rules/prefer-object-define-properties.js‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
import {getStaticValue} from '@eslint-community/eslint-utils';
2+
import helperValidatorIdentifier from '@babel/helper-validator-identifier';
3+
import {isMethodCall} from './ast/index.js';
4+
import {
5+
getIndentString,
6+
getNextNode,
7+
getPreviousNode,
8+
isSameReference,
9+
} from './utils/index.js';
10+
11+
const {isIdentifierName} = helperValidatorIdentifier;
12+
13+
const MESSAGE_ID = 'prefer-object-define-properties';
14+
const messages = {
15+
[MESSAGE_ID]: 'Prefer `Object.defineProperties()` over multiple `Object.defineProperty()` calls.',
16+
};
17+
18+
const isDefinePropertyCall = node =>
19+
isMethodCall(node, {
20+
object: 'Object',
21+
method: 'defineProperty',
22+
argumentsLength: 3,
23+
optionalCall: false,
24+
optionalMember: false,
25+
computed: false,
26+
});
27+
28+
function getDefinePropertyCall(expressionStatement) {
29+
if (expressionStatement?.type !== 'ExpressionStatement') {
30+
return;
31+
}
32+
33+
const {expression} = expressionStatement;
34+
if (!isDefinePropertyCall(expression)) {
35+
return;
36+
}
37+
38+
return expression;
39+
}
40+
41+
function getStaticPropertyKey(node, sourceCode) {
42+
const staticValue = getStaticValue(node, sourceCode.getScope(node));
43+
44+
if (!staticValue) {
45+
return;
46+
}
47+
48+
return typeof staticValue.value === 'symbol' ? staticValue.value : String(staticValue.value);
49+
}
50+
51+
function getPropertyKeyText(node, sourceCode) {
52+
const nodeText = sourceCode.getText(node);
53+
54+
if (node.type === 'Literal') {
55+
if (typeof node.value === 'string') {
56+
if (node.value === '__proto__') {
57+
return `[${nodeText}]`;
58+
}
59+
60+
return isIdentifierName(node.value) ? node.value : nodeText;
61+
}
62+
63+
if (typeof node.value === 'number') {
64+
return nodeText;
65+
}
66+
}
67+
68+
return `[${nodeText}]`;
69+
}
70+
71+
function hasDuplicatePropertyKeys(calls, sourceCode) {
72+
const staticKeys = new Set();
73+
const dynamicKeys = [];
74+
75+
for (const call of calls) {
76+
const keyNode = call.arguments[1];
77+
const key = getStaticPropertyKey(keyNode, sourceCode);
78+
if (key === undefined) {
79+
if (dynamicKeys.some(dynamicKey => isSameReference(dynamicKey, keyNode))) {
80+
return true;
81+
}
82+
83+
dynamicKeys.push(keyNode);
84+
continue;
85+
}
86+
87+
if (staticKeys.has(key)) {
88+
return true;
89+
}
90+
91+
staticKeys.add(key);
92+
}
93+
94+
return false;
95+
}
96+
97+
function hasCommentsInRange(sourceCode, range) {
98+
return sourceCode.getAllComments().some(comment => {
99+
const [start, end] = sourceCode.getRange(comment);
100+
101+
return start >= range[0] && end <= range[1];
102+
});
103+
}
104+
105+
function getDefinePropertiesFix(expressionStatements, calls, context) {
106+
const {sourceCode} = context;
107+
const firstExpressionStatement = expressionStatements[0];
108+
const lastExpressionStatement = expressionStatements.at(-1);
109+
const indent = getIndentString(firstExpressionStatement, context);
110+
const propertyIndent = `${indent}\t`;
111+
const targetText = sourceCode.getText(calls[0].arguments[0]);
112+
const propertiesText = calls.map(call => {
113+
const propertyKeyText = getPropertyKeyText(call.arguments[1], sourceCode);
114+
const descriptorText = sourceCode.getText(call.arguments[2]).replaceAll('\n', '\n\t');
115+
116+
return `${propertyIndent}${propertyKeyText}: ${descriptorText},`;
117+
}).join('\n');
118+
119+
return fixer => fixer.replaceTextRange(
120+
[
121+
sourceCode.getRange(firstExpressionStatement)[0],
122+
sourceCode.getRange(lastExpressionStatement)[1],
123+
],
124+
`Object.defineProperties(${targetText}, {\n${propertiesText}\n${indent}});`,
125+
);
126+
}
127+
128+
/** @param {import('eslint').Rule.RuleContext} context */
129+
const create = context => {
130+
const {sourceCode} = context;
131+
132+
context.on('ExpressionStatement', expressionStatement => {
133+
const firstCall = getDefinePropertyCall(expressionStatement);
134+
if (!firstCall) {
135+
return;
136+
}
137+
138+
const previousCall = getDefinePropertyCall(getPreviousNode(expressionStatement, context));
139+
if (
140+
previousCall
141+
&& isSameReference(previousCall.arguments[0], firstCall.arguments[0])
142+
) {
143+
return;
144+
}
145+
146+
const expressionStatements = [expressionStatement];
147+
const calls = [firstCall];
148+
149+
let nextExpressionStatement = getNextNode(expressionStatement, context);
150+
while (true) {
151+
const nextCall = getDefinePropertyCall(nextExpressionStatement);
152+
153+
if (
154+
!nextCall
155+
|| !isSameReference(firstCall.arguments[0], nextCall.arguments[0])
156+
) {
157+
break;
158+
}
159+
160+
expressionStatements.push(nextExpressionStatement);
161+
calls.push(nextCall);
162+
nextExpressionStatement = getNextNode(nextExpressionStatement, context);
163+
}
164+
165+
if (calls.length < 2) {
166+
return;
167+
}
168+
169+
const problem = {
170+
node: firstCall.callee.property,
171+
messageId: MESSAGE_ID,
172+
};
173+
174+
const range = [
175+
sourceCode.getRange(expressionStatements[0])[0],
176+
sourceCode.getRange(expressionStatements.at(-1))[1],
177+
];
178+
179+
if (
180+
!hasDuplicatePropertyKeys(calls, sourceCode)
181+
&& !hasCommentsInRange(sourceCode, range)
182+
) {
183+
problem.fix = getDefinePropertiesFix(expressionStatements, calls, context);
184+
}
185+
186+
return problem;
187+
});
188+
};
189+
190+
/** @type {import('eslint').Rule.RuleModule} */
191+
const config = {
192+
create,
193+
meta: {
194+
type: 'suggestion',
195+
docs: {
196+
description: 'Prefer `Object.defineProperties()` over multiple `Object.defineProperty()` calls.',
197+
recommended: 'unopinionated',
198+
},
199+
fixable: 'code',
200+
schema: [],
201+
messages,
202+
languages: [
203+
'js/js',
204+
],
205+
},
206+
};
207+
208+
export default config;

‎test/prefer-object-define-properties.js‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import {getTester, parsers} from './utils/test.js';
2+
3+
const {test} = getTester(import.meta);
4+
5+
test.snapshot({
6+
valid: [
7+
'Object.defineProperty(foo, "bar", {value: 1});',
8+
'Object.defineProperty(foo, "bar", {value: 1});\nfoo();\nObject.defineProperty(foo, "baz", {value: 2});',
9+
'Object.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(bar, "baz", {value: 2});',
10+
'Object.defineProperty(foo, "bar");\nObject.defineProperty(foo, "baz", {value: 2});',
11+
'Object.defineProperty(foo, "bar", {value: 1}, extra);\nObject.defineProperty(foo, "baz", {value: 2});',
12+
'Object.defineProperty(...foo);\nObject.defineProperty(foo, "baz", {value: 2});',
13+
'Object.defineProperty(foo, ...bar);\nObject.defineProperty(foo, "baz", {value: 2});',
14+
'Object.defineProperty?.(foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
15+
'Object?.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
16+
'Object["defineProperty"](foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
17+
'Reflect.defineProperty(foo, "bar", {value: 1});\nReflect.defineProperty(foo, "baz", {value: 2});',
18+
'defineProperty(foo, "bar", {value: 1});\ndefineProperty(foo, "baz", {value: 2});',
19+
'const result = Object.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
20+
'Object.defineProperty(foo, "bar", {value: 1});\nconst result = Object.defineProperty(foo, "baz", {value: 2});',
21+
'Object.defineProperty(getObject(), "bar", {value: 1});\nObject.defineProperty(getObject(), "baz", {value: 2});',
22+
],
23+
invalid: [
24+
'Object.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
25+
'Object.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});\nObject.defineProperty(foo, "qux", {value: 3});',
26+
'Object.defineProperty(this, "bar", {value: 1});\nObject.defineProperty(this, "baz", {value: 2});',
27+
'Object.defineProperty(Foo.prototype, "bar", {value: 1});\nObject.defineProperty(Foo.prototype, "baz", {value: 2});',
28+
'Object.defineProperty(foo, "default", {value: 1});\nObject.defineProperty(foo, "not-valid-key", {value: 2});',
29+
'Object.defineProperty(foo, 1, {value: "one"});\nObject.defineProperty(foo, 2, {value: "two"});',
30+
'Object.defineProperty(foo, `bar`, {value: 1});\nObject.defineProperty(foo, key, {value: 2});',
31+
'Object.defineProperty(foo, "__proto__", {value: 1});\nObject.defineProperty(foo, "bar", {value: 2});',
32+
`if (condition) {
33+
Object.defineProperty(foo, 'bar', {
34+
value: 1,
35+
writable: true,
36+
});
37+
Object.defineProperty(foo, 'baz', {
38+
value: 2,
39+
writable: true,
40+
});
41+
}`,
42+
'Object.defineProperty(foo, "bar", {value: 1});\nObject.defineProperty(foo, "bar", {value: 2});',
43+
'Object.defineProperty(foo, 1, {value: "number"});\nObject.defineProperty(foo, "1", {value: "string"});',
44+
'const key = "bar";\nObject.defineProperty(foo, key, {value: 1});\nObject.defineProperty(foo, "bar", {value: 2});',
45+
'Object.defineProperty(foo, Symbol.iterator, {value: 1});\nObject.defineProperty(foo, Symbol.iterator, {value: 2});',
46+
'Object.defineProperty(foo, key, {value: 1});\nObject.defineProperty(foo, key, {value: 2});',
47+
'Object.defineProperty(foo, keys.name, {value: 1});\nObject.defineProperty(foo, keys.name, {value: 2});',
48+
{
49+
code: 'Object.defineProperty(foo as Foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
50+
languageOptions: {parser: parsers.typescript},
51+
},
52+
{
53+
code: 'Object.defineProperty(foo!, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
54+
languageOptions: {parser: parsers.typescript},
55+
},
56+
{
57+
code: 'Object.defineProperty(<Foo>foo, "bar", {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
58+
languageOptions: {parser: parsers.typescript},
59+
},
60+
'Object.defineProperty(foo, "bar", {value: 1});\n// comment\nObject.defineProperty(foo, "baz", {value: 2});',
61+
'Object.defineProperty(foo, "bar", /* comment */ {value: 1});\nObject.defineProperty(foo, "baz", {value: 2});',
62+
],
63+
});

0 commit comments

Comments
 (0)