From bad88a006135258e8da18902a84bdb52a9bb9fa7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 22 May 2020 09:59:44 +1200 Subject: [PATCH] feat(prefer-expect-assertions): provide suggestions --- .../prefer-expect-assertions.test.ts | 195 ++++++++++++++++-- src/rules/prefer-expect-assertions.ts | 151 +++++++++++--- 2 files changed, 304 insertions(+), 42 deletions(-) diff --git a/src/rules/__tests__/prefer-expect-assertions.test.ts b/src/rules/__tests__/prefer-expect-assertions.test.ts index 96cedeff4..648b7904f 100644 --- a/src/rules/__tests__/prefer-expect-assertions.test.ts +++ b/src/rules/__tests__/prefer-expect-assertions.test.ts @@ -1,4 +1,5 @@ import { TSESLint } from '@typescript-eslint/experimental-utils'; +import dedent from 'dedent'; import resolveFrom from 'resolve-from'; import rule from '../prefer-expect-assertions'; @@ -13,35 +14,196 @@ ruleTester.run('prefer-expect-assertions', rule, { invalid: [ { code: 'it("it1", () => {})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + suggestions: [ + { + messageId: 'suggestAddingHasAssertions', + output: 'it("it1", () => {expect.hasAssertions();})', + }, + { + messageId: 'suggestAddingAssertions', + output: 'it("it1", () => {expect.assertions();})', + }, + ], + }, + ], }, { code: 'it("it1", () => { foo()})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + suggestions: [ + { + messageId: 'suggestAddingHasAssertions', + output: 'it("it1", () => { expect.hasAssertions();foo()})', + }, + { + messageId: 'suggestAddingAssertions', + output: 'it("it1", () => { expect.assertions();foo()})', + }, + ], + }, + ], }, { - code: - 'it("it1", function() {' + - '\n\t\t\tsomeFunctionToDo();' + - '\n\t\t\tsomeFunctionToDo2();\n' + - '\t\t\t})', - errors: [{ messageId: 'haveExpectAssertions' }], + code: dedent` + it("it1", function() { + someFunctionToDo(); + someFunctionToDo2(); + }) + `, + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + suggestions: [ + { + messageId: 'suggestAddingHasAssertions', + output: dedent` + it("it1", function() { + expect.hasAssertions();someFunctionToDo(); + someFunctionToDo2(); + }) + `, + }, + { + messageId: 'suggestAddingAssertions', + output: dedent` + it("it1", function() { + expect.assertions();someFunctionToDo(); + someFunctionToDo2(); + }) + `, + }, + ], + }, + ], }, { code: 'it("it1", function() {var a = 2;})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + suggestions: [ + { + messageId: 'suggestAddingHasAssertions', + output: + 'it("it1", function() {expect.hasAssertions();var a = 2;})', + }, + { + messageId: 'suggestAddingAssertions', + output: 'it("it1", function() {expect.assertions();var a = 2;})', + }, + ], + }, + ], }, { code: 'it("it1", function() {expect.assertions();})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'assertionsRequiresOneArgument', + column: 30, + line: 1, + suggestions: [], + }, + ], }, { code: 'it("it1", function() {expect.assertions(1,2);})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'assertionsRequiresOneArgument', + column: 43, + line: 1, + suggestions: [ + { + messageId: 'suggestRemovingExtraArguments', + output: 'it("it1", function() {expect.assertions(1);})', + }, + ], + }, + ], }, { code: 'it("it1", function() {expect.assertions("1");})', - errors: [{ messageId: 'haveExpectAssertions' }], + errors: [ + { + messageId: 'assertionsRequiresNumberArgument', + column: 41, + line: 1, + suggestions: [], + }, + ], + }, + { + code: 'it("it1", function() {expect.hasAssertions("1");})', + errors: [ + { + messageId: 'hasAssertionsTakesNoArguments', + column: 30, + line: 1, + suggestions: [ + { + messageId: 'suggestRemovingExtraArguments', + output: 'it("it1", function() {expect.hasAssertions();})', + }, + ], + }, + ], + }, + { + code: 'it("it1", function() {expect.hasAssertions("1", "2");})', + errors: [ + { + messageId: 'hasAssertionsTakesNoArguments', + column: 30, + line: 1, + suggestions: [ + { + messageId: 'suggestRemovingExtraArguments', + output: 'it("it1", function() {expect.hasAssertions();})', + }, + ], + }, + ], + }, + { + code: dedent` + it("it1", function() { + expect.hasAssertions(() => { + someFunctionToDo(); + someFunctionToDo2(); + }); + }) + `, + errors: [ + { + messageId: 'hasAssertionsTakesNoArguments', + column: 10, + line: 2, + suggestions: [ + { + messageId: 'suggestRemovingExtraArguments', + output: dedent` + it("it1", function() { + expect.hasAssertions(); + }) + `, + }, + ], + }, + ], }, ], @@ -52,9 +214,12 @@ ruleTester.run('prefer-expect-assertions', rule, { 'test("it1", function() {expect.assertions(0);})', 'test("it1", function() {expect.hasAssertions();})', 'it("it1", function() {expect.assertions(0);})', - 'it("it1", function() {\n\t\t\texpect.assertions(1);' + - '\n\t\t\texpect(someValue).toBe(true)\n' + - '\t\t\t})', + ` + it("it1", function() { + expect.assertions(1); + expect(someValue).toBe(true) + }) + `, 'test("it1")', 'itHappensToStartWithIt("foo", function() {})', 'testSomething("bar", function() {})', diff --git a/src/rules/prefer-expect-assertions.ts b/src/rules/prefer-expect-assertions.ts index fcaf8b541..7ff02c19e 100644 --- a/src/rules/prefer-expect-assertions.ts +++ b/src/rules/prefer-expect-assertions.ts @@ -1,39 +1,26 @@ import { AST_NODE_TYPES, + TSESLint, TSESTree, } from '@typescript-eslint/experimental-utils'; import { + KnownCallExpression, createRule, getAccessorValue, hasOnlyOneArgument, isSupportedAccessor, } from './utils'; -const isExpectAssertionsOrHasAssertionsCall = (expression: TSESTree.Node) => { - if ( - expression.type !== AST_NODE_TYPES.CallExpression || - expression.callee.type !== AST_NODE_TYPES.MemberExpression || - !isSupportedAccessor(expression.callee.object, 'expect') || - !isSupportedAccessor(expression.callee.property) - ) { - return false; - } - - const expectAssertionName = getAccessorValue(expression.callee.property); - - if (expectAssertionName !== 'assertions') { - return expectAssertionName === 'hasAssertions'; - } - - const [arg] = expression.arguments; - - return ( - hasOnlyOneArgument(expression) && - arg.type === AST_NODE_TYPES.Literal && - typeof arg.value === 'number' && - Number.isInteger(arg.value) +const isExpectAssertionsOrHasAssertionsCall = ( + expression: TSESTree.Node, +): expression is KnownCallExpression<'assertions' | 'hasAssertions'> => + expression.type === AST_NODE_TYPES.CallExpression && + expression.callee.type === AST_NODE_TYPES.MemberExpression && + isSupportedAccessor(expression.callee.object, 'expect') && + isSupportedAccessor(expression.callee.property) && + ['assertions', 'hasAssertions'].includes( + getAccessorValue(expression.callee.property), ); -}; const isFirstLineExprStmt = ( functionBody: TSESTree.Statement[], @@ -41,6 +28,18 @@ const isFirstLineExprStmt = ( functionBody[0] && functionBody[0].type === AST_NODE_TYPES.ExpressionStatement; +const suggestRemovingExtraArguments = ( + args: TSESTree.CallExpression['arguments'], + extraArgsStartAt: number, +): TSESLint.ReportSuggestionArray[0] => ({ + messageId: 'suggestRemovingExtraArguments', + fix: fixer => + fixer.removeRange([ + args[extraArgsStartAt].range[0] - Math.sign(extraArgsStartAt), + args[args.length - 1].range[1], + ]), +}); + interface PreferExpectAssertionsCallExpression extends TSESTree.CallExpression { arguments: [ TSESTree.Expression, @@ -48,7 +47,21 @@ interface PreferExpectAssertionsCallExpression extends TSESTree.CallExpression { ]; } -export default createRule({ +type MessageIds = + | 'hasAssertionsTakesNoArguments' + | 'assertionsRequiresOneArgument' + | 'assertionsRequiresNumberArgument' + | 'haveExpectAssertions' + | 'suggestAddingHasAssertions' + | 'suggestAddingAssertions' + | 'suggestRemovingExtraArguments'; + +const suggestions: Array<[MessageIds, string]> = [ + ['suggestAddingHasAssertions', 'expect.hasAssertions();'], + ['suggestAddingAssertions', 'expect.assertions();'], +]; + +export default createRule<[], MessageIds>({ name: __filename, meta: { docs: { @@ -58,8 +71,17 @@ export default createRule({ recommended: false, }, messages: { + hasAssertionsTakesNoArguments: + '`expect.hasAssertions` expects no arguments', + assertionsRequiresOneArgument: + '`expect.assertions` excepts a single argument of type number', + assertionsRequiresNumberArgument: 'This argument should be a number', haveExpectAssertions: 'Every test should have either `expect.assertions()` or `expect.hasAssertions()` as its first expression', + suggestAddingHasAssertions: 'Add `expect.hasAssertions()`', + suggestAddingAssertions: + 'Add `expect.assertions()`', + suggestRemovingExtraArguments: 'Remove extra arguments', }, type: 'suggestion', schema: [], @@ -73,7 +95,21 @@ export default createRule({ const testFuncBody = node.arguments[1].body.body; if (!isFirstLineExprStmt(testFuncBody)) { - context.report({ messageId: 'haveExpectAssertions', node }); + context.report({ + messageId: 'haveExpectAssertions', + node, + suggest: suggestions.map(([messageId, text]) => ({ + messageId, + fix: fixer => + fixer.insertTextBeforeRange( + [ + node.arguments[1].body.range[0] + 1, + node.arguments[1].body.range[1], + ], + text, + ), + })), + }); return; } @@ -81,8 +117,69 @@ export default createRule({ const testFuncFirstLine = testFuncBody[0].expression; if (!isExpectAssertionsOrHasAssertionsCall(testFuncFirstLine)) { - context.report({ messageId: 'haveExpectAssertions', node }); + context.report({ + messageId: 'haveExpectAssertions', + node, + suggest: suggestions.map(([messageId, text]) => ({ + messageId, + fix: fixer => fixer.insertTextBefore(testFuncBody[0], text), + })), + }); + + return; } + + if ( + isSupportedAccessor( + testFuncFirstLine.callee.property, + 'hasAssertions', + ) + ) { + if (testFuncFirstLine.arguments.length) { + context.report({ + messageId: 'hasAssertionsTakesNoArguments', + node: testFuncFirstLine.callee.property, + suggest: [ + suggestRemovingExtraArguments(testFuncFirstLine.arguments, 0), + ], + }); + } + + return; + } + + if (!hasOnlyOneArgument(testFuncFirstLine)) { + const report: TSESLint.ReportDescriptor = { + messageId: 'assertionsRequiresOneArgument', + loc: testFuncFirstLine.callee.property.loc, + }; + + if (testFuncFirstLine.arguments.length) { + report.loc = testFuncFirstLine.arguments[1].loc; + report.suggest = [ + suggestRemovingExtraArguments(testFuncFirstLine.arguments, 1), + ]; + } + + context.report(report); + + return; + } + + const [arg] = testFuncFirstLine.arguments; + + if ( + arg.type === AST_NODE_TYPES.Literal && + typeof arg.value === 'number' && + Number.isInteger(arg.value) + ) { + return; + } + + context.report({ + messageId: 'assertionsRequiresNumberArgument', + node: arg, + }); }, }; },