From 9199211e4f3fe16fc9055ae080d4fb83552b0904 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 1 May 2017 10:04:11 +0200 Subject: [PATCH] Add eslint rules for ensuring methods on expect are called correctly --- .../src/rules/__tests__/valid-expect-test.js | 180 +++++++++ .../src/rules/valid-expect.js | 356 ++++++++++++++---- 2 files changed, 453 insertions(+), 83 deletions(-) diff --git a/packages/eslint-plugin-jest/src/rules/__tests__/valid-expect-test.js b/packages/eslint-plugin-jest/src/rules/__tests__/valid-expect-test.js index 4610129c3d73..96b110eb8bc7 100644 --- a/packages/eslint-plugin-jest/src/rules/__tests__/valid-expect-test.js +++ b/packages/eslint-plugin-jest/src/rules/__tests__/valid-expect-test.js @@ -25,6 +25,16 @@ ruleTester.run('valid-expect', rules['valid-expect'], { 'expect(undefined).not.toBeDefined();', 'expect(Promise.resolve(2)).resolves.toBeDefined();', 'expect(Promise.reject(2)).rejects.toBeDefined();', + 'expect.hasAssertions();', + 'expect.anything();', + 'expect.stringContaining("foobar");', + 'expect.arrayContaining(["foobar"]);', + 'expect.addSnapshotSerializer({});', + 'expect.extend({});', + 'expect.objectContaining({});', + 'expect.assertions(2);', + 'expect.stringMatching(/foobar/);', + 'expect.stringMatching(new RegExp("foobar"));', ], invalid: [ @@ -113,5 +123,175 @@ ruleTester.run('valid-expect', rules['valid-expect'], { }, ], }, + { + code: 'expect.assertions();', + errors: [ + { + endColumn: 18, + column: 8, + message: '"assertions" must be called with arguments.', + }, + ], + }, + { + code: 'expect.assertions("foobar");', + errors: [ + { + endColumn: 27, + column: 19, + message: 'Argument to "assertions" must be a number.', + }, + ], + }, + { + code: 'expect.hasAssertions("foobar");', + errors: [ + { + endColumn: 30, + column: 22, + message: '"hasAssertions" must not be called with arguments.', + }, + ], + }, + { + code: 'expect.anything("foobar");', + errors: [ + { + endColumn: 25, + column: 17, + message: '"anything" must not be called with arguments.', + }, + ], + }, + { + code: 'expect.stringContaining();', + errors: [ + { + endColumn: 24, + column: 8, + message: '"stringContaining" must be called with arguments.', + }, + ], + }, + { + code: 'expect.stringContaining(true);', + errors: [ + { + endColumn: 29, + column: 25, + message: 'Argument to "stringContaining" must be a string.', + }, + ], + }, + { + code: 'expect.arrayContaining();', + errors: [ + { + endColumn: 23, + column: 8, + message: '"arrayContaining" must be called with arguments.', + }, + ], + }, + { + code: 'expect.arrayContaining("foobar");', + errors: [ + { + endColumn: 32, + column: 24, + message: 'Argument to "arrayContaining" must be an array.', + }, + ], + }, + { + code: 'expect.arrayContaining({});', + errors: [ + { + endColumn: 26, + column: 24, + message: 'Argument to "arrayContaining" must be an array.', + }, + ], + }, + { + code: 'expect.addSnapshotSerializer();', + errors: [ + { + endColumn: 29, + column: 8, + message: '"addSnapshotSerializer" must be called with arguments.', + }, + ], + }, + { + code: 'expect.addSnapshotSerializer("foobar");', + errors: [ + { + endColumn: 38, + column: 30, + message: 'Argument to "addSnapshotSerializer" must be an object.', + }, + ], + }, + { + code: 'expect.extend();', + errors: [ + { + endColumn: 14, + column: 8, + message: '"extend" must be called with arguments.', + }, + ], + }, + { + code: 'expect.extend("foobar");', + errors: [ + { + endColumn: 23, + column: 15, + message: 'Argument to "extend" must be an object.', + }, + ], + }, + { + code: 'expect.objectContaining();', + errors: [ + { + endColumn: 24, + column: 8, + message: '"objectContaining" must be called with arguments.', + }, + ], + }, + { + code: 'expect.objectContaining("foobar");', + errors: [ + { + endColumn: 33, + column: 25, + message: 'Argument to "objectContaining" must be an object.', + }, + ], + }, + { + code: 'expect.stringMatching();', + errors: [ + { + endColumn: 22, + column: 8, + message: '"stringMatching" must be called with arguments.', + }, + ], + }, + { + code: 'expect.stringMatching("foobar");', + errors: [ + { + endColumn: 31, + column: 23, + message: 'Argument to "stringMatching" must be a regexp.', + }, + ], + }, ], }); diff --git a/packages/eslint-plugin-jest/src/rules/valid-expect.js b/packages/eslint-plugin-jest/src/rules/valid-expect.js index 5a7487ea7b61..e500415aaf00 100644 --- a/packages/eslint-plugin-jest/src/rules/valid-expect.js +++ b/packages/eslint-plugin-jest/src/rules/valid-expect.js @@ -18,92 +18,282 @@ import type {EslintContext, CallExpression} from './types'; const expectProperties = ['not', 'resolves', 'rejects']; +const expectFunctionsWithoutArgs = ['anything', 'hasAssertions']; +const expectFunctionsWithStringArgs = ['stringContaining']; +const expectFunctionsWithArrayArgs = ['arrayContaining']; +const expectFunctionsWithObjectArgs = [ + 'extend', + 'objectContaining', + 'addSnapshotSerializer', +]; +const expectFunctionsWithNumberArgs = ['assertions']; +const expectFunctionsWithRegExpArgs = ['stringMatching']; + +const expectFunctionsWithArgs = ['any'].concat( + expectFunctionsWithArrayArgs, + expectFunctionsWithNumberArgs, + expectFunctionsWithObjectArgs, + expectFunctionsWithRegExpArgs, + expectFunctionsWithStringArgs, +); + +const expectFunctions = expectFunctionsWithArgs.concat( + expectFunctionsWithoutArgs, +); + +function checkExpectFunction(calleeName, node, context) { + // checking "expect()" arguments + if (node.arguments.length > 1) { + const secondArgumentLocStart = node.arguments[1].loc.start; + const lastArgumentLocEnd = + node.arguments[node.arguments.length - 1].loc.end; + + context.report({ + loc: { + end: { + column: lastArgumentLocEnd.column - 1, + line: lastArgumentLocEnd.line, + }, + start: secondArgumentLocStart, + }, + message: 'More than one argument was passed to expect().', + node, + }); + } else if (node.arguments.length === 0) { + const expectLength = calleeName.length; + context.report({ + loc: { + end: { + column: node.loc.start.column + expectLength + 1, + line: node.loc.start.line, + }, + start: { + column: node.loc.start.column + expectLength, + line: node.loc.start.line, + }, + }, + message: 'No arguments were passed to expect().', + node, + }); + } + + // something was called on `expect()` + if ( + node.parent && + node.parent.type === 'MemberExpression' && + node.parent.parent + ) { + let parentNode = node.parent; + let parentProperty = parentNode.property; + let propertyName = parentProperty.name; + let grandParent = parentNode.parent; + + // a property is accessed, get the next node + if (grandParent.type === 'MemberExpression') { + // a modifier is used, just get the next one + if (expectProperties.indexOf(propertyName) > -1) { + grandParent = grandParent.parent; + } else { + // only a few properties are allowed + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: parentProperty.loc, + message: `"${propertyName}" is not a valid property of expect.`, + node: parentProperty, + }); + } + + // this next one should be the matcher + parentNode = parentNode.parent; + // $FlowFixMe + parentProperty = parentNode.property; + propertyName = parentProperty.name; + } + + // matcher was not called + if (grandParent.type === 'ExpressionStatement') { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is not + // added + loc: parentProperty.loc, + message: `"${propertyName}" was not called.`, + node: parentProperty, + }); + } + } +} + +function checkExpectExpression(callee, node, context) { + const {property} = callee; + const propertyName = property.name; + const argument = node.arguments[0]; + + if (expectFunctions.indexOf(propertyName) < 0) { + // only a few properties are allowed + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: property.loc, + message: `"${propertyName}" is not a valid property of expect.`, + node: property, + }); + + return; + } + + if ( + expectFunctionsWithArgs.indexOf(propertyName) >= 0 && + node.arguments.length === 0 + ) { + // only a few properties are allowed + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: property.loc, + message: `"${propertyName}" must be called with arguments.`, + node: property, + }); + + return; + } + + if (node.arguments.length > 1) { + const secondArgumentLocStart = node.arguments[1].loc.start; + const lastArgumentLocEnd = + node.arguments[node.arguments.length - 1].loc.end; + + context.report({ + loc: { + end: { + column: lastArgumentLocEnd.column - 1, + line: lastArgumentLocEnd.line, + }, + start: secondArgumentLocStart, + }, + message: `More than one argument was passed to ${propertyName}().`, + node, + }); + + return; + } + + if (expectFunctionsWithoutArgs.indexOf(propertyName) >= 0) { + if (node.arguments.length > 0) { + // only a few properties are allowed + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `"${propertyName}" must not be called with arguments.`, + node: argument, + }); + } + + return; + } + + let {type} = argument; + + if (type === 'Literal') { + // if `type` is `object`, it's a regex. Objects are + // `type === ObjectExpression` (or `Identifier`) + type = typeof argument.value; + } + + if (type === 'Identifier') { + // No way to typecheck an identifier + return; + } + + if (expectFunctionsWithStringArgs.indexOf(propertyName) >= 0) { + if (type !== 'string') { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `Argument to "${propertyName}" must be a string.`, + node: argument, + }); + } + + return; + } + + if (expectFunctionsWithArrayArgs.indexOf(propertyName) >= 0) { + if (type !== 'ArrayExpression') { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `Argument to "${propertyName}" must be an array.`, + node: argument, + }); + } + + return; + } + + if (expectFunctionsWithObjectArgs.indexOf(propertyName) >= 0) { + if (type !== 'ObjectExpression') { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `Argument to "${propertyName}" must be an object.`, + node: argument, + }); + } + + return; + } + + if (expectFunctionsWithNumberArgs.indexOf(propertyName) >= 0) { + if (type !== 'number') { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `Argument to "${propertyName}" must be a number.`, + node: argument, + }); + } + + return; + } + + if (expectFunctionsWithRegExpArgs.indexOf(propertyName) >= 0) { + if ( + type !== 'object' && + !(type === 'NewExpression' && argument.callee.name === 'RegExp') + ) { + context.report({ + // For some reason `endColumn` isn't set in tests if `loc` is + // not added + loc: argument.loc, + message: `Argument to "${propertyName}" must be a regexp.`, + node: argument, + }); + } + + return; + } +} + module.exports = (context: EslintContext) => { return { CallExpression(node: CallExpression) { - const calleeName = node.callee.name; - - if (calleeName === 'expect') { - // checking "expect()" arguments - if (node.arguments.length > 1) { - const secondArgumentLocStart = node.arguments[1].loc.start; - const lastArgumentLocEnd = - node.arguments[node.arguments.length - 1].loc.end; - - context.report({ - loc: { - end: { - column: lastArgumentLocEnd.column - 1, - line: lastArgumentLocEnd.line, - }, - start: secondArgumentLocStart, - }, - message: 'More than one argument was passed to expect().', - node, - }); - } else if (node.arguments.length === 0) { - const expectLength = calleeName.length; - context.report({ - loc: { - end: { - column: node.loc.start.column + expectLength + 1, - line: node.loc.start.line, - }, - start: { - column: node.loc.start.column + expectLength, - line: node.loc.start.line, - }, - }, - message: 'No arguments were passed to expect().', - node, - }); - } - - // something was called on `expect()` - if ( - node.parent && - node.parent.type === 'MemberExpression' && - node.parent.parent - ) { - let parentNode = node.parent; - let parentProperty = parentNode.property; - let propertyName = parentProperty.name; - let grandParent = parentNode.parent; - - // a property is accessed, get the next node - if (grandParent.type === 'MemberExpression') { - // a modifier is used, just get the next one - if (expectProperties.indexOf(propertyName) > -1) { - grandParent = grandParent.parent; - } else { - // only a few properties are allowed - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is - // not added - loc: parentProperty.loc, - message: `"${propertyName}" is not a valid property of expect.`, - node: parentProperty, - }); - } - - // this next one should be the matcher - parentNode = parentNode.parent; - // $FlowFixMe - parentProperty = parentNode.property; - propertyName = parentProperty.name; - } - - // matcher was not called - if (grandParent.type === 'ExpressionStatement') { - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is not - // added - loc: parentProperty.loc, - message: `"${propertyName}" was not called.`, - node: parentProperty, - }); - } - } + const callee = node.callee; + + if (callee.name === 'expect') { + checkExpectFunction(callee.name, node, context); + } + + if ( + callee.type === 'MemberExpression' && + callee.object.name === 'expect' + ) { + checkExpectExpression(callee, node, context); } },