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-empty-title rule #238

Merged
merged 17 commits into from Mar 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -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-empty-title][] | Disallow empty titles | | |
| [no-focused-tests][] | Disallow focused tests | ![recommended][] | |
| [no-hooks][] | Disallow setup and teardown hooks | | |
| [no-identical-title][] | Disallow identical titles | ![recommended][] | |
Expand Down Expand Up @@ -131,6 +132,7 @@ for more information about extending configuration files.
[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-empty-title]: docs/rules/no-empty-title.md
[no-focused-tests]: docs/rules/no-focused-tests.md
[no-hooks]: docs/rules/no-hooks.md
[no-identical-title]: docs/rules/no-identical-title.md
Expand Down
36 changes: 36 additions & 0 deletions docs/rules/no-empty-title.md
@@ -0,0 +1,36 @@
# Disallow empty titles

Having an empty string as your test title is pretty useless. This rule reports
an error if it finds an empty string as s test title.

This rule is not auto-fixable.

## Rule Details

The following patterns are considered warnings:

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

These patterns would not be considered warnings:

```js
describe('foo', () => {});
describe('foo', () => {
it('bar', () => {});
});
test('foo', () => {});
it('foo', () => {});
xdescribe('foo', () => {});
xit('foo', () => {});
xtest('foo', () => {});
```
108 changes: 108 additions & 0 deletions rules/__tests__/no-empty-title.js
@@ -0,0 +1,108 @@
'use strict';

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

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

ruleTester.run('no-empty-title', rule, {
valid: [
'someFn("", function () {})',
'describe(1, function () {})',
'describe("foo", function () {})',
'describe("foo", function () { it("bar", function () {}) })',
'test("foo", function () {})',
'test(`foo`, function () {})',
'test(`${foo}`, function () {})',
"it('foo', function () {})",
"xdescribe('foo', function () {})",
"xit('foo', function () {})",
"xtest('foo', function () {})",
],
invalid: [
{
code: 'describe("", function () {})',
errors: [
{
message: rule.errorMessages.describe,
column: 1,
line: 1,
},
],
},
{
code: ["describe('foo', () => {", "it('', () => {})", '})'].join('\n'),
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 2,
},
],
},
{
code: 'it("", function () {})',
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 1,
},
],
},
{
code: 'test("", function () {})',
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 1,
},
],
},
{
code: 'test(``, function () {})',
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 1,
},
],
},
{
code: "xdescribe('', () => {})",
errors: [
{
message: rule.errorMessages.describe,
column: 1,
line: 1,
},
],
},
{
code: "xit('', () => {})",
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 1,
},
],
},
{
code: "xtest('', () => {})",
errors: [
{
message: rule.errorMessages.test,
column: 1,
line: 1,
},
],
},
],
});
6 changes: 6 additions & 0 deletions rules/__tests__/no-identical-title.test.js
Expand Up @@ -69,6 +69,12 @@ ruleTester.run('no-identical-title', rule, {
es6: true,
},
},
{
code: 'it(`${n}`, function() {});',
env: {
es6: true,
},
},
[
'describe("title " + foo, function() {',
' describe("describe1", function() {});',
Expand Down
54 changes: 54 additions & 0 deletions rules/no-empty-title.js
@@ -0,0 +1,54 @@
'use strict';

const {
getDocsUrl,
hasExpressions,
isDescribe,
isTestCase,
isTemplateLiteral,
isString,
getStringValue,
} = require('./util');

const errorMessages = {
describe: 'describe should not have an empty title',
test: 'test should not have an empty title',
};

module.exports = {
meta: {
docs: {
url: getDocsUrl(__filename),
},
},
create(context) {
return {
CallExpression(node) {
const is = {
describe: isDescribe(node),
testCase: isTestCase(node),
};
if (!is.describe && !is.testCase) {
return;
}
const [firstArgument] = node.arguments;
if (!isString(firstArgument)) {
return;
}
if (isTemplateLiteral(firstArgument) && hasExpressions(firstArgument)) {
return;
}
if (getStringValue(firstArgument) === '') {
const message = is.describe
? errorMessages.describe
: errorMessages.test;
context.report({
message,
node,
});
}
},
};
},
errorMessages,
};
11 changes: 2 additions & 9 deletions rules/no-identical-title.js
Expand Up @@ -6,6 +6,7 @@ const {
isTestCase,
isString,
hasExpressions,
getStringValue,
} = require('./util');

const newDescribeContext = () => ({
Expand Down Expand Up @@ -50,14 +51,6 @@ const isFirstArgValid = arg => {
return true;
};

const getArgValue = arg => {
if (arg.type === 'TemplateLiteral') {
return arg.quasis[0].value.raw;
}

return arg.value;
};

module.exports = {
meta: {
docs: {
Expand All @@ -76,7 +69,7 @@ module.exports = {
if (!isFirstArgValid(firstArgument)) {
return;
}
const title = getArgValue(firstArgument);
const title = getStringValue(firstArgument);
handleTestCaseTitles(context, currentLayer.testTitles, node, title);
handleDescribeBlockTitles(
context,
Expand Down
9 changes: 8 additions & 1 deletion rules/util.js
Expand Up @@ -132,10 +132,15 @@ const isFunction = node =>

const isString = node =>
(node.type === 'Literal' && typeof node.value === 'string') ||
node.type === 'TemplateLiteral';
isTemplateLiteral(node);

const isTemplateLiteral = node => node.type === 'TemplateLiteral';

const hasExpressions = node => node.expressions && node.expressions.length > 0;

const getStringValue = arg =>
isTemplateLiteral(arg) ? arg.quasis[0].value.raw : arg.value;

/**
* Generates the URL to documentation for the given rule name. It uses the
* package version to build the link to a tagged version of the
Expand Down Expand Up @@ -210,8 +215,10 @@ module.exports = {
expectToEqualCase,
expectNotToEqualCase,
getNodeName,
getStringValue,
isDescribe,
isFunction,
isTemplateLiteral,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops my bad!

isTestCase,
isString,
hasExpressions,
Expand Down