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 import-style rule #232

Closed
wants to merge 6 commits into from
Closed
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
22 changes: 22 additions & 0 deletions docs/rules/import-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Whether to allow default imports or destructuring/named imports

Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, likely to be utilised. By default `path` and `chalk` are enforced to have default export and `util`, `lodash` and `underscore` are having named export. But you can easily override these properties by passing `false` for respective module.

## Fail

```js
const util = require('util');

import util from 'util';

import * as util from 'util';
```


## Pass

```js
const {promisify} = require('util');

import {promisify} from 'util';
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = {
'unicorn/explicit-length-check': 'error',
'unicorn/filename-case': 'error',
'unicorn/import-index': 'error',
'unicorn/import-style': 'error',
'unicorn/new-for-builtins': 'error',
'unicorn/no-abusive-eslint-disable': 'error',
'unicorn/no-array-instanceof': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Configure it in `package.json`.
"unicorn/explicit-length-check": "error",
"unicorn/filename-case": "error",
"unicorn/import-index": "error",
"unicorn/import-style": "error",
"unicorn/new-for-builtins": "error",
"unicorn/no-abusive-eslint-disable": "error",
"unicorn/no-array-instanceof": "error",
Expand Down Expand Up @@ -95,6 +96,7 @@ Configure it in `package.json`.
- [explicit-length-check](docs/rules/explicit-length-check.md) - Enforce explicitly comparing the `length` property of a value. *(partly fixable)*
- [filename-case](docs/rules/filename-case.md) - Enforce a case style for filenames.
- [import-index](docs/rules/import-index.md) - Enforce importing index files with `.`. *(fixable)*
- [import-style](docs/rules/import-style.md) - Whether to allow default imports or destructuring/named imports
- [new-for-builtins](docs/rules/new-for-builtins.md) - Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. *(fixable)*
- [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments.
- [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)*
Expand Down
93 changes: 93 additions & 0 deletions rules/import-style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
'use strict';
const getDocsUrl = require('./utils/get-docs-url');

const declarationHandler = (context, node, options) => {
const moduleName = getModuleName(node);
const moduleImportType = getModuleImportType(node);

for (const importStyle in options) {
if ({}.hasOwnProperty.call(options, importStyle)) {
for (const importFile in options[importStyle]) {
if (importFile === moduleName && moduleImportType === 'namedImport' && importStyle === 'defaultExport' && options[importStyle][importFile] === true) {
context.report({
node,
message: `Do not make named import for ${moduleName}`
});
} else if (importFile === moduleName && moduleImportType === 'defaultImport' && importStyle === 'namedExport' && options[importStyle][importFile] === true) {
context.report({
node,
message: `Do not make default import for ${moduleName}`
});
}
}
}
}
};

function getModuleName(node) {
if (node.type === 'VariableDeclaration') {
return node.declarations[0].init.arguments[0].value;
}

return node.source.value;
}

function getModuleImportType(node) {
if (node.type === 'VariableDeclaration') {
if (node.declarations[0].id.type === 'ObjectPattern') {
return 'namedImport';
}

return 'defaultImport';
}

if (node.specifiers[0].type === 'ImportSpecifier') {
return 'namedImport';
}

return 'defaultImport';
}

const create = context => {
const options = {
defaultExport: {
path: true,
chalk: true,
...(context.options[0] && context.options[0].defaultExport)
},
namedExport: {
util: true,
lodash: true,
underscore: true,
...(context.options[0] && context.options[0].namedExport)
}
};
return {
VariableDeclaration: node => declarationHandler(context, node, options),
ImportDeclaration: node => declarationHandler(context, node, options)
};
};

const schema = [{
type: 'object',
properties: {
defaultExport: {
type: 'object'
},
namedExport: {
type: 'object'
}
}
}];

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocsUrl(__filename)
},
fixable: 'code',
schema
}
};
88 changes: 88 additions & 0 deletions test/import-style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/import-style';

const ruleTester = avaRuleTester(test, {
parserOptions: {
ecmaVersion: 2019,
sourceType: 'module'
},
env: {
es6: true
}
});

function buildError({moduleName, type}) {
const error = {
ruleId: 'import-style'
};

if (type === 'defaultImport') {
error.message = `Do not make default import for ${moduleName}`;
return error;
}

error.message = `Do not make named import for ${moduleName}`;
return error;
}

ruleTester.run('import-style', rule, {
valid: [
{
code: 'const {promisify} = require(\'util\');',
options: [
{
defaultExport: {
path: false
},
namedExport: {
util: false
}
}
]
},
'import {promisify} from \'util\';',
'const file = require(\'unrestricted\');',
'import file from \'unrestricted\';',
'import {promisify as foo} from \'util\';',
'import {debuglog, promisify} from \'util\';',
'const {promisify : foo} = require(\'util\');'
],
invalid: [
{
code: 'const util = require(\'util\');',
errors: [buildError({moduleName: 'util', type: 'defaultImport'})]
},
{
code: 'import util from \'util\';',
errors: [buildError({moduleName: 'util', type: 'defaultImport'})]
},
{
code: 'import * as util from \'util\';',
errors: [buildError({moduleName: 'util', type: 'defaultImport'})]
},
{
code: 'import {something} from \'path\';',
errors: [buildError({moduleName: 'path', type: 'namedImport'})]
},
{
code: 'import util, {promisify} from \'util\';',
errors: [buildError({moduleName: 'util', type: 'defaultImport'})]
},
{
code: 'const foo = myFunction(\'util\')',
Copy link
Collaborator

@fisker fisker Sep 25, 2019

Choose a reason for hiding this comment

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

Why this is error?

Copy link
Author

Choose a reason for hiding this comment

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

Oh! Thanks for the review. I was a little busy in academic work. Yeah, it shouldn't be error, I'll add the check to throw error only in case of require().

errors: [buildError({moduleName: 'util', type: 'defaultImport'})]
},
{
code: 'import {something} from \'restricted\';',
options: [
{
defaultExport: {
restricted: true
}
}
],
errors: [buildError({moduleName: 'restricted', type: 'namedImport'})]
}
]
});