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

[Fix] extensions: handle . and .. properly #2778

Merged
merged 1 commit into from Jul 26, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Expand Up @@ -12,8 +12,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [Perf] `ExportMap`: Improve `ExportMap.for` performance on larger codebases ([#2756], thanks [@leipert])
- [`no-extraneous-dependencies`]/TypeScript: do not error when importing inline type from dev dependencies ([#1820], thanks [@andyogo])
- [`newline-after-import`]/TypeScript: do not error when re-exporting a namespaced import ([#2832], thanks [@laurens-dg])
* [`order`]: partial fix for [#2687] (thanks [@ljharb])
- [`order`]: partial fix for [#2687] (thanks [@ljharb])
- [`no-duplicates`]: Detect across type and regular imports ([#2835], thanks [@benkrejci])
- [`extensions`]: handle `.` and `..` properly ([#2778], thanks [@benasher44])

### Changed
- [Docs] [`no-duplicates`]: fix example schema ([#2684], thanks [@simmo])
Expand Down Expand Up @@ -1076,6 +1077,7 @@ for info on changes for earlier releases.

[#2835]: https://github.com/import-js/eslint-plugin-import/pull/2835
[#2832]: https://github.com/import-js/eslint-plugin-import/pull/2832
[#2778]: https://github.com/import-js/eslint-plugin-import/pull/2778
[#2756]: https://github.com/import-js/eslint-plugin-import/pull/2756
[#2754]: https://github.com/import-js/eslint-plugin-import/pull/2754
[#2748]: https://github.com/import-js/eslint-plugin-import/pull/2748
Expand Down Expand Up @@ -1649,6 +1651,7 @@ for info on changes for earlier releases.
[@BarryThePenguin]: https://github.com/BarryThePenguin
[@be5invis]: https://github.com/be5invis
[@beatrizrezener]: https://github.com/beatrizrezener
[@benasher44]: https://github.com/benasher44
[@benkrejci]: https://github.com/benkrejci
[@benmosher]: https://github.com/benmosher
[@benmunro]: https://github.com/benmunro
Expand Down
1 change: 1 addition & 0 deletions src/rules/extensions.js
Expand Up @@ -130,6 +130,7 @@ module.exports = {
}

function isExternalRootModule(file) {
if (file === '.' || file === '..') { return false; }
const slashCount = file.split('/').length - 1;

if (slashCount === 0) { return true; }
Expand Down
Empty file.
1 change: 1 addition & 0 deletions tests/index.js
@@ -0,0 +1 @@
export * from './files';
49 changes: 49 additions & 0 deletions tests/src/rules/extensions.js
Expand Up @@ -266,6 +266,26 @@ ruleTester.run('extensions', rule, {
],
}),

test({
code: [
'import barjs from "."',
'import barjs2 from ".."',
].join('\n'),
options: [ 'always' ],
errors: [
{
message: 'Missing file extension "js" for "."',
line: 1,
column: 19,
},
{
message: 'Missing file extension "js" for ".."',
line: 2,
column: 20,
},
],
}),

test({
code: [
'import barjs from "./bar.js"',
Expand Down Expand Up @@ -594,6 +614,35 @@ ruleTester.run('extensions', rule, {
},
],
}),

// TODO: properly ignore packages resolved via relative imports
test({
code: [
'import * as test from "."',
].join('\n'),
filename: testFilePath('./internal-modules/test.js'),
options: [ 'ignorePackages' ],
errors: [
{
message: 'Missing file extension for "."',
line: 1,
},
],
}),
// TODO: properly ignore packages resolved via relative imports
test({
code: [
'import * as test from ".."',
].join('\n'),
filename: testFilePath('./internal-modules/plugins/plugin.js'),
options: [ 'ignorePackages' ],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about whether these should be always. I think with ignorePackages it should definitely not error, so it makes for a less ambiguous test case.

In the always + relative case it's a bit more ambiguous to me, since if we assume the relative path is in your project, you might have this set to always because you always want someone to choose a file (within the project).

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what to do here, tbh.

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to merge this, and please file followup issues and/or PRs :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #2844!

errors: [
{
message: 'Missing file extension for ".."',
line: 1,
},
],
}),
],
});

Expand Down