[Fix] importType: properly resolve @/*-aliased imports as internal

Author: ombene

CHANGELOG.md

@@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 ### Fixed
 - `importType`: avoid crashing on a non-string' ([#2305], thanks [@ljharb])
 - [`first`]: prevent crash when parsing angular templates ([#2210], thanks [@ljharb])
+- `importType`: properly resolve `@/*`-aliased imports as internal ([#2334], thanks [@ombene])
 
 ### Changed
 - [`no-default-import`]: report on the token "default" instead of the entire node ([#2299], thanks [@pmcelhaney])
@@ -950,6 +951,7 @@ for info on changes for earlier releases.
 
 [`memo-parser`]: ./memo-parser/README.md
 
+[#2334]: https://github.com/import-js/eslint-plugin-import/pull/2334
 [#2305]: https://github.com/import-js/eslint-plugin-import/pull/2305
 [#2299]: https://github.com/import-js/eslint-plugin-import/pull/2299
 [#2297]: https://github.com/import-js/eslint-plugin-import/pull/2297
@@ -1571,6 +1573,7 @@ for info on changes for earlier releases.
 [@noelebrun]: https://github.com/noelebrun
 [@ntdb]: https://github.com/ntdb
 [@nwalters512]: https://github.com/nwalters512
+[@ombene]: https://github.com/ombene
 [@ota-meshi]: https://github.com/ota-meshi
 [@panrafal]: https://github.com/panrafal
 [@paztis]: https://github.com/paztis

src/core/importType.js

@@ -13,6 +13,11 @@ function baseModule(name) {
   return pkg;
 }
 
+function isInternalRegexMatch(name, settings) {
+  const internalScope = (settings && settings['import/internal-regex']);
+  return internalScope && new RegExp(internalScope).test(name);
+}
+
 export function isAbsolute(name) {
   return typeof name === 'string' && nodeIsAbsolute(name);
 }
@@ -25,33 +30,18 @@ export function isBuiltIn(name, settings, path) {
   return isCoreModule(base) || extras.indexOf(base) > -1;
 }
 
-export function isExternalModule(name, settings, path, context) {
-  if (arguments.length < 4) {
-    throw new TypeError('isExternalModule: name, settings, path, and context are all required');
+export function isExternalModule(name, path, context) {
+  if (arguments.length < 3) {                                                                                                                                                                              
+    throw new TypeError('isExternalModule: name, path, and context are all required');
   }
-  return (isModule(name) || isScoped(name)) && isExternalPath(name, settings, path, getContextPackagePath(context));
-}
-
-export function isExternalModuleMain(name, settings, path, context) {
-  return isModuleMain(name) && isExternalPath(name, settings, path, getContextPackagePath(context));
+  return (isModule(name) || isScoped(name)) && typeTest(name, context, path) === 'external';
 }
 
-function isExternalPath(name, settings, path, packagePath) {
-  const internalScope = (settings && settings['import/internal-regex']);
-  if (internalScope && new RegExp(internalScope).test(name)) {
-    return false;
-  }
-
-  if (!path || relative(packagePath, path).startsWith('..')) {
-    return true;
+export function isExternalModuleMain(name, path, context) {
+  if (arguments.length < 3) {                                                                                                                                                                              
+    throw new TypeError('isExternalModule: name, path, and context are all required');
   }
-
-  const folders = (settings && settings['import/external-module-folders']) || ['node_modules'];
-  return folders.some((folder) => {
-    const folderPath = nodeResolve(packagePath, folder);
-    const relativePath = relative(folderPath, path);
-    return !relativePath.startsWith('..');
-  });
+  return isModuleMain(name) && typeTest(name, context, path) === 'external';
 }
 
 const moduleRegExp = /^\w/;
@@ -87,17 +77,49 @@ function isRelativeToSibling(name) {
   return /^\.[\\/]/.test(name);
 }
 
-function typeTest(name, context, path) {
+function isExternalPath(path, context) {
+  if (!path) {
+    return false;
+  }
+
+  const { settings } = context;
+  const packagePath = getContextPackagePath(context);
+
+  if (relative(packagePath, path).startsWith('..')) {
+    return true;
+  }
+
+  const folders = (settings && settings['import/external-module-folders']) || ['node_modules'];
+  return folders.some((folder) => {
+    const folderPath = nodeResolve(packagePath, folder);
+    const relativePath = relative(folderPath, path);
+    return !relativePath.startsWith('..');
+  });
+}
+
+function isInternalPath(path, context) {
+  if (!path) {
+    return false;
+  }
+  const packagePath = getContextPackagePath(context);
+  return !relative(packagePath, path).startsWith('../');
+}
+
+function isExternalLookingName(name) {
+  return isModule(name) || isScoped(name);
+}
+
+function typeTest(name, context, path ) {
   const { settings } = context;
+  if (isInternalRegexMatch(name, settings)) { return 'internal'; }
   if (isAbsolute(name, settings, path)) { return 'absolute'; }
   if (isBuiltIn(name, settings, path)) { return 'builtin'; }
-  if (isModule(name, settings, path) || isScoped(name, settings, path)) {
-    const packagePath = getContextPackagePath(context);
-    return isExternalPath(name, settings, path, packagePath) ? 'external' : 'internal';
-  }
   if (isRelativeToParent(name, settings, path)) { return 'parent'; }
   if (isIndex(name, settings, path)) { return 'index'; }
   if (isRelativeToSibling(name, settings, path)) { return 'sibling'; }
+  if (isExternalPath(path, context)) { return 'external'; }
+  if (isInternalPath(path, context)) { return 'internal'; }
+  if (isExternalLookingName(name)) { return 'external'; }
   return 'unknown';
 }
 

src/rules/extensions.js

@@ -159,7 +159,6 @@ module.exports = {
       // determine if this is a module
       const isPackage = isExternalModule(
         importPath,
-        context.settings,
         resolve(importPath, context),
         context,
       ) || isScoped(importPath);

src/rules/no-cycle.js

@@ -44,7 +44,6 @@ module.exports = {
     const maxDepth = typeof options.maxDepth === 'number' ? options.maxDepth : Infinity;
     const ignoreModule = (name) => options.ignoreExternal && isExternalModule(
       name,
-      context.settings,
       resolve(name, context),
       context,
     );

tests/src/core/importType.js

@@ -75,6 +75,17 @@ describe('importType(name)', function () {
     expect(importType('constants', pathContext)).to.equal('internal');
   });
 
+  it("should return 'internal' for aliased internal modules that are found, even if they are not discernible as scoped", function () {
+    // `@` for internal modules is a common alias and is different from scoped names.
+    // Scoped names are prepended with `@` (e.g. `@scoped/some-file.js`) whereas `@`
+    // as an alias by itelf is the full root name (e.g. `@/some-file.js`).
+    const alias = { '@': path.join(pathToTestFiles, 'internal-modules') };
+    const webpackConfig = { resolve: { alias } };
+    const pathContext = testContext({ 'import/resolver': { webpack: { config: webpackConfig } } });
+    expect(importType('@/api/service', pathContext)).to.equal('internal');
+    expect(importType('@/does-not-exist', pathContext)).to.equal('unknown');
+  });
+
   it("should return 'parent' for internal modules that go through the parent", function () {
     expect(importType('../foo', context)).to.equal('parent');
     expect(importType('../../foo', context)).to.equal('parent');
@@ -100,6 +111,7 @@ describe('importType(name)', function () {
   it("should return 'unknown' for any unhandled cases", function () {
     expect(importType('  /malformed', context)).to.equal('unknown');
     expect(importType('   foo', context)).to.equal('unknown');
+    expect(importType('-/no-such-path', context)).to.equal('unknown');
   });
 
   it("should return 'builtin' for additional core modules", function () {
@@ -233,20 +245,20 @@ describe('importType(name)', function () {
 
   it('`isExternalModule` works with windows directory separator', function () {
     const context = testContext();
-    expect(isExternalModule('foo', {}, 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true);
-    expect(isExternalModule('@foo/bar', {}, 'E:\\path\\to\\node_modules\\@foo\\bar', context)).to.equal(true);
-    expect(isExternalModule('foo', {
-      'import/external-module-folders': ['E:\\path\\to\\node_modules'],
-    }, 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true);
+    expect(isExternalModule('foo', 'E:\\path\\to\\node_modules\\foo', context)).to.equal(true);
+    expect(isExternalModule('@foo/bar', 'E:\\path\\to\\node_modules\\@foo\\bar', context)).to.equal(true);
+    expect(isExternalModule('foo', 'E:\\path\\to\\node_modules\\foo', testContext({
+      settings: { 'import/external-module-folders': ['E:\\path\\to\\node_modules'] },
+    }))).to.equal(true);
   });
 
   it('`isExternalModule` works with unix directory separator', function () {
     const context = testContext();
-    expect(isExternalModule('foo', {}, '/path/to/node_modules/foo', context)).to.equal(true);
-    expect(isExternalModule('@foo/bar', {}, '/path/to/node_modules/@foo/bar', context)).to.equal(true);
-    expect(isExternalModule('foo', {
-      'import/external-module-folders': ['/path/to/node_modules'],
-    }, '/path/to/node_modules/foo', context)).to.equal(true);
+    expect(isExternalModule('foo', '/path/to/node_modules/foo', context)).to.equal(true);
+    expect(isExternalModule('@foo/bar', '/path/to/node_modules/@foo/bar', context)).to.equal(true);
+    expect(isExternalModule('foo', '/path/to/node_modules/foo', testContext({
+      settings: { 'import/external-module-folders': ['/path/to/node_modules'] },
+    }))).to.equal(true);
   });
 
   it('correctly identifies scoped modules with `isScoped`', () => {

tests/src/rules/no-extraneous-dependencies.js

@@ -164,6 +164,23 @@ ruleTester.run('no-extraneous-dependencies', rule, {
       `,
       settings: { 'import/resolver': 'webpack' },
     }),
+
+    test({
+      code: 'import "@custom-internal-alias/api/service";',
+      settings: {
+        'import/resolver': {
+          webpack: {
+            config: {
+              resolve: {
+                alias: {
+                  '@custom-internal-alias': testFilePath('internal-modules'),
+                },
+              },
+            },
+          },
+        },
+      },
+    }),
   ],
   invalid: [
     test({

tests/src/rules/no-internal-modules.js

@@ -304,6 +304,30 @@ ruleTester.run('no-internal-modules', rule, {
         column: 8,
       } ],
     }),
+    test({
+      code: 'import "@/api/service";',
+      options: [ {
+        forbid: [ '**/api/*' ],
+      } ],
+      errors: [ {
+        message: 'Reaching to "@/api/service" is not allowed.',
+        line: 1,
+        column: 8,
+      } ],
+      settings: {
+        'import/resolver': {
+          webpack: {
+            config: {
+              resolve: {
+                alias: {
+                  '@': testFilePath('internal-modules'),
+                },
+              },
+            },
+          },
+        },
+      },
+    }),
     // exports
     test({
       code: 'export * from "./plugin2/index.js";\nexport * from "./plugin2/app/index"',

tests/src/rules/order.js

@@ -1,4 +1,4 @@
-import { test, getTSParsers, getNonDefaultParsers } from '../utils';
+import { test, getTSParsers, getNonDefaultParsers, testFilePath } from '../utils';
 
 import { RuleTester } from 'eslint';
 import eslintPkg from 'eslint/package.json';
@@ -847,6 +847,43 @@ ruleTester.run('order', rule, {
         ],
       }),
     ]),
+    // Using `@/*` to alias internal modules
+    test({
+      code: `
+        import fs from 'fs';
+
+        import express from 'express';
+
+        import service from '@/api/service';
+        
+        import fooParent from '../foo';
+        
+        import fooSibling from './foo';
+        
+        import index from './';
+        
+        import internalDoesNotExistSoIsUnknown from '@/does-not-exist';
+      `,
+      options: [
+        {
+          groups: ['builtin', 'external', 'internal', 'parent', 'sibling', 'index', 'unknown'],
+          'newlines-between': 'always',
+        },
+      ],
+      settings: {
+        'import/resolver': {
+          webpack: {
+            config: {
+              resolve: {
+                alias: {
+                  '@': testFilePath('internal-modules'),
+                },
+              },
+            },
+          },
+        },
+      },
+    }),
   ],
   invalid: [
     // builtin before external module (require)