-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
[DO NOT MERGE] Auto fix prefer-destructuring-in-parameters
on codebase
#1046
Changes from all commits
b9f5126
c1f16d5
ff01e56
d36b112
db2d17a
c01b302
1a029a5
7801f77
72947d5
86a33f4
e778758
8a4bc33
e87ae3d
a8ed118
22cc069
4882743
2972162
d8f8ab5
c5c62b1
14cad62
11b6d23
6921209
66ad0af
76fcf7f
3b879a0
51ab3bb
24f7d0b
4cd6d05
43669b0
9887f56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# Prefer destructuring in parameters over accessing properties | ||
|
||
Makes your code shorter and nicer. | ||
|
||
This rule is fixable. | ||
|
||
## Fail | ||
|
||
```js | ||
const getObjectProperty = object => object.property; | ||
``` | ||
|
||
```js | ||
const removeEmptyValues = object => Object.fromEntries( | ||
Object.entries(object).filter(keyValuePair => Boolean(keyValuePair[1])) | ||
); | ||
``` | ||
|
||
## Pass | ||
|
||
```js | ||
const getFoo = ({property}) => property; | ||
``` | ||
|
||
```js | ||
const removeEmptyValues = object => Object.fromEntries( | ||
Object.entries(object).filter(([, value]) => Boolean(value)) | ||
); | ||
``` | ||
|
||
```js | ||
// Used property and index together | ||
function foo(array) { | ||
if (array.length > 0) { | ||
return bar(array[0]); | ||
} | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,8 +28,8 @@ function checkReferences(scope, parent, scopeManager) { | |
return isSameScope(parent, resolved.scope); | ||
}); | ||
|
||
const hitDefinitions = definitions => definitions.some(definition => { | ||
const scope = scopeManager.acquire(definition.node); | ||
const hitDefinitions = definitions => definitions.some(({node}) => { | ||
const scope = scopeManager.acquire(node); | ||
return isSameScope(parent, scope); | ||
}); | ||
|
||
|
@@ -70,10 +70,10 @@ function checkReferences(scope, parent, scopeManager) { | |
return getReferences(scope) | ||
.map(({resolved}) => resolved) | ||
.filter(Boolean) | ||
.some(variable => | ||
hitReference(variable.references) || | ||
hitDefinitions(variable.defs) || | ||
hitIdentifier(variable.identifiers) | ||
.some(({references, defs, identifiers}) => | ||
hitReference(references) || | ||
hitDefinitions(defs) || | ||
hitIdentifier(identifiers) | ||
); | ||
} | ||
|
||
|
@@ -90,18 +90,18 @@ const reactHooks = new Set([ | |
'useLayoutEffect', | ||
'useDebugValue' | ||
]); | ||
const isReactHook = scope => | ||
scope.block && | ||
scope.block.parent && | ||
scope.block.parent.callee && | ||
scope.block.parent.callee.type === 'Identifier' && | ||
reactHooks.has(scope.block.parent.callee.name); | ||
|
||
const isArrowFunctionWithThis = scope => | ||
scope.type === 'function' && | ||
scope.block && | ||
scope.block.type === 'ArrowFunctionExpression' && | ||
(scope.thisFound || scope.childScopes.some(scope => isArrowFunctionWithThis(scope))); | ||
const isReactHook = ({block}) => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's no longer possible to infer from the function what type it supports (that it supports a scope... |
||
block && | ||
block.parent && | ||
block.parent.callee && | ||
block.parent.callee.type === 'Identifier' && | ||
reactHooks.has(block.parent.callee.name); | ||
|
||
const isArrowFunctionWithThis = ({type, block, thisFound, childScopes}) => | ||
type === 'function' && | ||
block && | ||
block.type === 'ArrowFunctionExpression' && | ||
(thisFound || childScopes.some(scope => isArrowFunctionWithThis(scope))); | ||
Comment on lines
+100
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the previous version was more readable, and not much longer. |
||
|
||
const iifeFunctionTypes = new Set([ | ||
'FunctionExpression', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with destructuring is that you lose some readability information. It's no longer clear when you read this that
key
is the key of a property...