From 2a8ac30afdb8b730342d15388d9a9afe97796307 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 20 Jun 2020 23:11:30 +1200 Subject: [PATCH] chore: minor tidy up of code (#603) * test: ensure `cwd` is restored after each test * chore: add `eslint-plugin-eslint-config` * chore: replace `ban-ts-ignore` with `ban-ts-comment` rule * chore: enable `padding-line-between-statements` rule * chore: remove some unneeded conditional checks * chore: use correct jsdoc syntax for optional params --- .eslintignore | 1 + .eslintrc.js | 16 +++++++++++++++- package.json | 1 + src/__tests__/rules.test.ts | 2 ++ src/rules/expect-expect.ts | 2 ++ src/rules/lowercase-name.ts | 1 + src/rules/no-alias-methods.ts | 1 + src/rules/no-duplicate-hooks.ts | 2 ++ src/rules/no-focused-tests.ts | 4 +++- src/rules/no-identical-title.ts | 3 +++ src/rules/no-jasmine-globals.ts | 5 +++++ src/rules/no-large-snapshots.ts | 2 +- src/rules/no-mocks-import.ts | 2 +- src/rules/no-standalone-expect.ts | 7 +++++++ src/rules/no-test-return-statement.ts | 4 ++++ src/rules/no-truthy-falsy.ts | 1 + src/rules/prefer-expect-assertions.ts | 6 ++---- src/rules/prefer-hooks-on-top.ts | 1 + src/rules/prefer-to-contain.ts | 4 +++- src/rules/prefer-todo.ts | 1 + src/rules/require-top-level-describe.ts | 4 ++++ src/rules/utils.ts | 18 ++++++++++-------- src/rules/valid-expect-in-promise.ts | 2 ++ src/rules/valid-expect.ts | 2 +- yarn.lock | 13 +++++++++++++ 25 files changed, 87 insertions(+), 18 deletions(-) diff --git a/.eslintignore b/.eslintignore index 64be67e70..30e85ed0b 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,2 +1,3 @@ coverage/ lib/ +!.eslintrc.js diff --git a/.eslintrc.js b/.eslintrc.js index f59291d44..1d51c96d1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -5,6 +5,7 @@ const globals = require('./src/globals.json'); module.exports = { parser: require.resolve('@typescript-eslint/parser'), extends: [ + 'plugin:eslint-config/rc', 'plugin:eslint-plugin/recommended', 'plugin:eslint-comments/recommended', 'plugin:node/recommended', @@ -13,6 +14,7 @@ module.exports = { 'prettier/@typescript-eslint', ], plugins: [ + 'eslint-config', 'eslint-plugin', 'eslint-comments', 'node', @@ -30,7 +32,7 @@ module.exports = { rules: { '@typescript-eslint/array-type': ['error', { default: 'array-simple' }], '@typescript-eslint/no-require-imports': 'error', - '@typescript-eslint/ban-ts-ignore': 'warn', + '@typescript-eslint/ban-ts-comment': 'warn', '@typescript-eslint/ban-types': 'error', '@typescript-eslint/no-unused-vars': 'error', 'eslint-comments/no-unused-disable': 'error', @@ -58,6 +60,18 @@ module.exports = { 'error', { alphabetize: { order: 'asc' }, 'newlines-between': 'never' }, ], + 'padding-line-between-statements': [ + 'error', + { blankLine: 'always', prev: '*', next: 'return' }, + { blankLine: 'always', prev: ['const', 'let', 'var'], next: '*' }, + { + blankLine: 'any', + prev: ['const', 'let', 'var'], + next: ['const', 'let', 'var'], + }, + { blankLine: 'always', prev: 'directive', next: '*' }, + { blankLine: 'any', prev: 'directive', next: 'directive' }, + ], }, overrides: [ { diff --git a/package.json b/package.json index 10897c375..2d1e4ffb2 100644 --- a/package.json +++ b/package.json @@ -109,6 +109,7 @@ "eslint": "^5.1.0 || ^6.0.0", "eslint-config-prettier": "^6.5.0", "eslint-plugin-eslint-comments": "^3.1.2", + "eslint-plugin-eslint-config": "^1.0.2", "eslint-plugin-eslint-plugin": "^2.0.0", "eslint-plugin-import": "^2.20.2", "eslint-plugin-node": "^11.0.0", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index ec4461e4a..98bdb5a1b 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -36,6 +36,7 @@ describe('rules', () => { it('should have the correct amount of rules', () => { const { length } = ruleNames; + if (length !== numberOfRules) { throw new Error( `There should be exactly ${numberOfRules} rules, but there are ${length}. If you've added a new rule, please update this number.`, @@ -65,6 +66,7 @@ describe('rules', () => { allConfigRules.forEach(rule => { const ruleNamePrefix = 'jest/'; const ruleName = rule.slice(ruleNamePrefix.length); + expect(rule.startsWith(ruleNamePrefix)).toBe(true); expect(ruleNames).toContain(ruleName); // eslint-disable-next-line @typescript-eslint/no-require-imports diff --git a/src/rules/expect-expect.ts b/src/rules/expect-expect.ts index 7657f2a11..a2691e0a2 100644 --- a/src/rules/expect-expect.ts +++ b/src/rules/expect-expect.ts @@ -31,6 +31,7 @@ function matchesAssertFunctionName( .split('.') .map(x => { if (x === '**') return '[a-z\\.]*'; + return x.replace(/\*/gu, '[a-z]*'); }) .join('\\.')}(\\.|$)`, @@ -97,6 +98,7 @@ export default createRule< return { CallExpression(node) { const name = getNodeName(node.callee); + if (name === TestCaseName.it || name === TestCaseName.test) { unchecked.push(node); } else if ( diff --git a/src/rules/lowercase-name.ts b/src/rules/lowercase-name.ts index f89aa1d7d..1419db04b 100644 --- a/src/rules/lowercase-name.ts +++ b/src/rules/lowercase-name.ts @@ -43,6 +43,7 @@ const jestFunctionName = ( allowedPrefixes: readonly string[], ) => { const description = getStringValue(node.arguments[0]); + if (allowedPrefixes.some(name => description.startsWith(name))) { return null; } diff --git a/src/rules/no-alias-methods.ts b/src/rules/no-alias-methods.ts index 8b726c814..8c6eb15d5 100644 --- a/src/rules/no-alias-methods.ts +++ b/src/rules/no-alias-methods.ts @@ -45,6 +45,7 @@ export default createRule({ } const alias = matcher.name; + if (alias in methodNames) { const canonical = methodNames[alias]; diff --git a/src/rules/no-duplicate-hooks.ts b/src/rules/no-duplicate-hooks.ts index 10a688170..d06da84b6 100644 --- a/src/rules/no-duplicate-hooks.ts +++ b/src/rules/no-duplicate-hooks.ts @@ -24,6 +24,7 @@ export default createRule({ defaultOptions: [], create(context) { const hookContexts = [newHookContext()]; + return { CallExpression(node) { if (isDescribe(node)) { @@ -32,6 +33,7 @@ export default createRule({ if (isHook(node)) { const currentLayer = hookContexts[hookContexts.length - 1]; + currentLayer[node.callee.name] += 1; if (currentLayer[node.callee.name] > 1) { context.report({ diff --git a/src/rules/no-focused-tests.ts b/src/rules/no-focused-tests.ts index 834a35938..e1439e7db 100644 --- a/src/rules/no-focused-tests.ts +++ b/src/rules/no-focused-tests.ts @@ -24,7 +24,6 @@ interface ConcurrentExpression extends TSESTree.MemberExpressionComputedName { const isConcurrentExpression = ( expression: TSESTree.MemberExpression, ): expression is ConcurrentExpression => - expression.type === AST_NODE_TYPES.MemberExpression && isSupportedAccessor(expression.property, TestCaseProperty.concurrent) && !!expression.parent && expression.parent.type === AST_NODE_TYPES.MemberExpression; @@ -72,6 +71,7 @@ export default createRule({ isCallToFocusedTestFunction(callee.object) ) { context.report({ messageId: 'focusedTest', node: callee.object }); + return; } @@ -83,11 +83,13 @@ export default createRule({ messageId: 'focusedTest', node: callee.object.property, }); + return; } if (isCallToTestOnlyFunction(callee)) { context.report({ messageId: 'focusedTest', node: callee.property }); + return; } } diff --git a/src/rules/no-identical-title.ts b/src/rules/no-identical-title.ts index 7d330cf72..374219408 100644 --- a/src/rules/no-identical-title.ts +++ b/src/rules/no-identical-title.ts @@ -36,13 +36,16 @@ export default createRule({ defaultOptions: [], create(context) { const contexts = [newDescribeContext()]; + return { CallExpression(node) { const currentLayer = contexts[contexts.length - 1]; + if (isDescribe(node)) { contexts.push(newDescribeContext()); } const [argument] = node.arguments; + if (!argument || !isStringNode(argument)) { return; } diff --git a/src/rules/no-jasmine-globals.ts b/src/rules/no-jasmine-globals.ts index fdb1e4d39..a742f9ac8 100644 --- a/src/rules/no-jasmine-globals.ts +++ b/src/rules/no-jasmine-globals.ts @@ -67,6 +67,7 @@ export default createRule({ context.report({ node, messageId: 'illegalPending' }); break; } + return; } @@ -92,6 +93,7 @@ export default createRule({ replacement: `expect.${functionName}`, }, }); + return; } @@ -104,6 +106,7 @@ export default createRule({ replacement: 'expect.extend', }, }); + return; } @@ -116,6 +119,7 @@ export default createRule({ replacement: 'jest.fn', }, }); + return; } @@ -141,6 +145,7 @@ export default createRule({ node, messageId: 'illegalJasmine', }); + return; } } diff --git a/src/rules/no-large-snapshots.ts b/src/rules/no-large-snapshots.ts index d4f364b1f..ac4309b42 100644 --- a/src/rules/no-large-snapshots.ts +++ b/src/rules/no-large-snapshots.ts @@ -43,7 +43,6 @@ const reportOnViolation = ( let isWhitelisted = false; if ( - whitelistedSnapshots && node.type === AST_NODE_TYPES.ExpressionStatement && 'left' in node.expression && isExpectMember(node.expression.left) @@ -53,6 +52,7 @@ const reportOnViolation = ( if (whitelistedSnapshotsInFile) { const snapshotName = getAccessorValue(node.expression.left.property); + isWhitelisted = whitelistedSnapshotsInFile.some(name => { if (name instanceof RegExp) { return name.test(snapshotName); diff --git a/src/rules/no-mocks-import.ts b/src/rules/no-mocks-import.ts index a12b3fc48..53d973448 100644 --- a/src/rules/no-mocks-import.ts +++ b/src/rules/no-mocks-import.ts @@ -28,7 +28,7 @@ export default createRule({ create(context) { return { ImportDeclaration(node: TSESTree.ImportDeclaration) { - if (node.source && isMockImportLiteral(node.source)) { + if (isMockImportLiteral(node.source)) { context.report({ node, messageId: 'noManualImport' }); } }, diff --git a/src/rules/no-standalone-expect.ts b/src/rules/no-standalone-expect.ts index 42c5612ed..0f8f2fa20 100644 --- a/src/rules/no-standalone-expect.ts +++ b/src/rules/no-standalone-expect.ts @@ -29,6 +29,7 @@ const getBlockType = ( } if (isFunction(func) && func.parent) { const expr = func.parent; + // arrowfunction or function expr if (expr.type === AST_NODE_TYPES.VariableDeclarator) { return 'function'; @@ -38,6 +39,7 @@ const getBlockType = ( return DescribeAlias.describe; } } + return null; }; @@ -78,9 +80,11 @@ export default createRule({ CallExpression(node) { if (isExpectCall(node)) { const parent = callStack[callStack.length - 1]; + if (!parent || parent === DescribeAlias.describe) { context.report({ node, messageId: 'unexpectedExpect' }); } + return; } if (isTestCase(node)) { @@ -92,6 +96,7 @@ export default createRule({ }, 'CallExpression:exit'(node: TSESTree.CallExpression) { const top = callStack[callStack.length - 1]; + if ( (((isTestCase(node) && node.callee.type !== AST_NODE_TYPES.MemberExpression) || @@ -105,12 +110,14 @@ export default createRule({ }, BlockStatement(stmt) { const blockType = getBlockType(stmt); + if (blockType) { callStack.push(blockType); } }, 'BlockStatement:exit'(stmt: TSESTree.BlockStatement) { const blockType = getBlockType(stmt); + if (blockType && blockType === callStack[callStack.length - 1]) { callStack.pop(); } diff --git a/src/rules/no-test-return-statement.ts b/src/rules/no-test-return-statement.ts index 126d1af9a..94caac433 100644 --- a/src/rules/no-test-return-statement.ts +++ b/src/rules/no-test-return-statement.ts @@ -20,6 +20,7 @@ const getBody = (args: TSESTree.Expression[]) => { ) { return secondArg.body.body; } + return []; }; @@ -46,6 +47,7 @@ export default createRule({ const returnStmt = body.find( t => t.type === AST_NODE_TYPES.ReturnStatement, ); + if (!returnStmt) return; context.report({ messageId: 'noReturnValue', node: returnStmt }); @@ -55,11 +57,13 @@ export default createRule({ const testCallExpressions = getTestCallExpressionsFromDeclaredVariables( declaredVariables, ); + if (testCallExpressions.length === 0) return; const returnStmt = node.body.body.find( t => t.type === AST_NODE_TYPES.ReturnStatement, ); + if (!returnStmt) return; context.report({ messageId: 'noReturnValue', node: returnStmt }); diff --git a/src/rules/no-truthy-falsy.ts b/src/rules/no-truthy-falsy.ts index dfdd4998b..ef8b4daf6 100644 --- a/src/rules/no-truthy-falsy.ts +++ b/src/rules/no-truthy-falsy.ts @@ -26,6 +26,7 @@ export default createRule({ } const { matcher } = parseExpectCall(node); + if (!matcher || !['toBeTruthy', 'toBeFalsy'].includes(matcher.name)) { return; } diff --git a/src/rules/prefer-expect-assertions.ts b/src/rules/prefer-expect-assertions.ts index 70685b834..fcaf8b541 100644 --- a/src/rules/prefer-expect-assertions.ts +++ b/src/rules/prefer-expect-assertions.ts @@ -35,9 +35,6 @@ const isExpectAssertionsOrHasAssertionsCall = (expression: TSESTree.Node) => { ); }; -const getFunctionFirstLine = (functionBody: [TSESTree.ExpressionStatement]) => - functionBody[0] && functionBody[0].expression; - const isFirstLineExprStmt = ( functionBody: TSESTree.Statement[], ): functionBody is [TSESTree.ExpressionStatement] => @@ -81,7 +78,8 @@ export default createRule({ return; } - const testFuncFirstLine = getFunctionFirstLine(testFuncBody); + const testFuncFirstLine = testFuncBody[0].expression; + if (!isExpectAssertionsOrHasAssertionsCall(testFuncFirstLine)) { context.report({ messageId: 'haveExpectAssertions', node }); } diff --git a/src/rules/prefer-hooks-on-top.ts b/src/rules/prefer-hooks-on-top.ts index abb2e5e0e..f1507bb49 100644 --- a/src/rules/prefer-hooks-on-top.ts +++ b/src/rules/prefer-hooks-on-top.ts @@ -17,6 +17,7 @@ export default createRule({ defaultOptions: [], create(context) { const hooksContext = [false]; + return { CallExpression(node) { if (!isHook(node) && isTestCase(node)) { diff --git a/src/rules/prefer-to-contain.ts b/src/rules/prefer-to-contain.ts index 11ad3435c..b13439f81 100644 --- a/src/rules/prefer-to-contain.ts +++ b/src/rules/prefer-to-contain.ts @@ -153,6 +153,7 @@ const getCommonFixes = ( openParenthesis, ]; }; + // expect(array.includes()[not.]{toBe,toEqual}() export default createRule({ name: __filename, @@ -206,7 +207,7 @@ export default createRule({ fileName, ).map(target => fixer.remove(target)); - if (modifier && modifier.name === ModifierName.not) { + if (modifier) { return getNegationFixes( includesCall, modifier, @@ -232,6 +233,7 @@ export default createRule({ sourceCode.getText(containArg), ), ); + return fixArr; }, messageId: 'useToContain', diff --git a/src/rules/prefer-todo.ts b/src/rules/prefer-todo.ts index e08aa7792..ebf4eea08 100644 --- a/src/rules/prefer-todo.ts +++ b/src/rules/prefer-todo.ts @@ -40,6 +40,7 @@ function createTodoFixer( const testName = getNodeName(node.callee) .split('.') .shift(); + return fixer.replaceText(node.callee, `${testName}.todo`); } diff --git a/src/rules/require-top-level-describe.ts b/src/rules/require-top-level-describe.ts index 28d6f6fa4..dfee7f276 100644 --- a/src/rules/require-top-level-describe.ts +++ b/src/rules/require-top-level-describe.ts @@ -20,21 +20,25 @@ export default createRule({ defaultOptions: [], create(context) { let numberOfDescribeBlocks = 0; + return { CallExpression(node) { if (isDescribe(node)) { numberOfDescribeBlocks++; + return; } if (numberOfDescribeBlocks === 0) { if (isTestCase(node)) { context.report({ node, messageId: 'unexpectedTestCase' }); + return; } if (isHook(node)) { context.report({ node, messageId: 'unexpectedHook' }); + return; } } diff --git a/src/rules/utils.ts b/src/rules/utils.ts index fd4431383..3fac9f46f 100644 --- a/src/rules/utils.ts +++ b/src/rules/utils.ts @@ -11,6 +11,7 @@ const REPO_URL = 'https://github.com/jest-community/eslint-plugin-jest'; export const createRule = ESLintUtils.RuleCreator(name => { const ruleName = parsePath(name).name; + return `${REPO_URL}/blob/v${version}/docs/rules/${ruleName}.md`; }); @@ -64,11 +65,11 @@ interface StringLiteral * the `value` will be compared to that of the `StringLiteral`. * * @param {Node} node - * @param {V?} value + * @param {V} [value] * * @return {node is StringLiteral} * - * @template {V}. + * @template V */ const isStringLiteral = ( node: TSESTree.Node, @@ -92,7 +93,7 @@ interface TemplateLiteral * the `value` will be compared to that of the `TemplateLiteral`. * * @param {Node} node - * @param {V?} value + * @param {V} [value] * * @return {node is TemplateLiteral} * @@ -114,7 +115,7 @@ export type StringNode = * Checks if the given `node` is a {@link StringNode}. * * @param {Node} node - * @param {V?} specifics + * @param {V} [specifics] * * @return {node is StringNode} * @@ -187,8 +188,7 @@ export interface CallExpressionWithSingleArgument< */ export const hasOnlyOneArgument = ( call: TSESTree.CallExpression, -): call is CallExpressionWithSingleArgument => - call.arguments && call.arguments.length === 1; +): call is CallExpressionWithSingleArgument => call.arguments.length === 1; /** * An `Identifier` with a known `name` value - i.e `expect`. @@ -204,7 +204,7 @@ interface KnownIdentifier extends TSESTree.Identifier { * the `name` will be compared to that of the `identifier`. * * @param {Node} node - * @param {V?} name + * @param {V} [name] * * @return {node is KnownIdentifier} * @@ -233,7 +233,7 @@ const isIdentifier = ( * The property that holds the value is not always called `name`. * * @param {Node} node - * @param {V?} value + * @param {V} [value] * * @return {node is AccessorNode} * @@ -496,6 +496,7 @@ export const parseExpectCall = ( } const parsedMember = parseExpectMember(expect.parent); + if (!shouldBeParsedExpectModifier(parsedMember)) { expectation.matcher = reparseAsMatcher(parsedMember); @@ -738,6 +739,7 @@ export const scopeHasLocalReference = ( referenceName: string, ) => { const references = collectReferences(scope); + return ( // referenceName was found as a local variable or function declaration. references.locals.has(referenceName) || diff --git a/src/rules/valid-expect-in-promise.ts b/src/rules/valid-expect-in-promise.ts index 76440cd71..f15216f41 100644 --- a/src/rules/valid-expect-in-promise.ts +++ b/src/rules/valid-expect-in-promise.ts @@ -68,6 +68,7 @@ const isPromiseReturnedLater = ( testFunctionBody: TSESTree.Statement[], ) => { let promiseName; + if ( node.type === AST_NODE_TYPES.ExpressionStatement && node.expression.type === AST_NODE_TYPES.CallExpression && @@ -182,6 +183,7 @@ export default createRule({ return; } const testFunction = findTestFunction(node); + if (testFunction && !isHavingAsyncCallBackParam(testFunction)) { const { body } = testFunction; diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts index 45e5d7b39..7f658a8dc 100644 --- a/src/rules/valid-expect.ts +++ b/src/rules/valid-expect.ts @@ -26,7 +26,6 @@ import { */ const getPromiseCallExpressionNode = (node: TSESTree.Node) => { if ( - node && node.type === AST_NODE_TYPES.ArrayExpression && node.parent && node.parent.type === AST_NODE_TYPES.CallExpression @@ -273,6 +272,7 @@ export default createRule< const targetNode = getParentIfThenified(parentNode); const finalNode = findPromiseCallExpressionNode(targetNode) || targetNode; + if ( finalNode.parent && // If node is not awaited or returned diff --git a/yarn.lock b/yarn.lock index 5a5ee5d2e..f788681f8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3327,6 +3327,14 @@ eslint-plugin-eslint-comments@^3.1.2: escape-string-regexp "^1.0.5" ignore "^5.0.5" +eslint-plugin-eslint-config@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/eslint-plugin-eslint-config/-/eslint-plugin-eslint-config-1.0.2.tgz#b08c261e7c619f247e5f5bb6eb6b6e55be2a4e6b" + integrity sha512-MPlJtWOn+GuVg5SgydPYF9yPQhJ9IzygBKtrJzmNbgCt5EZxkwauK5s8ULlzCX+dRCZVfCD3/K2aM63HBogUPA== + dependencies: + "@typescript-eslint/experimental-utils" "^2.5.0" + require-relative "^0.8.7" + eslint-plugin-eslint-plugin@^2.0.0: version "2.2.1" resolved "https://registry.yarnpkg.com/eslint-plugin-eslint-plugin/-/eslint-plugin-eslint-plugin-2.2.1.tgz#9d567f2bb6891935d5d3e741f66c9ac42dc688fd" @@ -7806,6 +7814,11 @@ require-main-filename@^2.0.0: resolved "https://registry.yarnpkg.com/require-main-filename/-/require-main-filename-2.0.0.tgz#d0b329ecc7cc0f61649f62215be69af54aa8989b" integrity sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg== +require-relative@^0.8.7: + version "0.8.7" + resolved "https://registry.yarnpkg.com/require-relative/-/require-relative-0.8.7.tgz#7999539fc9e047a37928fa196f8e1563dabd36de" + integrity sha1-eZlTn8ngR6N5KPoZb44VY9q9Nt4= + resolve-cwd@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/resolve-cwd/-/resolve-cwd-3.0.0.tgz#0f0075f1bb2544766cf73ba6a6e2adfebcb13f2d"