-
-
Notifications
You must be signed in to change notification settings - Fork 30
New: rule prefer-output-null #20
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
Conversation
Oops, I think I might have miscommunicated what the rule should do in #19. I think it's fine for test cases to not have an However, the rule should warn when a test case does have an // valid
{
code: 'foo',
errors: 1
}
// valid
{
code: 'foo',
output: 'bar',
errors: 1
}
// invalid
{
code: 'foo',
output: 'foo', // <-- same as `code`
errors: 1
} |
Hmm... seem like I missed the tests' meaning. don't worry, easy to modify:( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have a few nitpicks, but feel free to merge when they are fixed.
docs/rules/prefer-output-null.md
Outdated
new RuleTester().run('foo', bar, { | ||
valid: [], | ||
invalid: [ | ||
{ code: 'foo', errors: [{ message: 'bar' }] }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example should be updated to have output: 'foo'
.
docs/rules/prefer-output-null.md
Outdated
|
||
## Rule Details | ||
|
||
This rule aims to enforce `output: null` for invalid test cases to indicate that no output is produced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior of the rule would be clearer if a sentence was added:
The rule reports an error if it encounters a test case where the output is the same as the code.
lib/rules/prefer-output-null.js
Outdated
// Public | ||
// ---------------------------------------------------------------------- | ||
|
||
const message = 'Prefer `output: null` to assert that a test case is not autofixed.'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I think it would be better to change the first word to "Use" rather than "Prefer".
fixes #19