Skip to content

Add rule prefer-object-rule #101

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 1 commit into from
Jun 23, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Name | ✔️ | 🛠 | Description
[no-missing-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-placeholders.md) | ✔️ | | disallow missing placeholders in rule report messages
[no-unused-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-unused-placeholders.md) | ✔️ | | disallow unused placeholders in rule report messages
[no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 | disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken
[prefer-object-rule](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-object-rule.md) | | 🛠 | disallow rule exports where the export is a function.
[prefer-output-null](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-output-null.md) | | 🛠 | disallow invalid RuleTester test cases with the output the same as the code.
[prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | disallow template literals as report messages
[prefer-replace-text](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-replace-text.md) | | | require using replaceText instead of replaceTextRange.
Expand Down
49 changes: 49 additions & 0 deletions docs/rules/prefer-object-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Disallow rule exports where the export is a function. (prefer-object-rule)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

## Rule Details

The rule reports an error if it encounters a rule that's defined using the old style of just a `create` function.

Examples of **incorrect** code for this rule:

```js
/* eslint eslint-plugin/prefer-object-rule: error */

module.exports = function (context) {
return { Program() { context.report() } };
};

module.exports = function create(context) {
return { Program() { context.report() } };
};

module.exports = (context) => {
return { Program() { context.report() } };
};
Copy link
Member

Choose a reason for hiding this comment

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

I personally wouldn't bother showing multiple ways of defining the same function or object. A single function (incorrect) example and a single object (correct) example get the idea across. But up to you.

```

Examples of **correct** code for this rule:

```js
/* eslint eslint-plugin/prefer-object-rule: error */

module.exports = {
create(context) {
return { Program() { context.report() } };
},
};

module.exports = {
create(context) {
return { Program() { context.report() } };
},
};
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a duplicate example of the above.


module.exports = {
create: (context) => {
return { Program() { context.report() } };
},
};
```
Copy link
Member

Choose a reason for hiding this comment

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

This rule doc could have a ## References section which links to this doc:

https://eslint.org/docs/developer-guide/working-with-rules

74 changes: 74 additions & 0 deletions lib/rules/prefer-object-rule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* @author Brad Zacher <https://github.com/bradzacher>
*/

'use strict';

const utils = require('../utils');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'disallow rule exports where the export is a function.',
category: 'Rules',
recommended: false,
},
messages: {
preferObject: 'Rules should be declared using the object style.',
},
type: 'suggestion',
fixable: 'code',
schema: [],
},

create (context) {
// ----------------------------------------------------------------------
// Public
// ----------------------------------------------------------------------

const sourceCode = context.getSourceCode();
const ruleInfo = utils.getRuleInfo(sourceCode.ast);

return {
Program () {
if (!ruleInfo || ruleInfo.isNewStyle) {
return;
}

context.report({
node: ruleInfo.create,
messageId: 'preferObject',
*fix (fixer) {
// note - we intentionally don't worry about formatting here, as otherwise we have
// to indent the function correctly

if (ruleInfo.create.type === 'FunctionExpression') {
const openParenToken = sourceCode.getFirstToken(
ruleInfo.create,
token => token.type === 'Punctuator' && token.value === '('
);

if (!openParenToken) {
// this shouldn't happen, but guarding against crashes just in case
return null;
}

yield fixer.replaceTextRange(
[ruleInfo.create.range[0], openParenToken.range[0]],
'{create'
);
yield fixer.insertTextAfter(ruleInfo.create, '}');
} else if (ruleInfo.create.type === 'ArrowFunctionExpression') {
yield fixer.insertTextBefore(ruleInfo.create, '{create: ');
yield fixer.insertTextAfter(ruleInfo.create, '}');
}
},
});
},
};
},
};
109 changes: 109 additions & 0 deletions tests/lib/rules/prefer-object-rule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/**
* @author Brad Zacher <https://github.com/bradzacher>
*/

'use strict';

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const rule = require('../../../lib/rules/prefer-object-rule');
const RuleTester = require('eslint').RuleTester;

const ERROR = { messageId: 'preferObject', line: 2, column: 26 };

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
ruleTester.run('prefer-object-rule', rule, {
valid: [
`
module.exports = {
create(context) {
return { Program() { context.report() } };
},
};
`,
`
module.exports = {
create: (context) => {
return { Program() { context.report() } };
},
};
`,
`
module.exports.create = (context) => {
return { Program() { context.report() } };
};
`,
`
module.exports.create = function (context) {
return { Program() { context.report() } };
};
`,
`
module.exports.create = function create(context) {
return { Program() { context.report() } };
};
`,
`
function create(context) {
return { Program() { context.report() } };
};
module.exports.create = create;
`,
`
const rule = {
create(context) {
return { Program() { context.report() } };
},
};
module.exports = rule;
`,
],

invalid: [
{
code: `
module.exports = function (context) {
return { Program() { context.report() } };
};
`,
output: `
module.exports = {create(context) {
return { Program() { context.report() } };
}};
`,
errors: [ERROR],
},
{
code: `
module.exports = function create(context) {
return { Program() { context.report() } };
};
`,
output: `
module.exports = {create(context) {
return { Program() { context.report() } };
}};
`,
errors: [ERROR],
},
{
code: `
module.exports = (context) => {
return { Program() { context.report() } };
};
`,
output: `
module.exports = {create: (context) => {
return { Program() { context.report() } };
}};
`,
errors: [ERROR],
},
],
});