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

[New] extensions: add auto fixer! #2593

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
| [consistent-type-specifier-style](docs/rules/consistent-type-specifier-style.md) | Enforce or ban the use of inline type-only markers for named imports. | | | | 🔧 | | |
| [dynamic-import-chunkname](docs/rules/dynamic-import-chunkname.md) | Enforce a leading comment with the webpackChunkName for dynamic imports. | | | | | | |
| [exports-last](docs/rules/exports-last.md) | Ensure all exports appear after other statements. | | | | | | |
| [extensions](docs/rules/extensions.md) | Ensure consistent use of file extension within the import path. | | | | | | |
| [extensions](docs/rules/extensions.md) | Ensure consistent use of file extension within the import path. | | | | 🔧 | | |
| [first](docs/rules/first.md) | Ensure all imports appear before other statements. | | | | 🔧 | | |
| [group-exports](docs/rules/group-exports.md) | Prefer named exports to be grouped together in a single export declaration | | | | | | |
| [imports-first](docs/rules/imports-first.md) | Replaced by `import/first`. | | | | 🔧 | | ❌ |
Expand Down
2 changes: 2 additions & 0 deletions docs/rules/extensions.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# import/extensions

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

Some file resolve algorithms allow you to omit the file extension within the import source path. For example the `node` resolver can resolve `./foo/bar` to the absolute path `/User/someone/foo/bar.js` because the `.js` extension is resolved automatically by default. Depending on the resolver you can configure more extensions to get resolved automatically.
Expand Down
17 changes: 16 additions & 1 deletion src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ module.exports = {
description: 'Ensure consistent use of file extension within the import path.',
url: docsUrl('extensions'),
},

fixable: 'code',
schema: {
anyOf: [
{
Expand Down Expand Up @@ -175,13 +175,28 @@ module.exports = {
node: source,
message:
`Missing file extension ${extension ? `"${extension}" ` : ''}for "${importPathWithQueryString}"`,
fix: extension && (fixer => {
const [start, end] = source.range;
return [
fixer.replaceTextRange([start + 1, end - 1], `${source.value}.${extension}`),
];
}),
});
}
} else if (extension) {
if (isUseOfExtensionForbidden(extension) && isResolvableWithoutExtension(importPath)) {
context.report({
node: source,
message: `Unexpected use of file extension "${extension}" for "${importPathWithQueryString}"`,
fix: (fixer) => {
const [start, end] = source.range;
const extensionIndex = source.value.lastIndexOf(`.${extension}`);
const specifierBeforeExt = source.value.slice(0, extensionIndex);
const specifierAfterExt = source.value.slice(extensionIndex + extension.length + 1);
return [
fixer.replaceTextRange([start + 1, end - 1], `${specifierBeforeExt}${specifierAfterExt}`),
];
},
});
}
}
Expand Down
106 changes: 105 additions & 1 deletion tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ ruleTester.run('extensions', rule, {
valid: [
test({ code: 'import a from "@/a"' }),
test({ code: 'import a from "a"' }),
test({ code: 'import dot from "./file.with.dot"' }),
test({
code: 'import dot from "./file.with.dot"',
// output: 'import dot from "./file.with.dot.js"',
}),
test({
code: 'import a from "a/index.js"',
options: [ 'always' ],
Expand All @@ -30,6 +33,11 @@ ruleTester.run('extensions', rule, {
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
// output: [
// 'import lib from "./bar.js"',
// 'import component from "./bar.jsx"',
// 'import data from "./bar.json"',
// ].join('\n'),
options: [ 'never' ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } },
}),
Expand Down Expand Up @@ -151,6 +159,7 @@ ruleTester.run('extensions', rule, {
invalid: [
test({
code: 'import a from "a/index.js"',
output: 'import a from "a/index"',
Copy link
Member

Choose a reason for hiding this comment

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

this isn't a safe autofix unless we can validate that a/index and a/index.js resolve to the same thing.

if we can't add that validation using custom resolvers, then this would need to be a suggestion instead of an autofix.

errors: [ {
message: 'Unexpected use of file extension "js" for "a/index.js"',
line: 1,
Expand All @@ -159,6 +168,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'import dot from "./file.with.dot"',
output: 'import dot from "./file.with.dot.js"',
options: [ 'always' ],
errors: [
{
Expand All @@ -173,6 +183,10 @@ ruleTester.run('extensions', rule, {
'import a from "a/index.js"',
'import packageConfig from "./package"',
].join('\n'),
output: [
'import a from "a/index"',
'import packageConfig from "./package.json"',
].join('\n'),
options: [ { json: 'always', js: 'never' } ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.json' ] } },
errors: [
Expand All @@ -194,6 +208,11 @@ ruleTester.run('extensions', rule, {
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
output: [
'import lib from "./bar"',
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
options: [ 'never' ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } },
errors: [
Expand All @@ -210,6 +229,11 @@ ruleTester.run('extensions', rule, {
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
output: [
'import lib from "./bar"',
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
options: [ { json: 'always', js: 'never', jsx: 'never' } ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } },
errors: [
Expand All @@ -226,6 +250,10 @@ ruleTester.run('extensions', rule, {
'import component from "./bar.jsx"',
'import data from "./bar.json"',
].join('\n'),
output: [
'import component from "./bar"',
'import data from "./bar.json"',
].join('\n'),
options: [ { json: 'always', js: 'never', jsx: 'never' } ],
settings: { 'import/resolve': { 'extensions': [ '.jsx', '.json', '.js' ] } },
errors: [
Expand All @@ -238,6 +266,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'import "./bar.coffee"',
output: 'import "./bar"',
errors: [
{
message: 'Unexpected use of file extension "coffee" for "./bar.coffee"',
Expand All @@ -255,6 +284,11 @@ ruleTester.run('extensions', rule, {
'import barjson from "./bar.json"',
'import barnone from "./bar"',
].join('\n'),
output: [
'import barjs from "./bar"',
'import barjson from "./bar.json"',
'import barnone from "./bar"',
].join('\n'),
options: [ 'always', { json: 'always', js: 'never', jsx: 'never' } ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } },
errors: [
Expand All @@ -272,6 +306,11 @@ ruleTester.run('extensions', rule, {
'import barjson from "./bar.json"',
'import barnone from "./bar"',
].join('\n'),
output: [
'import barjs from "./bar"',
'import barjson from "./bar.json"',
'import barnone from "./bar"',
].join('\n'),
options: [ 'never', { json: 'always', js: 'never', jsx: 'never' } ],
settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } },
errors: [
Expand All @@ -286,6 +325,7 @@ ruleTester.run('extensions', rule, {
// unresolved (#271/#295)
test({
code: 'import thing from "./fake-file.js"',
output: 'import thing from "./fake-file"',
options: [ 'never' ],
errors: [
{
Expand All @@ -309,6 +349,7 @@ ruleTester.run('extensions', rule, {

test({
code: 'import thing from "@name/pkg/test"',
output: 'import thing from "@name/pkg/test"',
options: [ 'always' ],
errors: [
{
Expand All @@ -321,6 +362,7 @@ ruleTester.run('extensions', rule, {

test({
code: 'import thing from "@name/pkg/test.js"',
output: 'import thing from "@name/pkg/test"',
options: [ 'never' ],
errors: [
{
Expand All @@ -342,6 +384,15 @@ ruleTester.run('extensions', rule, {
import chart from '@/configs/chart'
import express from 'express'
`,
output: `
import foo from './foo.js'
import bar from './bar.json'
import Component from './Component'
import baz from 'foo/baz'
import baw from '@scoped/baw/import'
import chart from '@/configs/chart'
import express from 'express'
`,
options: [ 'always', { ignorePackages: true } ],
errors: [
{
Expand All @@ -367,6 +418,15 @@ ruleTester.run('extensions', rule, {
import chart from '@/configs/chart'
import express from 'express'
`,
output: `
import foo from './foo.js'
import bar from './bar.json'
import Component from './Component'
import baz from 'foo/baz'
import baw from '@scoped/baw/import'
import chart from '@/configs/chart'
import express from 'express'
`,
options: [ 'ignorePackages' ],
errors: [
{
Expand All @@ -389,6 +449,12 @@ ruleTester.run('extensions', rule, {
import Component from './Component.jsx'
import express from 'express'
`,
output: `
import foo from './foo'
import bar from './bar.json'
import Component from './Component'
import express from 'express'
`,
errors: [
{
message: 'Unexpected use of file extension "js" for "./foo.js"',
Expand All @@ -409,6 +475,11 @@ ruleTester.run('extensions', rule, {
import bar from './bar.json'
import Component from './Component.jsx'
`,
output: `
import foo from './foo.js'
import bar from './bar.json'
import Component from './Component'
`,
errors: [
{
message: 'Unexpected use of file extension "jsx" for "./Component.jsx"',
Expand All @@ -425,6 +496,10 @@ ruleTester.run('extensions', rule, {
'export { foo } from "./foo"',
'let bar; export { bar }',
].join('\n'),
output: [
'export { foo } from "./foo"',
'let bar; export { bar }',
].join('\n'),
options: [ 'always' ],
errors: [
{
Expand All @@ -439,6 +514,10 @@ ruleTester.run('extensions', rule, {
'export { foo } from "./foo.js"',
'let bar; export { bar }',
].join('\n'),
output: [
'export { foo } from "./foo"',
'let bar; export { bar }',
].join('\n'),
options: [ 'never' ],
errors: [
{
Expand All @@ -452,6 +531,7 @@ ruleTester.run('extensions', rule, {
// Query strings.
test({
code: 'import withExtension from "./foo.js?a=True"',
output: 'import withExtension from "./foo?a=True"',
options: [ 'never' ],
errors: [
{
Expand All @@ -463,6 +543,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'import withoutExtension from "./foo?a=True.ext"',
output: 'import withoutExtension from "./foo?a=True.ext"',
options: [ 'always' ],
errors: [
{
Expand All @@ -478,6 +559,10 @@ ruleTester.run('extensions', rule, {
'const { foo } = require("./foo")',
'export { foo }',
].join('\n'),
output: [
'const { foo } = require("./foo")',
'export { foo }',
].join('\n'),
options: [ 'always' ],
errors: [
{
Expand All @@ -492,6 +577,10 @@ ruleTester.run('extensions', rule, {
'const { foo } = require("./foo.js")',
'export { foo }',
].join('\n'),
output: [
'const { foo } = require("./foo")',
'export { foo }',
].join('\n'),
options: [ 'never' ],
errors: [
{
Expand All @@ -505,6 +594,7 @@ ruleTester.run('extensions', rule, {
// export { } from
test({
code: 'export { foo } from "./foo"',
output: 'export { foo } from "./foo"',
options: [ 'always' ],
errors: [
{
Expand All @@ -519,6 +609,10 @@ ruleTester.run('extensions', rule, {
import foo from "@/ImNotAScopedModule";
import chart from '@/configs/chart';
`,
output: `
import foo from "@/ImNotAScopedModule";
import chart from '@/configs/chart';
`,
options: ['always'],
errors: [
{
Expand All @@ -533,6 +627,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'export { foo } from "./foo.js"',
output: 'export { foo } from "./foo"',
options: [ 'never' ],
errors: [
{
Expand All @@ -546,6 +641,7 @@ ruleTester.run('extensions', rule, {
// export * from
test({
code: 'export * from "./foo"',
output: 'export * from "./foo"',
options: [ 'always' ],
errors: [
{
Expand All @@ -557,6 +653,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'export * from "./foo.js"',
output: 'export * from "./foo"',
options: [ 'never' ],
errors: [
{
Expand All @@ -568,6 +665,7 @@ ruleTester.run('extensions', rule, {
}),
test({
code: 'import foo from "@/ImNotAScopedModule.js"',
output: 'import foo from "@/ImNotAScopedModule"',
options: ['never'],
errors: [
{
Expand All @@ -583,6 +681,12 @@ ruleTester.run('extensions', rule, {

import bar from './bar';
`,
output: `
import _ from 'lodash';
import m from '@test-scope/some-module/index';

import bar from './bar';
`,
options: ['never'],
settings: {
'import/resolver': 'webpack',
Expand Down