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): add no-commented-out rule #262

Merged
merged 13 commits into from
May 22, 2019
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ for more information about extending configuration files.
| [lowercase-name][] | Disallow capitalized test names | | ![fixable-green][] |
| [no-alias-methods][] | Disallow alias methods | ![recommended][] | ![fixable-green][] |
| [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | |
| [no-commented-tests][] | Disallow commented tests | | | s |
| [no-empty-title][] | Disallow empty titles | | |
| [no-focused-tests][] | Disallow focused tests | ![recommended][] | |
| [no-hooks][] | Disallow setup and teardown hooks | | |
Expand Down Expand Up @@ -142,6 +143,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
[lowercase-name]: docs/rules/lowercase-name.md
[no-alias-methods]: docs/rules/no-alias-methods.md
[no-disabled-tests]: docs/rules/no-disabled-tests.md
[no-commented-tests]: docs/rules/no-commented-tests.md
[no-empty-title]: docs/rules/no-empty-title.md
[no-focused-tests]: docs/rules/no-focused-tests.md
[no-hooks]: docs/rules/no-hooks.md
Expand Down
61 changes: 61 additions & 0 deletions docs/rules/no-commented-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Disallow commented tests (no-commented-tests)

This rule raises a warning about commented out tests. It's similar to
no-disabled-tests rule.

## Rule Details

The rule uses fuzzy matching to do its best to determine what constitutes a
commented out test, checking for a presence of `it(`, `describe(`, `it.skip(`,
etc. in code comments.

<!-- The following patterns are considered warnings:

```js
// describe('foo', () => {});
// it('foo', () => {});
// test('foo', () => {});

// describe.skip('foo', () => {});
// it.skip('foo', () => {});
// test.skip('foo', () => {});

// describe['skip']('bar', () => {});
// it['skip']('bar', () => {});
// test['skip']('bar', () => {});

// xdescribe('foo', () => {});
// xit('foo', () => {});
// xtest('foo', () => {});

/*
describe('foo', () => {});
*/
```

These patterns would not be considered warnings:

```js
describe('foo', () => {});
it('foo', () => {});
test('foo', () => {});

describe.only('bar', () => {});
it.only('bar', () => {});
test.only('bar', () => {});

// foo('bar', () => {});
```

### Limitations

The plugin looks at the literal function names within test code, so will not
catch more complex examples of commented out tests, such as:

```js
// const testSkip = test.skip;
// testSkip('skipped test', () => {});

// const myTest = test;
// myTest('does not have function body');
``` -->
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const path = require('path');
const { rules } = require('../');

const ruleNames = Object.keys(rules);
const numberOfRules = 31;
const numberOfRules = 32;

describe('rules', () => {
it('should have a corresponding doc for each rule', () => {
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports = {
rules: {
'jest/no-alias-methods': 'warn',
'jest/no-disabled-tests': 'warn',
'jest/no-commented-tests': 'warn',
'jest/no-focused-tests': 'error',
'jest/no-identical-title': 'error',
'jest/no-jest-import': 'error',
Expand Down
161 changes: 161 additions & 0 deletions src/rules/__tests__/no-commented-tests.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
'use strict';

const { RuleTester } = require('eslint');
const rule = require('../no-commented-tests');

const ruleTester = new RuleTester({
parserOptions: {
sourceType: 'module',
},
});

ruleTester.run('no-commented-tests', rule, {
valid: [
'// foo("bar", function () {})',
'describe("foo", function () {})',
'it("foo", function () {})',
'describe.only("foo", function () {})',
'it.only("foo", function () {})',
'test("foo", function () {})',
'test.only("foo", function () {})',
'var appliedSkip = describe.skip; appliedSkip.apply(describe)',
'var calledSkip = it.skip; calledSkip.call(it)',
'({ f: function () {} }).f()',
'(a || b).f()',
'itHappensToStartWithIt()',
'testSomething()',
[
kangax marked this conversation as resolved.
Show resolved Hide resolved
'import { pending } from "actions"',
'',
'test("foo", () => {',
' expect(pending()).toEqual({})',
'})',
].join('\n'),
[
'const { pending } = require("actions")',
'',
'test("foo", () => {',
' expect(pending()).toEqual({})',
'})',
].join('\n'),
[
'test("foo", () => {',
' const pending = getPending()',
' expect(pending()).toEqual({})',
'})',
].join('\n'),
[
'test("foo", () => {',
' expect(pending()).toEqual({})',
'})',
'',
'function pending() {',
' return {}',
'}',
].join('\n'),
],

invalid: [
{
code: '// describe("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// describe["skip"]("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// describe[\'skip\']("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// it.skip("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// it.only("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// it["skip"]("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// test.skip("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// test["skip"]("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// xdescribe("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// xit("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// xtest("foo", function () {})',
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: '// it("has title but no callback")',
errors: [
{
message: 'Some tests seem to be commented',
column: 1,
line: 1,
},
],
},
{
code: '// test("has title but no callback")',
errors: [
{
message: 'Some tests seem to be commented',
column: 1,
line: 1,
},
],
},
{
code: `
foo()
/*
describe("has title but no callback", () => {})
*/
bar()`,
errors: [
{
message: 'Some tests seem to be commented',
column: 7,
line: 3,
},
],
},
],
});
39 changes: 39 additions & 0 deletions src/rules/no-commented-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const { getDocsUrl } = require('./util');

const message = 'Some tests seem to be commented';

function hasAssertions(node) {
kangax marked this conversation as resolved.
Show resolved Hide resolved
return /x?(test|it|describe)((\.only|\.skip|\[['"]skip['"]\]))?\(/.test(
kangax marked this conversation as resolved.
Show resolved Hide resolved
node.value,
);
}

module.exports = {
meta: {
docs: {
url: getDocsUrl(__filename),
},
},
create(context) {
const sourceCode = context.getSourceCode();

function checkNode(node) {
if (!hasAssertions(node)) return;

context.report({
message,
node,
});
}

return {
Program() {
const comments = sourceCode.getAllComments();

comments.filter(token => token.type !== 'Shebang').forEach(checkNode);
},
};
},
};