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.

Sign up for GitHub

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

Jump to bottom

Some fixes for require-tothrow-message #161

Merged
merged 7 commits into from Sep 30, 2018
Merged

Some fixes for require-tothrow-message #161

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
df46b83
test: clean up. use actual errors. add comments. add more tests
garyking Sep 29, 2018
c691faa
test: allow for any argument type, even functions
garyking Sep 29, 2018
9f1b240
fix: use ES6 since already using babel-eslint
garyking Sep 29, 2018
16d4369
test: ensure using expect
garyking Sep 29, 2018
33da0cb
test: use ecmaVersion 6, and uncomment template literal test
garyking Sep 30, 2018
88139a4
test: replace `function()` with `() =>`, since now ES6
garyking Sep 30, 2018
112aa12
test: fix filename
garyking Sep 30, 2018
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
33 changes: 27 additions & 6 deletions rules/__tests__/require-tothrow-message.js
View file
Open in desktop
Expand Up @@ -7,23 +7,44 @@ const ruleTester = new RuleTester();

ruleTester.run('require-tothrow-message', rule, {
valid: [
"expect(function() { a() }).toThrow('a');",
"expect(function() { a() }).toThrowError('a');",
// String
"expect(function() { throw new Error('a'); }).toThrow('a');",
"expect(function() { throw new Error('a'); }).toThrowError('a');",

// Template literal
// {
// code:
// "const a = 'a'; expect(() => { throw new Error('a'); })" +
// '.toThrow(`${a}`);',
// parser: 'babel-eslint',
Copy link
Member

Choose a reason for hiding this comment

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

I think it should work if you set ecmaVersion: https://eslint.org/docs/user-guide/configuring#specifying-parser-options

// },

// Regex
"expect(function() { throw new Error('a'); }).toThrow(/^a$/);",

// Function
"expect(function() { throw new Error('a'); })" +
".toThrow((function() { return 'a'; })());",

// Allow no message for `not`.
"expect(function() { throw new Error('a'); }).not.toThrow();",
],

invalid: [
// Empty toThrow
{
code: 'expect(function() { a() }).toThrow();',
code: "expect(function() { throw new Error('a'); }).toThrow();",
errors: [
{ message: 'Add an error message to toThrow()', column: 28, line: 1 },
{ message: 'Add an error message to toThrow()', column: 46, line: 1 },
],
},
// Empty toThrowError
{
code: 'expect(function() { a() }).toThrowError();',
code: "expect(function() { throw new Error('a'); }).toThrowError();",
errors: [
{
message: 'Add an error message to toThrowError()',
column: 28,
column: 46,
line: 1,
},
],
Expand Down
13 changes: 10 additions & 3 deletions rules/require-tothrow-message.js
View file
Open in desktop
@@ -1,6 +1,9 @@
'use strict';

const argument = require('./util').argument;
const expectCase = require('./util').expectCase;
const getDocsUrl = require('./util').getDocsUrl;
const method = require('./util').method;

module.exports = {
meta: {
Expand All @@ -11,19 +14,23 @@ module.exports = {
create(context) {
return {
CallExpression(node) {
const propertyName = node.callee.property && node.callee.property.name;
if (!expectCase(node)) {
return;
}

const propertyName = method(node) && method(node).name;

// Look for `toThrow` calls with no arguments.
if (
['toThrow', 'toThrowError'].indexOf(propertyName) > -1 &&
!(node.arguments[0] && node.arguments[0].type === 'Literal')
!argument(node)
) {
context.report({
message: `Add an error message to {{ propertyName }}()`,
data: {
propertyName,
},
node: node.callee.property,
node: method(node),
});
}
},
Expand Down