-
Notifications
You must be signed in to change notification settings - Fork 149
Move rules settings to ESLint shared config: refactor no-promise-in-fire-event #266
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
Changes from all commits
dee556f
b60b423
5cf1d68
45b8d67
84cddcb
bfb8220
74b87e3
3f1b5e6
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 |
---|---|---|
|
@@ -83,12 +83,6 @@ export function isBlockStatement( | |
return node?.type === AST_NODE_TYPES.BlockStatement; | ||
} | ||
|
||
export function isVariableDeclarator( | ||
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. Removed this one in favor of |
||
node: TSESTree.Node | ||
): node is TSESTree.VariableDeclarator { | ||
return node?.type === AST_NODE_TYPES.VariableDeclarator; | ||
} | ||
|
||
export function isObjectPattern( | ||
node: TSESTree.Node | ||
): node is TSESTree.ObjectPattern { | ||
|
@@ -191,14 +185,6 @@ export function isImportDeclaration( | |
return node?.type === AST_NODE_TYPES.ImportDeclaration; | ||
} | ||
|
||
export function isAwaited(node: TSESTree.Node): boolean { | ||
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. Not used anymore |
||
return ( | ||
ASTUtils.isAwaitExpression(node) || | ||
isArrowFunctionExpression(node) || | ||
isReturnStatement(node) | ||
); | ||
} | ||
|
||
export function hasChainedThen(node: TSESTree.Node): boolean { | ||
const parent = node.parent; | ||
|
||
|
@@ -211,11 +197,16 @@ export function hasChainedThen(node: TSESTree.Node): boolean { | |
return hasThenProperty(parent); | ||
} | ||
|
||
export function isPromiseIdentifier( | ||
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. Nice, I like this one 👍 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 got the inspiration for typing it from ts-eslint itself, it's a really cool way of checking specific node types with a particular attribute. |
||
node: TSESTree.Node | ||
): node is TSESTree.Identifier & { name: 'Promise' } { | ||
return ASTUtils.isIdentifier(node) && node.name === 'Promise'; | ||
} | ||
|
||
export function isPromiseAll(node: TSESTree.CallExpression): boolean { | ||
return ( | ||
isMemberExpression(node.callee) && | ||
ASTUtils.isIdentifier(node.callee.object) && | ||
node.callee.object.name === 'Promise' && | ||
isPromiseIdentifier(node.callee.object) && | ||
ASTUtils.isIdentifier(node.callee.property) && | ||
node.callee.property.name === 'all' | ||
); | ||
|
@@ -224,8 +215,7 @@ export function isPromiseAll(node: TSESTree.CallExpression): boolean { | |
export function isPromiseAllSettled(node: TSESTree.CallExpression): boolean { | ||
return ( | ||
isMemberExpression(node.callee) && | ||
ASTUtils.isIdentifier(node.callee.object) && | ||
node.callee.object.name === 'Promise' && | ||
isPromiseIdentifier(node.callee.object) && | ||
ASTUtils.isIdentifier(node.callee.property) && | ||
node.callee.property.name === 'allSettled' | ||
); | ||
|
@@ -304,7 +294,7 @@ export function getVariableReferences( | |
node: TSESTree.Node | ||
): TSESLint.Scope.Reference[] { | ||
return ( | ||
(isVariableDeclarator(node) && | ||
(ASTUtils.isVariableDeclarator(node) && | ||
context.getDeclaredVariables(node)[0]?.references?.slice(1)) || | ||
[] | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,18 @@ | ||
import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; | ||
import { createTestingLibraryRule } from '../create-testing-library-rule'; | ||
import { | ||
TSESTree, | ||
ESLintUtils, | ||
ASTUtils, | ||
} from '@typescript-eslint/experimental-utils'; | ||
import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils'; | ||
import { | ||
isNewExpression, | ||
isImportSpecifier, | ||
findClosestCallExpressionNode, | ||
getIdentifierNode, | ||
isCallExpression, | ||
isNewExpression, | ||
isPromiseIdentifier, | ||
} from '../node-utils'; | ||
|
||
export const RULE_NAME = 'no-promise-in-fire-event'; | ||
export type MessageIds = 'noPromiseInFireEvent'; | ||
type Options = []; | ||
|
||
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({ | ||
export default createTestingLibraryRule<Options, MessageIds>({ | ||
name: RULE_NAME, | ||
meta: { | ||
type: 'problem', | ||
|
@@ -28,49 +26,74 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({ | |
noPromiseInFireEvent: | ||
"A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element", | ||
}, | ||
fixable: 'code', | ||
fixable: null, | ||
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. This was set incorrectly. |
||
schema: [], | ||
}, | ||
defaultOptions: [], | ||
|
||
create(context) { | ||
return { | ||
'ImportDeclaration[source.value=/testing-library/]'( | ||
node: TSESTree.ImportDeclaration | ||
) { | ||
const fireEventImportNode = node.specifiers.find( | ||
(specifier) => | ||
isImportSpecifier(specifier) && | ||
specifier.imported && | ||
'fireEvent' === specifier.imported.name | ||
) as TSESTree.ImportSpecifier; | ||
create(context, _, helpers) { | ||
function checkSuspiciousNode( | ||
node: TSESTree.Node, | ||
originalNode?: TSESTree.Node | ||
): void { | ||
if (ASTUtils.isAwaitExpression(node)) { | ||
return; | ||
} | ||
|
||
const { references } = context.getDeclaredVariables( | ||
fireEventImportNode | ||
)[0]; | ||
if (isNewExpression(node)) { | ||
if (isPromiseIdentifier(node.callee)) { | ||
return context.report({ | ||
node: originalNode ?? node, | ||
messageId: 'noPromiseInFireEvent', | ||
}); | ||
} | ||
} | ||
|
||
for (const reference of references) { | ||
const referenceNode = reference.identifier; | ||
const callExpression = referenceNode.parent | ||
.parent as TSESTree.CallExpression; | ||
const [element] = callExpression.arguments as TSESTree.Node[]; | ||
if (isCallExpression(element) || isNewExpression(element)) { | ||
const methodName = ASTUtils.isIdentifier(element.callee) | ||
? element.callee.name | ||
: ((element.callee as TSESTree.MemberExpression) | ||
.property as TSESTree.Identifier).name; | ||
if (isCallExpression(node)) { | ||
const domElementIdentifier = getIdentifierNode(node); | ||
|
||
if ( | ||
helpers.isAsyncQuery(domElementIdentifier) || | ||
isPromiseIdentifier(domElementIdentifier) | ||
) { | ||
return context.report({ | ||
node: originalNode ?? node, | ||
messageId: 'noPromiseInFireEvent', | ||
}); | ||
} | ||
} | ||
|
||
if (ASTUtils.isIdentifier(node)) { | ||
const nodeVariable = ASTUtils.findVariable( | ||
context.getScope(), | ||
node.name | ||
); | ||
if (!nodeVariable || !nodeVariable.defs) { | ||
return; | ||
} | ||
|
||
if ( | ||
ASYNC_QUERIES_VARIANTS.some((q) => methodName.startsWith(q)) || | ||
methodName === 'Promise' | ||
) { | ||
context.report({ | ||
node: element, | ||
messageId: 'noPromiseInFireEvent', | ||
}); | ||
} | ||
} | ||
for (const definition of nodeVariable.defs) { | ||
const variableDeclarator = definition.node as TSESTree.VariableDeclarator; | ||
checkSuspiciousNode(variableDeclarator.init, node); | ||
} | ||
} | ||
} | ||
|
||
return { | ||
'CallExpression Identifier'(node: TSESTree.Identifier) { | ||
if (!helpers.isFireEventMethod(node)) { | ||
return; | ||
} | ||
|
||
const closestCallExpression = findClosestCallExpressionNode(node, true); | ||
|
||
if (!closestCallExpression) { | ||
return; | ||
} | ||
|
||
const domElementArgument = closestCallExpression.arguments[0]; | ||
|
||
checkSuspiciousNode(domElementArgument); | ||
}, | ||
}; | ||
}, | ||
|
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.
This one is reported properly now! It has been moved to "incorrect" section.