Skip to content

Commit

Permalink
feat(rules): no-standalone-expect (#350)
Browse files Browse the repository at this point in the history
Fixes #342
  • Loading branch information
rabelfish authored and SimenB committed Jul 26, 2019
1 parent 1f92185 commit 9e3e94f
Show file tree
Hide file tree
Showing 5 changed files with 289 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ installations requiring long-term consistency.
| [no-jest-import][] | Disallow importing `jest` | ![recommended][] | |
| [no-large-snapshots][] | Disallow large snapshots | | |
| [no-mocks-import][] | Disallow manually importing from `__mocks__` | | |
| [no-standalone-expect][] | Prevents `expect` statements outside of a `test` or `it` block | | |
| [no-test-callback][] | Using a callback in asynchronous tests | | ![fixable-green][] |
| [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][] |
| [no-test-return-statement][] | Disallow explicitly returning from tests | | |
Expand Down Expand Up @@ -174,6 +175,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
[no-jest-import]: docs/rules/no-jest-import.md
[no-large-snapshots]: docs/rules/no-large-snapshots.md
[no-mocks-import]: docs/rules/no-mocks-import.md
[no-standalone-expect]: docs/rules/no-standalone-expect.md
[no-test-callback]: docs/rules/no-test-callback.md
[no-test-prefixes]: docs/rules/no-test-prefixes.md
[no-test-return-statement]: docs/rules/no-test-return-statement.md
Expand Down
69 changes: 69 additions & 0 deletions docs/rules/no-standalone-expect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# No standalone expect in a describe block (no-standalone-expect)

Prevents `expect` statements outside of a `test` or `it` block. An `expect`
within a helper function (but outside of a `test` or `it` block) will not
trigger this rule.

## Rule Details

This rule aims to eliminate `expect` statements that will not be executed. An
`expect` inside of a `describe` block but outside of a `test` or `it` block or
outside of a `describe` will not execute and therefore will trigger this rule.
It is viable, however, to have an `expect` in a helper function that is called
from within a `test` or `it` block so `expect` statements in a function will not
trigger this rule.

Statements like `expect.hasAssertions()` will NOT trigger this rule since these
calls will execute if they are not in a test block.

Examples of **incorrect** code for this rule:

```js
// in describe
describe('a test', () => {
expect(1).toBe(1);
});

// below other tests
describe('a test', () => {
it('an it', () => {
expect(1).toBe(1);
});

expect(1).toBe(1);
});
```

Examples of **correct** code for this rule:

```js
// in it block
describe('a test', () => {
it('an it', () => {
expect(1).toBe(1);
});
});

// in helper function
describe('a test', () => {
const helper = () => {
expect(1).toBe(1);
};

it('an it', () => {
helper();
});
});

describe('a test', () => {
expect.hasAssertions(1);
});
```

\*Note that this rule will not trigger if the helper function is never used even
thought the `expect` will not execute. Rely on a rule like no-unused-vars for
this case.

## When Not To Use It

Don't use this rule on non-jest test files.
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { resolve } from 'path';
import { rules } from '../';

const ruleNames = Object.keys(rules);
const numberOfRules = 36;
const numberOfRules = 37;

describe('rules', () => {
it('should have a corresponding doc for each rule', () => {
Expand Down
77 changes: 77 additions & 0 deletions src/rules/__tests__/no-standalone-expect-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../no-standalone-expect';

const ruleTester = new TSESLint.RuleTester({
parserOptions: {
ecmaVersion: 2015,
},
});

ruleTester.run('no-standalone-expect', rule, {
valid: [
'describe("a test", () => { it("an it", () => {expect(1).toBe(1); }); });',
'describe("a test", () => { it("an it", () => { const func = () => { expect(1).toBe(1); }; }); });',
'describe("a test", () => { const func = () => { expect(1).toBe(1); }; });',
'describe("a test", () => { function func() { expect(1).toBe(1); }; });',
'describe("a test", () => { const func = function(){ expect(1).toBe(1); }; });',
'it("an it", () => expect(1).toBe(1))',
'const func = function(){ expect(1).toBe(1); };',
'const func = () => expect(1).toBe(1);',
'expect.hasAssertions()',
'{}',
'it.each([1, true])("trues", value => { expect(value).toBe(true); });',
'it.each([1, true])("trues", value => { expect(value).toBe(true); }); it("an it", () => { expect(1).toBe(1) });',
`
it.each\`
num | value
\${1} | \${true}
\`('trues', ({ value }) => {
expect(value).toBe(true);
});
`,
'it.only("an only", value => { expect(value).toBe(true); });',
'describe.each([1, true])("trues", value => { it("an it", () => expect(value).toBe(true) ); });',
],
invalid: [
{
code: 'describe("a test", () => { expect(1).toBe(1); });',
errors: [{ endColumn: 37, column: 28, messageId: 'unexpectedExpect' }],
},
{
code: 'describe("a test", () => expect(1).toBe(1));',
errors: [{ endColumn: 35, column: 26, messageId: 'unexpectedExpect' }],
},
{
code:
'describe("a test", () => { const func = () => { expect(1).toBe(1); }; expect(1).toBe(1); });',
errors: [{ endColumn: 80, column: 71, messageId: 'unexpectedExpect' }],
},
{
code:
'describe("a test", () => { it(() => { expect(1).toBe(1); }); expect(1).toBe(1); });',
errors: [{ endColumn: 72, column: 63, messageId: 'unexpectedExpect' }],
},
{
code: 'expect(1).toBe(1);',
errors: [{ endColumn: 10, column: 1, messageId: 'unexpectedExpect' }],
},
{
code: 'expect(1).toBe',
errors: [{ endColumn: 10, column: 1, messageId: 'unexpectedExpect' }],
},
{
code: '{expect(1).toBe(1)}',
errors: [{ endColumn: 11, column: 2, messageId: 'unexpectedExpect' }],
},
{
code:
'it.each([1, true])("trues", value => { expect(value).toBe(true); }); expect(1).toBe(1);',
errors: [{ endColumn: 79, column: 70, messageId: 'unexpectedExpect' }],
},
{
code:
'describe.each([1, true])("trues", value => { expect(value).toBe(true); });',
errors: [{ endColumn: 59, column: 46, messageId: 'unexpectedExpect' }],
},
],
});
140 changes: 140 additions & 0 deletions src/rules/no-standalone-expect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import {
TestCaseName,
createRule,
isDescribe,
isExpectCall,
isFunction,
isTestCase,
} from './tsUtils';

const getBlockType = (
stmt: TSESTree.BlockStatement,
): 'function' | 'describe' | null => {
const func = stmt.parent;

/* istanbul ignore if */
if (!func) {
throw new Error(
`Unexpected BlockStatement. No parent defined. - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
);
}
// functionDeclaration: function func() {}
if (func.type === AST_NODE_TYPES.FunctionDeclaration) {
return 'function';
}
if (isFunction(func) && func.parent) {
const expr = func.parent;
// arrowfunction or function expr
if (expr.type === AST_NODE_TYPES.VariableDeclarator) {
return 'function';
}
// if it's not a variable, it will be callExpr, we only care about describe
if (expr.type === AST_NODE_TYPES.CallExpression && isDescribe(expr)) {
return 'describe';
}
}
return null;
};

const isEach = (node: TSESTree.CallExpression): boolean => {
if (
node &&
node.callee &&
node.callee.type === AST_NODE_TYPES.CallExpression &&
node.callee.callee &&
node.callee.callee.type === AST_NODE_TYPES.MemberExpression &&
node.callee.callee.property &&
node.callee.callee.property.type === AST_NODE_TYPES.Identifier &&
node.callee.callee.property.name === 'each' &&
node.callee.callee.object &&
node.callee.callee.object.type === AST_NODE_TYPES.Identifier &&
TestCaseName.hasOwnProperty(node.callee.callee.object.name)
) {
return true;
}
return false;
};

type callStackEntry =
| 'test'
| 'function'
| 'describe'
| 'arrowFunc'
| 'template';

export default createRule({
name: __filename,
meta: {
docs: {
category: 'Best Practices',
description: 'Prevents expects that are outside of an it or test block.',
recommended: false,
},
messages: {
unexpectedExpect: 'Expect must be inside of a test block.',
},
type: 'suggestion',
schema: [],
},
defaultOptions: [],
create(context) {
const callStack: callStackEntry[] = [];

return {
CallExpression(node) {
if (isExpectCall(node)) {
const parent = callStack[callStack.length - 1];
if (!parent || parent === 'describe') {
context.report({ node, messageId: 'unexpectedExpect' });
}
return;
}
if (isTestCase(node)) {
callStack.push('test');
}
if (node.callee.type === AST_NODE_TYPES.TaggedTemplateExpression) {
callStack.push('template');
}
},
'CallExpression:exit'(node: TSESTree.CallExpression) {
const top = callStack[callStack.length - 1];
if (
(((isTestCase(node) &&
node.callee.type !== AST_NODE_TYPES.MemberExpression) ||
isEach(node)) &&
top === 'test') ||
(node.callee.type === AST_NODE_TYPES.TaggedTemplateExpression &&
top === 'template')
) {
callStack.pop();
}
},
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();
}
},
ArrowFunctionExpression(node) {
if (node.parent && node.parent.type !== AST_NODE_TYPES.CallExpression) {
callStack.push('arrowFunc');
}
},
'ArrowFunctionExpression:exit'() {
if (callStack[callStack.length - 1] === 'arrowFunc') {
callStack.pop();
}
},
};
},
});

0 comments on commit 9e3e94f

Please sign in to comment.