diff --git a/docs/ExpectAPI.md b/docs/ExpectAPI.md index 06b4a9d643b5..d0ced25354bf 100644 --- a/docs/ExpectAPI.md +++ b/docs/ExpectAPI.md @@ -654,7 +654,7 @@ test('the house has my desired features', () => { ### `toHaveProperty(keyPath, value)` -Use `.toHaveProperty` to check if property at provided reference `keyPath` exists for an object. +Use `.toHaveProperty` to check if property at provided reference `keyPath` exists for an object. For checking deeply nested properties in an object use [dot notation](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/Property_accessors) for deep references. Optionally, you can provide a value to check if it's strictly equal to the `value` present @@ -666,17 +666,17 @@ to check for the existence and values of various properties in the object. ```js // Object containing house features to be tested const houseForSale = { - bath: true, - kitchen: { - amenities: ['oven', 'stove', 'washer'], - area: 20, - wallColor: 'white' - }, + bath: true, bedrooms: 4, + kitchen: { + amenities: ['oven', 'stove', 'washer'], + area: 20, + wallColor: 'white', + }, }; test('this house has my desired features', () => { - // Simple Referencing + // Simple Referencing expect(houseForSale).toHaveProperty('bath'); expect(houseForSale).toHaveProperty('bedrooms', 4); @@ -684,7 +684,11 @@ test('this house has my desired features', () => { // Deep referencing using dot notation expect(houseForSale).toHaveProperty('kitchen.area', 20); - expect(houseForSale).toHaveProperty('kitchen.amenities', ['oven', 'stove', 'washer']); + expect(houseForSale).toHaveProperty('kitchen.amenities', [ + 'oven', + 'stove', + 'washer', + ]); expect(hosueForSale).not.toHaveProperty('kitchen.open'); }); diff --git a/docs/SetupAndTeardown.md b/docs/SetupAndTeardown.md index f31cfa4ee461..847b6aacab0f 100644 --- a/docs/SetupAndTeardown.md +++ b/docs/SetupAndTeardown.md @@ -93,11 +93,11 @@ describe('matching cities to foods', () => { }); test('Vienna <3 sausage', () => { - expect(isValidCityFoodPair('Vienna', 'Wiener Schnitzel')); + expect(isValidCityFoodPair('Vienna', 'Wiener Schnitzel')).toBe(true); }); test('San Juan <3 plantains', () => { - expect(isValidCityFoodPair('San Juan', 'Mofongo')); + expect(isValidCityFoodPair('San Juan', 'Mofongo')).toBe(true); }); }); ``` diff --git a/packages/eslint-config-fb-strict/index.js b/packages/eslint-config-fb-strict/index.js index cd117af84654..bc27f4d89acb 100644 --- a/packages/eslint-config-fb-strict/index.js +++ b/packages/eslint-config-fb-strict/index.js @@ -41,6 +41,7 @@ module.exports = Object.assign({}, fbjsConfig, { 'flowtype/object-type-delimiter': [2, 'comma'], 'jest/no-focused-tests': [2], 'jest/no-identical-title': [2], + 'jest/valid-expect': [2], 'max-len': [2, { 'code': 80, 'ignorePattern': maxLenIgnorePattern, diff --git a/packages/eslint-plugin-jest/README.md b/packages/eslint-plugin-jest/README.md index 4477ff63e1e0..5e094766369b 100644 --- a/packages/eslint-plugin-jest/README.md +++ b/packages/eslint-plugin-jest/README.md @@ -31,6 +31,7 @@ Then configure the rules you want to use under the rules section. "jest/no-disabled-tests": "warn", "jest/no-focused-tests": "error", "jest/no-identical-title": "error", + "jest/valid-expect": "error", } } ``` @@ -50,6 +51,7 @@ You can also whitelist the environment variables provided by Jest by doing: - [no-disabled-tests](/packages/eslint-plugin-jest/docs/rules/no-disabled-tests.md) - disallow disabled tests. - [no-focused-tests](/packages/eslint-plugin-jest/docs/rules/no-focused-tests.md) - disallow focused tests. - [no-identical-title](/packages/eslint-plugin-jest/docs/rules/no-identical-title.md) - disallow identical titles. +- [valid-expect](/packages/eslint-plugin-jest/docs/rules/valid-expect.md) - ensure expect is called correctly. ## Shareable configurations @@ -72,6 +74,7 @@ The rules enabled in this configuration are: - [jest/no-disabled-tests](/packages/eslint-plugin-jest/docs/rules/no-disabled-tests.md) - [jest/no-focused-tests](/packages/eslint-plugin-jest/docs/rules/no-focused-tests.md) - [jest/no-identical-title](/packages/eslint-plugin-jest/docs/rules/no-identical-title.md) +- [jest/valid-expect](/packages/eslint-plugin-jest/docs/rules/valid-expect.md) ## Credit diff --git a/packages/eslint-plugin-jest/docs/rules/valid-expect.md b/packages/eslint-plugin-jest/docs/rules/valid-expect.md new file mode 100644 index 000000000000..8a3e10c4e79d --- /dev/null +++ b/packages/eslint-plugin-jest/docs/rules/valid-expect.md @@ -0,0 +1,41 @@ +# Enforce valid `expect()` usage (valid-expect) + +Ensure `expect()` is called with a single argument and there is an actual expectation made. + +## Rule details + +This rule triggers a warning if `expect()` is called with more than one argument or without arguments. +It would also issue a warning if there is nothing called on `expect()`, e.g.: + +```js +expect(); +expect('something'); +``` + +or when a matcher function was not called, e.g.: + +```js +expect(true).toBeDefined +``` + +This rule is enabled by default. + +### Default configuration + +The following patterns are considered warnings: + +```js +expect(); +expect().toEqual('something'); +expect('something', 'else'); +expect('something'); +expect(true).toBeDefined; +``` + +The following patterns are not warnings: + +```js +expect('something').toEqual('something'); +expect([1, 2, 3]).toEqual([1, 2, 3]); +expect(true).toBeDefined(); +``` diff --git a/packages/eslint-plugin-jest/src/index.js b/packages/eslint-plugin-jest/src/index.js index ebb599cccfe9..7f1d5a456a57 100644 --- a/packages/eslint-plugin-jest/src/index.js +++ b/packages/eslint-plugin-jest/src/index.js @@ -17,6 +17,7 @@ module.exports = { 'jest/no-disabled-tests': 'warn', 'jest/no-focused-tests': 'error', 'jest/no-identical-title': 'error', + 'jest/valid-expect': 'error', }, }, }, @@ -46,5 +47,6 @@ module.exports = { 'no-disabled-tests': require('./rules/no-disabled-tests'), 'no-focused-tests': require('./rules/no-focused-tests'), 'no-identical-title': require('./rules/no-identical-title'), + 'valid-expect': require('./rules/valid-expect'), }, }; 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 new file mode 100644 index 000000000000..b8785bfbc18c --- /dev/null +++ b/packages/eslint-plugin-jest/src/rules/__tests__/valid-expect-test.js @@ -0,0 +1,81 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @flow + */ + +/* eslint-disable sort-keys */ + +'use strict'; + +const RuleTester = require('eslint').RuleTester; +const rules = require('../../').rules; + +const ruleTester = new RuleTester(); + +ruleTester.run('valid-expect', rules['valid-expect'], { + valid: [ + 'expect("something").toEqual("else");', + 'expect(true).toBeDefined();', + 'expect([1, 2, 3]).toEqual([1, 2, 3]);', + 'expect(undefined).not.toBeDefined();', + ], + + invalid: [ + { + code: 'expect().toBe(true);', + errors: [ + { + message: 'No arguments were passed to expect().', + }, + ], + }, + { + code: 'expect().toEqual("something");', + errors: [ + { + message: 'No arguments were passed to expect().', + }, + ], + }, + { + code: 'expect("something", "else").toEqual("something");', + errors: [ + { + message: 'More than one argument was passed to expect().', + }, + ], + }, + { + code: 'expect("something");', + errors: [ + { + message: 'No assertion was called on expect().', + }, + ], + }, + { + code: 'expect();', + errors: [ + { + message: 'No arguments were passed to expect().', + }, + { + message: 'No assertion was called on expect().', + }, + ], + }, + { + code: 'expect(true).toBeDefined;', + errors: [ + { + message: '"toBeDefined" was not called.', + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin-jest/src/rules/types.js b/packages/eslint-plugin-jest/src/rules/types.js index 9bd936295768..90e1af71d1ad 100644 --- a/packages/eslint-plugin-jest/src/rules/types.js +++ b/packages/eslint-plugin-jest/src/rules/types.js @@ -8,31 +8,37 @@ * @flow */ -export type Identifier = {| +type Node = MemberExpression | CallExpression; + +export type Identifier = { type: 'Identifier', name: string, value: string, -|}; + parent: Node, +}; -export type MemberExpression = {| +export type MemberExpression = { type: 'MemberExpression', name: string, expression: CallExpression, property: Identifier, object: Identifier, -|}; + parent: Node, +}; -export type Literal = {| +export type Literal = { type: 'Literal', value?: string, rawValue?: string, -|}; + parent: Node, +}; -export type CallExpression = {| +export type CallExpression = { type: 'CallExpression', arguments: [Literal], callee: Identifier | MemberExpression, -|}; + parent: Node, +}; export type EslintContext = {| report: ({message: string, node: any}) => void diff --git a/packages/eslint-plugin-jest/src/rules/valid-expect.js b/packages/eslint-plugin-jest/src/rules/valid-expect.js new file mode 100644 index 000000000000..b325bd9282a6 --- /dev/null +++ b/packages/eslint-plugin-jest/src/rules/valid-expect.js @@ -0,0 +1,61 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @flow + */ +'use strict'; + +/* + * This implementation is ported from from eslint-plugin-jasmine. + * MIT license, Tom Vincent. + */ + +import type {EslintContext, CallExpression} from './types'; + +module.exports = (context: EslintContext) => { + return { + CallExpression(node: CallExpression) { + if (node.callee.name === 'expect') { + // checking "expect()" arguments + if (node.arguments.length > 1) { + context.report({ + message: 'More than one argument was passed to expect().', + node, + }); + } else if (node.arguments.length === 0) { + context.report({ + message: 'No arguments were passed to expect().', + node, + }); + } + + // matcher was not called + if ( + node.parent && + node.parent.type === 'MemberExpression' && + node.parent.parent && + node.parent.parent.type === 'ExpressionStatement' + ) { + context.report({ + message: `"${node.parent.property.name}" was not called.`, + node, + }); + } + } + }, + + // nothing called on "expect()" + 'CallExpression:exit'(node: CallExpression) { + if ( + node.callee.name === 'expect' && + node.parent.type === 'ExpressionStatement' + ) { + context.report({message: 'No assertion was called on expect().', node}); + } + }, + }; +}; diff --git a/packages/pretty-format/src/__tests__/pretty-format-test.js b/packages/pretty-format/src/__tests__/pretty-format-test.js index 9874df348955..4517369b51ad 100644 --- a/packages/pretty-format/src/__tests__/pretty-format-test.js +++ b/packages/pretty-format/src/__tests__/pretty-format-test.js @@ -234,8 +234,8 @@ describe('prettyFormat()', () => { }); it('prints a string with escapes', () => { - expect(prettyFormat('\"-\"'), '"\\"-\\""'); - expect(prettyFormat('\\ \\\\'), '"\\\\ \\\\\\\\"'); + expect(prettyFormat('\"-\"')).toEqual('"\\"-\\""'); + expect(prettyFormat('\\ \\\\')).toEqual('"\\\\ \\\\\\\\"'); }); it('prints a symbol', () => {