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: adds no jest import rule #95

Merged
merged 2 commits into from
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: adds no jest import rule
  • Loading branch information
brianlmacdonald authored and SimenB committed Mar 5, 2018
commit 37ba866ea928c02cb934d8c87a2f229f67158c9e
20 changes: 20 additions & 0 deletions docs/rules/no-jest-import.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Disallow importing Jest(no-jest-import)

The jest object is automatically in scope within every test file. The methods in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a code block for the jest object.

the jest object help create mocks and let you control Jest's overall behavior.
It is therefore completely unnecessary to import in Jest, as Jest doesn't export
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change Jest to jest here, as you refer to the object.

anything in the first place.

### Rule details

This rule reports on any importing of Jest.

To name a few: *var jest = require('jest'); *const jest = require('jest');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use code blocks here?

  • var jest = require('jest');
  • const jest = require('jest');
  • import jest from 'jest';
  • import {jest as test} from 'jest';

*import jest from 'jest'; *import {jest as test} from 'jest';

There is no correct usage of this code, other than to not import Jest it in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jest => jest

first place.

## Further Reading

\*[The Jest Object](https://facebook.github.io/jest/docs/en/jest-object.html)
3 changes: 3 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const noDisabledTests = require('./rules/no-disabled-tests');
const noFocusedTests = require('./rules/no-focused-tests');
const noHooks = require('./rules/no-hooks');
const noIdenticalTitle = require('./rules/no-identical-title');
const noJestImport = require('./rules/no-jest-import');
const noLargeSnapshots = require('./rules/no-large-snapshots');
const noTestPrefixes = require('./rules/no-test-prefixes');
const preferToBeNull = require('./rules/prefer-to-be-null');
Expand All @@ -29,6 +30,7 @@ module.exports = {
'jest/no-disabled-tests': 'warn',
'jest/no-focused-tests': 'error',
'jest/no-identical-title': 'error',
'jest/no-jest-import': 'error',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically breaking to add it here, mind removing it?

'jest/prefer-to-have-length': 'warn',
'jest/valid-expect': 'error',
},
Expand Down Expand Up @@ -67,6 +69,7 @@ module.exports = {
'no-focused-tests': noFocusedTests,
'no-hooks': noHooks,
'no-identical-title': noIdenticalTitle,
'no-jest-import': noJestImport,
'no-large-snapshots': noLargeSnapshots,
'no-test-prefixes': noTestPrefixes,
'prefer-to-be-null': preferToBeNull,
Expand Down
71 changes: 71 additions & 0 deletions rules/__tests__/no-jest-import.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
'use strict';

const rule = require('../no-jest-import.js');
const RuleTester = require('eslint').RuleTester;
const ruleTester = new RuleTester();

ruleTester.run('no-jest-import', rule, {
valid: [
{
code: 'import something from "something"',
parserOptions: { sourceType: 'module' },
},
'require("somethingElse")',
'entirelyDifferent(fn)',
],
invalid: [
{
code: 'require("jest")',
errors: [
{
endColumn: 15,
column: 9,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
},
],
},
{
code: 'import jest from "jest"',
parserOptions: { sourceType: 'module' },
errors: [
{
endColumn: 24,
column: 1,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
},
],
},
{
code: 'var jest = require("jest")',
errors: [
{
endColumn: 26,
column: 20,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
},
],
},
{
code: 'import {jest as test} from "jest"',
parserOptions: { sourceType: 'module' },
errors: [
{
endColumn: 34,
column: 1,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
},
],
},
{
code: 'const jest = require("jest")',
parserOptions: { sourceType: 'module' },
errors: [
{
endColumn: 28,
column: 22,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
},
],
},
],
});
39 changes: 39 additions & 0 deletions rules/no-jest-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const getDocsUrl = require('./util').getDocsUrl;
const getNodeName = require('./util').getNodeName;

module.exports = {
meta: {
docs: {
url: getDocsUrl(__filename),
},
},
create(context) {
return {
ImportDeclaration(node) {
if (node.source.value === 'jest') {
context.report({
node,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message can be moved into a variable so you won't write it twice.

});
}
},
CallExpression(node) {
const calleeName = getNodeName(node.callee);
if (calleeName === 'require' && node.arguments[0].value === 'jest') {
context.report({
loc: {
end: {
column: node.arguments[0].loc.end.column,
line: node.arguments[0].loc.end.line,
},
start: node.arguments[0].loc.start,
},
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
});
}
},
};
},
};