-
-
Notifications
You must be signed in to change notification settings - Fork 409
feat: add new rule no-array-fill-with-reference-type without fix solution #2661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
legend80s
wants to merge
54
commits into
sindresorhus:main
Choose a base branch
from
legend80s:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 39 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
87f000c
docs: add tips of how to install for new developers
a8f6d3f
fix: error when npm run create-rule > Cannot find package 'lodash-es'…
ea97dd1
feat: add new rule no-array-fill-with-reference-type without fix solu…
35057ee
feat(no-array-fill-with-reference-type): only check fill on builtin A…
aded9b0
test(no-array-fill-with-reference-type): add more valid cases
57f4d74
docs(no-array-fill-with-reference-type): remove comments
85bb03a
docs(no-array-fill-with-reference-type): gen code by npm run fix:esli…
3977224
docs(no-array-fill-with-reference-type): brief the desc and recover r…
8243f14
docs: no file configs/recommended.js
3635dd3
docs(no-array-fill-with-reference-type): gen code by npm run fix:esli…
fcd690e
docs: add tips of how to install for new developers
legend80s 51af337
fix: error when npm run create-rule > Cannot find package 'lodash-es'…
legend80s 09ca3b5
feat: add new rule no-array-fill-with-reference-type without fix solu…
legend80s b20ddf2
feat(no-array-fill-with-reference-type): only check fill on builtin A…
legend80s c70d17f
test(no-array-fill-with-reference-type): add more valid cases
legend80s 7c7fa38
docs(no-array-fill-with-reference-type): remove comments
legend80s d3a4179
docs(no-array-fill-with-reference-type): gen code by npm run fix:esli…
legend80s 681827d
docs(no-array-fill-with-reference-type): brief the desc and recover r…
legend80s a6ca789
docs: no file configs/recommended.js
legend80s 2091479
docs(no-array-fill-with-reference-type): gen code by npm run fix:esli…
legend80s d80842c
Merge branch 'main' of github.com:legend80s/eslint-plugin-unicorn
legend80s 90b768b
feat(no-array-fill-with-reference-type): no fixable
legend80s a0841ad
docs(no-array-fill-with-reference-type): no fixable
legend80s ab37dcb
feat(no-array-fill-with-reference-type): check Array.from().fill
legend80s 6b14629
docs(no-array-fill-with-reference-type): correct docs
legend80s b6bf56c
docs(no-array-fill-with-reference-type): correct docs by npm run fix:…
legend80s 928e51a
test: disable no-array-fill-with-reference-type because it is safe du…
legend80s 36b04e3
feat: hide unknown type in error message
legend80s 4f4cf65
test: hide unknown type in error message
legend80s 3813e76
test: add a case of hiding unknown type in error message
legend80s 2a7ffee
feat: report precisely on node.arguments[0] only.
legend80s d5f2398
feat(no-array-fill-with-reference-type): report precisely on node.arg…
legend80s 0d5c73c
chore(no-array-fill-with-reference-type): merge with remote
legend80s 639c593
test(no-array-fill-with-reference-type): add more case to improve cov…
legend80s 9ab3fb0
test(no-array-fill-with-reference-type): fix ci failed. code gen by -u
legend80s 4f3849f
feat: add option to not check for function expressions by default bec…
legend80s ef37d48
docs: add option to not check for function
legend80s 2d499c0
Merge branch 'main' into main
legend80s bac577b
style: add new line to fix lint errors
legend80s 59b4ae8
feat(no-array-fill-with-reference-type): ignore regexp literal and ne…
legend80s 04a2a55
docs(no-array-fill-with-reference-type): ignore regexp literal and ne…
legend80s 9b80603
docs(no-array-fill-with-reference-type): more precise suggestion tips
legend80s 00eb9a3
feat(no-array-fill-with-reference-type): try the best to retrie type …
legend80s b64f7cf
test(no-array-fill-with-reference-type): Class should be class to avo…
legend80s acaf4e7
feat(no-array-fill-with-reference-type): only check const
legend80s a7b36db
feat(no-array-fill-with-reference-type): report Array.from(arrayLike,…
legend80s 2bc40f8
chore(no-array-fill-with-reference-type): fix npm run lint error
legend80s bc9f7f5
refactor(no-array-fill-with-reference-type): rename options
legend80s b87ec58
refactor(no-array-fill-with-reference-type): use existing util isMemb…
legend80s 05893d1
fix(no-array-fill-with-reference-type): fix integration failed when c…
legend80s 88b4528
test(no-array-fill-with-reference-type): fix integration test failed …
legend80s ed1f6ae
test(no-array-fill-with-reference-type): fix integration test failed.…
legend80s 5e9494e
fix: correct return type for isMemberExpression function & add type p…
legend80s 4bddea9
feat(no-array-fill-with-reference-type): check is member expression i…
legend80s File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
# Disallows using `Array.fill()` or `Array.from().fill()` with **reference types** to prevent unintended shared references across array elements | ||
|
||
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). | ||
|
||
<!-- end auto-generated rule header --> | ||
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` --> | ||
|
||
Disallows using `Array.fill()` or `Array.from().fill()` with **reference types** (objects, arrays, functions, Maps, Sets, RegExp literals, etc.) to prevent unintended shared references across array elements. Encourages `Array.from()` or explicit iteration for creating independent instances. | ||
|
||
**Reason**: | ||
`Array(len).fill(value)` fills all elements with the **same reference** if `value` is non-primitive (e.g., `fill([])`), leading to bugs when one element’s mutations affect others. | ||
|
||
**Key Features**: | ||
|
||
- Catches **all reference types**: Objects, Arrays, Functions, `new` expressions, RegExp literals, and variables referencing them. | ||
- **Clear error messages**: Identifies the reference type (e.g., `Object`, `RegExp`, `variable (name)`). | ||
|
||
**Note**: Primitive types (`number`, `string`, `boolean`, `null`, `undefined`, `symbol`, `bigint`) are always allowed. | ||
|
||
## Examples | ||
|
||
```js | ||
// ❌ Object | ||
new Array(3).fill({}); | ||
Array(3).fill({}); | ||
Array.from({ length: 3 }).fill({}); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => ({})); | ||
``` | ||
|
||
```js | ||
// ❌ Array | ||
new Array(3).fill([]); | ||
Array(3).fill([]); | ||
Array.from({ length: 3 }).fill([]); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => []); | ||
``` | ||
|
||
```js | ||
// ❌ Map | ||
new Array(3).fill(new Map()); | ||
Array(3).fill(new Map()); | ||
Array.from({ length: 3 }).fill(new Map()); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => new Map()); | ||
``` | ||
|
||
```js | ||
// ❌ Date | ||
new Array(3).fill(new Date()); | ||
Array(3).fill(new Date()); | ||
Array.from({ length: 3 }).fill(new Date()); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => new Date()); | ||
``` | ||
|
||
```js | ||
// ❌ Class | ||
class BarClass {}; | ||
new Array(3).fill(new BarClass()); | ||
Array(3).fill(new BarClass()); | ||
Array.from({ length: 3 }).fill(new BarClass()); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => new BarClass()); | ||
``` | ||
|
||
```js | ||
// ❌ Function | ||
new Array(3).fill(function () {}) | ||
Array(3).fill(function () {}) | ||
Array.from({ length: 3 }).fill(function () {}); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => function () {}); | ||
``` | ||
|
||
```js | ||
// ❌ RegExp literal | ||
new Array(3).fill(/pattern/); | ||
Array(3).fill(/pattern/); | ||
Array.from({ length: 3 }).fill(/pattern/); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => /pattern/); | ||
``` | ||
|
||
```js | ||
const box = [] | ||
|
||
// ❌ Shared reference | ||
new Array(3).fill(box); | ||
Array(3).fill(box); | ||
Array.from({ length: 3 }).fill(box); | ||
|
||
// ✅ | ||
Array.from({ length: 3 }, () => []); | ||
``` | ||
|
||
## Options | ||
|
||
### canFillWithFunction | ||
|
||
Type: `boolean`\ | ||
Default: `true` | ||
|
||
Should check function when filling an array? | ||
|
||
This would pass by default: | ||
|
||
```js | ||
new Array(3).fill(function () {}) | ||
``` | ||
|
||
```js | ||
"unicorn/catch-error-name": [ | ||
"error", | ||
{ | ||
"canFillWithFunction": false | ||
} | ||
] | ||
``` | ||
|
||
with `canFillWithFunction: false`, this would fail: | ||
|
||
```js | ||
new Array(3).fill(function () {}) | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
import {isFunction} from './ast/index.js'; | ||
|
||
// @ts-check | ||
const MESSAGE_ID_ERROR = 'no-array-fill-with-reference-type/error'; | ||
const messages = { | ||
[MESSAGE_ID_ERROR]: 'Avoid using `{{actual}}` with reference type{{type}}. Use `Array.from({ ... }, () => { ... })` instead to ensure independent instances.', | ||
}; | ||
|
||
const debugging = false; | ||
const log = (...arguments_) => debugging && console.log(...arguments_); | ||
|
||
/** @param {import('eslint').Rule.RuleContext} context */ | ||
const create = context => ({ | ||
CallExpression(node) { | ||
const isArrayFill = node.callee.type === 'MemberExpression' | ||
&& ((node.callee.object.callee?.name === 'Array') || (context.sourceCode.getText(node.callee.object.callee) === 'Array.from')) | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&& node.callee.property.name === 'fill' | ||
&& node.arguments.length > 0; | ||
|
||
log('isArrayFill:', isArrayFill); | ||
|
||
if (!isArrayFill) { | ||
return; | ||
} | ||
|
||
const fillArgument = node.arguments[0]; | ||
log('fillArgument:', fillArgument); | ||
|
||
if (!isReferenceType(fillArgument, context)) { | ||
return; | ||
} | ||
|
||
const actual = context.sourceCode.getText(node.callee.object.callee) === 'Array.from' ? 'Array.from().fill()' : 'Array.fill()'; | ||
const type = getType(fillArgument); | ||
|
||
return { | ||
node: fillArgument, | ||
messageId: MESSAGE_ID_ERROR, | ||
data: { | ||
actual, | ||
type: type ? ` (${type})` : '', | ||
}, | ||
}; | ||
}, | ||
}); | ||
|
||
/** | ||
|
||
@param {*} fillArgument | ||
@returns {string} | ||
*/ | ||
function getType(fillArgument) { | ||
let type = ''; | ||
|
||
switch (fillArgument.type) { | ||
case 'ObjectExpression': { | ||
type = 'Object'; | ||
break; | ||
} | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
case 'ArrayExpression': { | ||
type = 'Array'; | ||
break; | ||
} | ||
|
||
case 'NewExpression': { | ||
type = `new ${fillArgument.callee.name}()`; | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
break; | ||
} | ||
|
||
case 'FunctionExpression': | ||
case 'ArrowFunctionExpression': { | ||
type = 'Function'; | ||
break; | ||
} | ||
|
||
default: { | ||
if (fillArgument.type === 'Literal' && fillArgument.regex) { | ||
type = 'RegExp'; | ||
} else if (fillArgument.type === 'Identifier') { | ||
type = `variable (${fillArgument.name})`; | ||
} | ||
} | ||
} | ||
|
||
return type; | ||
} | ||
|
||
/** | ||
@param {*} node | ||
@param {import('eslint').Rule.RuleContext} context | ||
@returns | ||
*/ | ||
function isReferenceType(node, context) { | ||
if (!node) { | ||
return false; | ||
} | ||
|
||
// For null, number, string, boolean. | ||
if (node.type === 'Literal') { | ||
// Exclude regular expression literals (e.g., `/pattern/`, which are objects despite being literals). | ||
return node.regex !== undefined; | ||
} | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// For template literals. | ||
if (node.type === 'TemplateLiteral') { | ||
return false; | ||
} | ||
|
||
// For variable identifiers (recursively check its declaration). | ||
if (node.type === 'Identifier') { | ||
const {variables} = context.sourceCode.getScope(node); | ||
const variable = variables.find(v => v.name === node.name); | ||
const definitionNode = variable?.defs[0].node; | ||
|
||
log('variables:', variables); | ||
log('variable:', variable); | ||
log('variable.defs[0].node:', definitionNode); | ||
|
||
if (!variable || !definitionNode) { | ||
return false; | ||
} | ||
|
||
// Check `const foo = []; Array(3).fill(foo);` | ||
if (definitionNode.type === 'VariableDeclarator') { | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return isReferenceType(definitionNode.init, context); | ||
} | ||
|
||
return isReferenceType(definitionNode, context); | ||
} | ||
|
||
// Symbol (such as `Symbol('name')`) | ||
if (node.type === 'CallExpression' && node.callee.name === 'Symbol') { | ||
const {variables} = context.sourceCode.getScope(node); | ||
|
||
log('variables 2:', variables); | ||
if (!variables || variables.length === 0) { | ||
// Variable declaration not found; it might be a global variable. | ||
return false; | ||
} | ||
} | ||
legend80s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const options = { | ||
// Not check for function expressions by default because it is rare to fill an array with a function and add properties to it. | ||
canFillWithFunction: true, | ||
...context.options[0], | ||
}; | ||
|
||
if (options.canFillWithFunction && isFunction(node)) { | ||
return false; | ||
} | ||
|
||
// Other cases: objects, arrays, new expressions, regular expressions, etc. | ||
return true; | ||
} | ||
|
||
const schema = [ | ||
{ | ||
type: 'object', | ||
additionalProperties: false, | ||
properties: { | ||
canFillWithFunction: { | ||
type: 'boolean', | ||
}, | ||
}, | ||
}, | ||
]; | ||
|
||
/** @type {import('eslint').Rule.RuleModule} */ | ||
const config = { | ||
create, | ||
meta: { | ||
type: 'problem', | ||
docs: { | ||
description: 'Disallows using `Array.fill()` or `Array.from().fill()` with **reference types** to prevent unintended shared references across array elements.', | ||
recommended: true, | ||
}, | ||
schema, | ||
defaultOptions: [{}], | ||
messages, | ||
}, | ||
}; | ||
|
||
export default config; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.