Skip to content

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

Merged
merged 6 commits into from
Jul 18, 2017

Conversation

aladdin-add
Copy link
Contributor

@aladdin-add aladdin-add commented Jul 16, 2017

fixes #19

@not-an-aardvark
Copy link
Contributor

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 output property at all. For example, some rules aren't fixable, and it would be annoying to specify output: null for every test case.

However, the rule should warn when a test case does have an output property specified, and the output property is the same as the code property.

// valid
{
  code: 'foo',
  errors: 1
}

// valid
{
  code: 'foo',
  output: 'bar',
  errors: 1
}

// invalid
{
  code: 'foo',
  output: 'foo', // <-- same as `code`
  errors: 1
}

@aladdin-add
Copy link
Contributor Author

Hmm... seem like I missed the tests' meaning. don't worry, easy to modify:(

Copy link
Contributor

@not-an-aardvark not-an-aardvark left a 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.

new RuleTester().run('foo', bar, {
valid: [],
invalid: [
{ code: 'foo', errors: [{ message: 'bar' }] },
Copy link
Contributor

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'.


## Rule Details

This rule aims to enforce `output: null` for invalid test cases to indicate that no output is produced.
Copy link
Contributor

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.

// Public
// ----------------------------------------------------------------------

const message = 'Prefer `output: null` to assert that a test case is not autofixed.';
Copy link
Contributor

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".

@aladdin-add aladdin-add merged commit 9f98ac4 into eslint-community:master Jul 18, 2017
@aladdin-add aladdin-add deleted the issue19 branch July 18, 2017 01:46
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.

Rule: prefer output: null to assert that a test case is not autofixed
2 participants