Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rules): no-standalone-expect #350

Merged
merged 34 commits into from Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ad187e2
This probably doesn't work
Jul 22, 2019
91561bd
please commit
Jul 22, 2019
8f9be1c
Fixed lint errors
Jul 24, 2019
5bc1a4d
Increased number of rules
Jul 24, 2019
ba1b60f
Added export to isExpectCall so I could use it in my rule
Jul 24, 2019
7a5429f
Merged master
Jul 24, 2019
a40edf7
Added another test
Jul 24, 2019
0b68d60
Added Docs
Jul 24, 2019
f5d7fce
Resolving PR comments
Jul 24, 2019
e2f0f5a
fix: added test cases and updated docs to address expect.hasAssertions
Jul 24, 2019
557e9a7
fix: added some more typescript, removed sourcetype etc
Jul 25, 2019
334beb7
chore: merge master
Jul 25, 2019
306b0ef
fix: couple more trivial edits
Jul 25, 2019
3c39ae4
Merge branch 'master' into woo
SimenB Jul 25, 2019
70d3a06
chore: fix lint
SimenB Jul 25, 2019
8aec8c2
chore: narrow type def and ditch extra else
SimenB Jul 25, 2019
7db9a6f
chore: add rule to readme
SimenB Jul 25, 2019
9656d90
fix: added more tests
Jul 25, 2019
d24f4af
chore: merged
Jul 25, 2019
a6f6253
fix: refactored code to handle es6 functions without block statements
Jul 25, 2019
0a693de
fix: added error for block statemnet with not parent
Jul 25, 2019
0ef09d8
chore: added another test case
Jul 25, 2019
ff3e800
fix: fixed the bug for arrowfunctionsgit statusgit status
Jul 25, 2019
9278be4
fix: added type for callexpression exit
Jul 25, 2019
1f4a838
chore: tighten type
SimenB Jul 25, 2019
91b9f2f
chore: return null over false
SimenB Jul 25, 2019
0fd6c7e
fix: false -> null, removed extra return
Jul 25, 2019
f2990ba
chore: merge master
Jul 25, 2019
a6ab052
fix: it.each now works
Jul 26, 2019
e9b48ba
fix: made sure describe.each also works
Jul 26, 2019
e8fa3e2
chore: fix type error
SimenB Jul 26, 2019
6231ed2
chore: add failing test
SimenB Jul 26, 2019
00ec789
fix: added functionality for tagged template expressions
Jul 26, 2019
e662eed
fix: taggedtemplate expressions have to be in the callExpression
Jul 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
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
@@ -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
rabelfish marked this conversation as resolved.
Show resolved Hide resolved
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
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
@@ -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, {
rabelfish marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -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) {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
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 &&
SimenB marked this conversation as resolved.
Show resolved Hide resolved
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();
}
},
};
},
});