Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Commit

Permalink
Merge pull request #138 from liferay/wincent/import-extensions
Browse files Browse the repository at this point in the history
feat!: add import-extensions rule
  • Loading branch information
wincent authored Jan 31, 2020
2 parents cc33cc1 + 9274501 commit e0452ed
Show file tree
Hide file tree
Showing 6 changed files with 270 additions and 0 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ If we were to provide configuration by default, then if `bottom-level/.eslintrc.
| [liferay-portal/no-side-navigation](./plugins/eslint-plugin-liferay-portal/docs/rules/no-side-navigation.md) | liferay/portal | [\#44](https://github.com/liferay/eslint-config-liferay/pull/44) |
| [liferay/destructure-requires](./plugins/eslint-plugin-liferay/docs/rules/destructure-requires.md) | liferay | [\#94](https://github.com/liferay/eslint-config-liferay/issues/94) |
| [liferay/group-imports](./plugins/eslint-plugin-liferay/docs/rules/group-imports.md) | liferay | [\#60](https://github.com/liferay/liferay-frontend-guidelines/issues/60) |
| [liferay/import-extensions](./plugins/eslint-plugin-liferay/docs/rules/import-extensions.md) | liferay | [\#137](https://github.com/liferay/eslint-config-liferay/issues/137) |
| [liferay/imports-first](./plugins/eslint-plugin-liferay/docs/rules/imports-first.md) | liferay | [\#60](https://github.com/liferay/liferay-frontend-guidelines/issues/60) |
| [liferay/no-absolute-import](./plugins/eslint-plugin-liferay/docs/rules/no-absolute-import.md) | liferay | [\#60](https://github.com/liferay/liferay-frontend-guidelines/issues/60) |
| [liferay/no-duplicate-class-names](./plugins/eslint-plugin-liferay/docs/rules/no-duplicate-class-names.md) | liferay | [\#108](https://github.com/liferay/eslint-config-liferay/issues/108) |
Expand Down Expand Up @@ -163,6 +164,7 @@ The bundled `eslint-plugin-liferay` plugin includes the following [rules](./plug

- [liferay/destructure-requires](./plugins/eslint-plugin-liferay/docs/rules/destructure-requires.md): Enforces (and autofixes) that `require` statements use destructuring.
- [liferay/group-imports](./plugins/eslint-plugin-liferay/docs/rules/group-imports.md): Enforces (and autofixes) `import` and `require` grouping.
- [liferay/import-extensions](./plugins/eslint-plugin-liferay/docs/rules/import-extensions.md): Enforces consistent usage/omission of file extensions in imports.
- [liferay/imports-first](./plugins/eslint-plugin-liferay/docs/rules/imports-first.md): Enforces that imports come first in the file.
- [liferay/no-absolute-import](./plugins/eslint-plugin-liferay/docs/rules/no-absolute-import.md): Enforces that imports do not use absolute paths.
- [liferay/no-duplicate-class-names](./plugins/eslint-plugin-liferay/docs/rules/no-duplicate-class-names.md): Enforces (and autofixes) uniqueness of class names inside JSX `className` attributes.
Expand Down
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const config = {
'default-case': 'error',
'liferay/destructure-requires': 'error',
'liferay/group-imports': 'error',
'liferay/import-extensions': 'error',
'liferay/imports-first': 'error',
'liferay/no-absolute-import': 'error',
'liferay/no-duplicate-imports': 'error',
Expand Down
41 changes: 41 additions & 0 deletions plugins/eslint-plugin-liferay/docs/rules/import-extensions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Omit extensions consistently with `import` and `require` (import-extensions)

This rule enforces that `import` statements and `require` calls — henceforth referred to as just "imports" — use (or omit) file extensions consistently.

## Rule Details

This rule complains if it finds an import with an unnecessary ".js" extension.

In its current form, it follows some simple heuristics and is not configurable (unlike more complex rules, such as the third-party [imports/extensions rule](https://github.com/benmosher/eslint-plugin-import/blob/HEAD/docs/rules/extensions.md)):

- Imports should omit the unnecessary ".js" extension.
- NPM package names (which may end in ".js" are exempted).

Based on the [current usages in liferay-portal](https://gist.github.com/wincent/1a6bbd06aec797032b6918153bef5d87) (via `git grep "import.+\\.js';" -- '*.js'`) and [clay](https://gist.github.com/wincent/775fdb7a0bc117c2fa8c66cd97b2d76f) (via `git grep "import.+\\.(js|ts|tsx)';" -- '*.ts' '*.tsx' '*.js'`) we believe this simpler approach should be sufficient, but we can add configurability in the future if that proves not to be the case.

## Examples

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

```js
import templates from './Something.soy.js';

import {Util} from './Util.es.js';

import * as Billboard from './billboard.js';
```

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

```js
import templates from './Something.soy';

import {Util} from './Util.es';

// OK because "billboard.js" is the name of an NPM package:
import {Data} from 'billboard.js';
```

## See also

- https://github.com/liferay/eslint-config-liferay/issues/137
1 change: 1 addition & 0 deletions plugins/eslint-plugin-liferay/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module.exports = {
rules: {
'destructure-requires': require('./lib/rules/destructure-requires'),
'group-imports': require('./lib/rules/group-imports'),
'import-extensions': require('./lib/rules/import-extensions'),
'imports-first': require('./lib/rules/imports-first'),
'no-absolute-import': require('./lib/rules/no-absolute-import'),
'no-duplicate-class-names': require('./lib/rules/no-duplicate-class-names'),
Expand Down
107 changes: 107 additions & 0 deletions plugins/eslint-plugin-liferay/lib/rules/import-extensions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* © 2017 Liferay, Inc. <https://liferay.com>
*
* SPDX-License-Identifier: MIT
*/

const path = require('path');

const {isLocal} = require('../common/imports');

const BAD_EXTENSIONS = new Set(['.js']);

function stripExtension(value) {
if (isLocal(value)) {
const extension = path.extname(value);

if (BAD_EXTENSIONS.has(extension)) {
return value.slice(0, -extension.length);
}
}

return value;
}

const message = 'unnecessary extension in import';

module.exports = {
create(context) {
function fix(node) {
let delimiter;
let original;
let stripped;

if (node.type === 'TemplateLiteral' && node.quasis.length === 1) {
// Only deal with template literals if they are static (ie. no
// interpolation).
delimiter = '`';
original = node.quasis[0].value.raw;
stripped = stripExtension(original);
} else if (node.type === 'Literal') {
delimiter = node.raw[0];
original = node.value;
stripped = stripExtension(original);
} else {
return;
}

if (stripped !== original) {
context.report({
fix: fixer =>
fixer.replaceText(
node,
`${delimiter}${stripped}${delimiter}`
),
message,
node,
});
}
}

function check(node) {
if (!node) {
return;
}

if (
node.type === 'ImportDeclaration' &&
(node.source.type === 'Literal' ||
node.source.type === 'TemplateLiteral')
) {
fix(node.source);
} else if (
node.type === 'CallExpression' &&
node.arguments &&
node.arguments.length &&
(node.arguments[0].type === 'Literal' ||
node.arguments[0].type === 'TemplateLiteral')
) {
fix(node.arguments[0]);
}
}

return {
CallExpression(node) {
if (node.callee.name === 'require') {
check(node);
}
},

ImportDeclaration(node) {
check(node);
},
};
},

meta: {
docs: {
category: 'Best Practices',
description: 'imports should use/omit extensions consistently',
recommended: false,
url: 'https://github.com/liferay/eslint-config-liferay/issues/137',
},
fixable: 'code',
schema: [],
type: 'problem',
},
};
118 changes: 118 additions & 0 deletions plugins/eslint-plugin-liferay/tests/lib/rules/import-extensions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/**
* © 2017 Liferay, Inc. <https://liferay.com>
*
* SPDX-License-Identifier: MIT
*/

const {RuleTester} = require('eslint');

const rule = require('../../../lib/rules/import-extensions');

const parserOptions = {
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
},
};

const ruleTester = new RuleTester(parserOptions);

const message = 'unnecessary extension in import';

const type = 'Literal';

const errors = [
{
message,
type,
},
];

ruleTester.run('import-extensions', rule, {
invalid: [
{
code: `import templates from './Something.soy.js';`,
errors,
output: `import templates from './Something.soy';`,
},
{
code: `import {Util} from './Util.es.js';`,
errors,
output: `import {Util} from './Util.es';`,
},
{
code: `import * as Billboard from './billboard.js';`,
errors,
output: `import * as Billboard from './billboard';`,
},
{
code: `const templates = require('./Something.soy.js');`,
errors,
output: `const templates = require('./Something.soy');`,
},
{
code: `const {Util} = require('./Util.es.js');`,
errors,
output: `const {Util} = require('./Util.es');`,
},
{
code: `const Billboard = require('./billboard.js');`,
errors,
output: `const Billboard = require('./billboard');`,
},
{
// Double quote delimiters are preserved.
code: `const Billboard = require("./billboard.js");`,
errors,
output: `const Billboard = require("./billboard");`,
},
{
// Backtick delimiters are preserved.
code: `const Billboard = require(\`./billboard.js\`);`,
errors: [
{
message,
type: 'TemplateLiteral',
},
],
output: `const Billboard = require(\`./billboard\`);`,
},
],

valid: [
{
code: `import templates from './Something.soy';`,
},
{
code: `import {Util} from './Util.es';`,
},
{
// OK because "billboard.js" is the name of an NPM package:
code: `import {Data} from 'billboard.js';`,
},
{
code: `const templates = require('./Something.soy');`,
},
{
code: `const {Util} = require('./Util.es');`,
},
{
code: `const {Data} = require('billboard.js');`,
},
{
// Note double quotes.
code: `const {Data} = require("billboard.js");`,
},
{
// Note backticks...
code: `const {Data} = require(\`billboard.js\`);`,
},
{
// ...but we don't look at backticks if they contain interpolation.
code: `const {Data} = require(\`./\${something}.js\`);`,
},
{
code: `const billboard = not_a_require('billboard.js');`,
},
],
});

0 comments on commit e0452ed

Please sign in to comment.