Skip to content

Commit

Permalink
Add eslint rule valid-expect (#3067)
Browse files Browse the repository at this point in the history
* Add eslint rule `valid-expect`

All credits goes to @tlvince and @alecxe

* Update valid-expect.md

* Update valid-expect-test.js

* Update valid-expect-test.js

* Update valid-expect.js

* Minor fixes and cleanups.

* Fix lint errors.
  • Loading branch information
SimenB authored and cpojer committed Mar 6, 2017
1 parent a78e120 commit e886064
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 21 deletions.
22 changes: 13 additions & 9 deletions docs/ExpectAPI.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -666,25 +666,29 @@ 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);

expect(houseForSale).not.toHaveProperty('pool');

// 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');
});
Expand Down
4 changes: 2 additions & 2 deletions docs/SetupAndTeardown.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
```
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-config-fb-strict/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions packages/eslint-plugin-jest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
}
```
Expand All @@ -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

Expand All @@ -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

Expand Down
41 changes: 41 additions & 0 deletions packages/eslint-plugin-jest/docs/rules/valid-expect.md
Original file line number Diff line number Diff line change
@@ -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();
```
2 changes: 2 additions & 0 deletions packages/eslint-plugin-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
},
Expand Down Expand Up @@ -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'),
},
};
Original file line number Diff line number Diff line change
@@ -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.',
},
],
},
],
});
22 changes: 14 additions & 8 deletions packages/eslint-plugin-jest/src/rules/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 61 additions & 0 deletions packages/eslint-plugin-jest/src/rules/valid-expect.js
Original file line number Diff line number Diff line change
@@ -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});
}
},
};
};
4 changes: 2 additions & 2 deletions packages/pretty-format/src/__tests__/pretty-format-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit e886064

Please sign in to comment.