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
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-out-tests][] | Disallow commented out tests | | |
| [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-out-tests]: docs/rules/no-commented-out-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-out-tests.md
@@ -0,0 +1,61 @@
# Disallow commented out tests (no-commented-out-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
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
215 changes: 215 additions & 0 deletions src/rules/__tests__/no-commented-out-tests.test.js
@@ -0,0 +1,215 @@
'use strict';

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

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

ruleTester.run('no-commented-out-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()',
[
'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: '// fit("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: `// test(
// "foo", function () {}
// )`,
errors: [
{ message: 'Some tests seem to be commented', column: 1, line: 1 },
],
},
{
code: `/* test
(
"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: '// it()',
errors: [
{
message: 'Some tests seem to be commented',
column: 1,
line: 1,
},
],
},
{
code: '// test.someNewMethodThatMightBeAddedInTheFuture()',
errors: [
{
message: 'Some tests seem to be commented',
column: 1,
line: 1,
},
],
},
{
code: '// test["someNewMethodThatMightBeAddedInTheFuture"]()',
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-out-tests.js
@@ -0,0 +1,39 @@
'use strict';

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

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

function hasTests(node) {
return /(x|f)?(test|it|describe)(\.\w+|\[['"]\w+['"]\])?\s*\(/m.test(
node.value,
);
}

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

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

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

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

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