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

Add no-error-ctor-with-notthrows rule #240

Closed
wants to merge 8 commits into from
Closed

Conversation

rahgurung
Copy link
Contributor

@rahgurung rahgurung commented May 3, 2019

fixes #178
added rule with tests (100% coverage).

@sindresorhus
Copy link
Member

Please read and adhere to https://github.com/avajs/eslint-plugin-ava/blob/master/docs/new-rule.md (Probably smart to read it multiple times as it's easy to miss something).

The rule needs a more specific name that follows the naming convention of the other rules.

rules/prevent-errortype.js Outdated Show resolved Hide resolved
test/prevent-errortype.js Outdated Show resolved Hide resolved
test/prevent-errortype.js Outdated Show resolved Hide resolved
rules/prevent-errortype.js Outdated Show resolved Hide resolved
@rahgurung
Copy link
Contributor Author

@novemberborn @sindresorhus can you guys suggest a better name? I think we can go with - no-errortype-with-tnotthrows. It's a little long name but is quite self-explanatory.

@rahgurung rahgurung changed the title rule prevent-errortype Add no-typeerror-with-notthrows May 5, 2019
@rahgurung rahgurung changed the title Add no-typeerror-with-notthrows Add no-typeerror-with-notthrows rule May 5, 2019
@rahgurung
Copy link
Contributor Author

@sindresorhus

@novemberborn
Copy link
Member

@gurungrahul2 thanks for the updates. We do get notifications when commits are pushed so there's no need to remind us. It just may take some time before we can get back to you.

@rahgurung
Copy link
Contributor Author

Sorry @novemberborn , i will take care.

@sindresorhus
Copy link
Member

sindresorhus commented May 14, 2019

You missed this:

(The description should be the same as the heading of the documentation file).

And:

Add the correct meta.type to the rule.

https://github.com/avajs/eslint-plugin-ava/blob/master/docs/new-rule.md

@sindresorhus
Copy link
Member

The rule should handle .notThrowsAsync too.

@sindresorhus
Copy link
Member

With #240 (comment), I meant they would be more readable if you format them like normal code, using multiple lines.

docs/rules/no-typeerror-with-notthrows.md Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
rules/no-typeerror-with-notthrows.js Outdated Show resolved Hide resolved
@rahgurung rahgurung changed the title Add no-typeerror-with-notthrows rule Add no-error-ctor-with-notthrows rule May 14, 2019
@rahgurung
Copy link
Contributor Author

Can this be more better?

@sindresorhus
Copy link
Member

I haven't forgotten about this. I just haven't had the chance to review it properly yet. Maybe @JLHwung and @GMartigny have time to help review.

readme.md Outdated Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Outdated Show resolved Hide resolved
test/no-error-ctor-with-notthrows.js Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Outdated Show resolved Hide resolved
test/no-error-ctor-with-notthrows.js Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Show resolved Hide resolved
rules/no-error-ctor-with-notthrows.js Outdated Show resolved Hide resolved
changed name of test
and added more tests
with other requested changes.
defined variables just before
they are used and added more
tests
added more checks as guided
@rahgurung
Copy link
Contributor Author

Review ping. :) @sindresorhus

@@ -84,6 +85,7 @@ The rules will only activate in test files.
- [no-async-fn-without-await](docs/rules/no-async-fn-without-await.md) - Ensure that async tests use `await`.
- [no-cb-test](docs/rules/no-cb-test.md) - Ensure no `test.cb()` is used.
- [no-duplicate-modifiers](docs/rules/no-duplicate-modifiers.md) - Ensure tests do not have duplicate modifiers.
- [no-error-ctor-with-notthrows](docs/rules/no-error-ctor-with-notthrows.md) - No specifying error type in `t.notThrows()`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [no-error-ctor-with-notthrows](docs/rules/no-error-ctor-with-notthrows.md) - No specifying error type in `t.notThrows()`.
- [no-error-ctor-with-notthrows](docs/rules/no-error-ctor-with-notthrows.md) - Ensure no error constructor is specified in `t.notThrows()`.


const errors = [{ruleId: 'no-error-ctor-with-notthrows'}];

const header = `const test = require('ava');\n`; // eslint-disable-line quotes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const header = `const test = require('ava');\n`; // eslint-disable-line quotes
const header = 'const test = require(\'ava\');\n';

ruleTester.run('no-error-ctor-with-notthrows', rule, {
valid: [
`${header}
test('some test',t => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
test('some test',t => {
test('some test', t => {

Could you fix this for all the other times ?

@@ -62,7 +62,7 @@
"ava": {
"files": [
"!rules",
"test/*.js"
"test/*js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"test/*js"
"test/*.js"

Why change this ?

Base automatically changed from master to main January 23, 2021 07:39
@sindresorhus
Copy link
Member

Closing for lack of activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent specifying error type in t.notThrows()
5 participants