[Fix] no-named-as-default: Allow using an identifier if the export …

…is both a named and a default export

 - add tests for import-js#1594

CHANGELOG.md

@@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 
 ### Fixed
 - `ExportMap` / flat config: include `languageOptions` in context ([#3052], thanks [@michaelfaith])
+- [`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export ([#3032], thanks [@akwodkiewicz])
 
 ## [2.30.0] - 2024-09-02
 
@@ -1139,6 +1140,7 @@ for info on changes for earlier releases.
 [#3043]: https://github.com/import-js/eslint-plugin-import/pull/3043
 [#3036]: https://github.com/import-js/eslint-plugin-import/pull/3036
 [#3033]: https://github.com/import-js/eslint-plugin-import/pull/3033
+[#3032]: https://github.com/import-js/eslint-plugin-import/pull/3032
 [#3018]: https://github.com/import-js/eslint-plugin-import/pull/3018
 [#3012]: https://github.com/import-js/eslint-plugin-import/pull/3012
 [#3011]: https://github.com/import-js/eslint-plugin-import/pull/3011
@@ -1726,6 +1728,7 @@ for info on changes for earlier releases.
 [@AdriAt360]: https://github.com/AdriAt360
 [@ai]: https://github.com/ai
 [@aks-]: https://github.com/aks-
+[@akwodkiewicz]: https://github.com/akwodkiewicz
 [@aladdin-add]: https://github.com/aladdin-add
 [@alex-page]: https://github.com/alex-page
 [@alexgorbatchev]: https://github.com/alexgorbatchev

src/rules/no-named-as-default.js

@@ -15,28 +15,71 @@ module.exports = {
 
   create(context) {
     function checkDefault(nameKey, defaultSpecifier) {
+      /**
+       * For ImportDefaultSpecifier we're interested in the "local" name (`foo` for `import {bar as foo} ...`)
+       * For ExportDefaultSpecifier we're interested in the "exported" name (`foo` for `export {bar as foo} ...`)
+       */
+      const analyzedName = defaultSpecifier[nameKey].name;
+
       // #566: default is a valid specifier
-      if (defaultSpecifier[nameKey].name === 'default') { return; }
+      if (analyzedName === 'default') { return; }
 
       const declaration = importDeclaration(context, defaultSpecifier);
+      /** @type {import('../exportMap').default | null} */
+      const importedModule = ExportMapBuilder.get(declaration.source.value, context);
+      if (importedModule == null) { return; }
 
-      const imports = ExportMapBuilder.get(declaration.source.value, context);
-      if (imports == null) { return; }
+      if (importedModule.errors.length > 0) {
+        importedModule.reportErrors(context, declaration);
+        return;
+      }
 
-      if (imports.errors.length) {
-        imports.reportErrors(context, declaration);
+      if (!importedModule.hasDefault) {
+        // The rule is triggered for default imports/exports, so if the imported module has no default
+        // this means we're dealing with incorrect source code anyway
         return;
       }
 
-      if (imports.has('default') && imports.has(defaultSpecifier[nameKey].name)) {
+      if (!importedModule.has(analyzedName)) {
+        // The name used locally for the default import was not even used in the imported module.
+        return;
+      }
 
-        context.report(
-          defaultSpecifier,
-          `Using exported name '${defaultSpecifier[nameKey].name}' as identifier for default export.`,
-        );
+      /**
+       * FIXME: We can verify if a default and a named export are pointing to the same symbol only
+       * if they are both `reexports`. In case one of the symbols is not a re-export, but defined
+       * in the file, the ExportMap structure has no info about what actually is being exported --
+       * the value in the `namespace` Map is an empty object.
+       *
+       * To solve this, it would require not relying on the ExportMap, but on some other way of
+       * accessing the imported module and its exported values.
+       *
+       * Additionally, although `ExportMap.get` is a unified way to get info from both `reexports`
+       * and `namespace` maps, it does not return valid output we need here, and I think this is
+       * related to the "cycle safeguards" in the `get` function.
+       */
 
+      if (importedModule.reexports.has(analyzedName) && importedModule.reexports.has('default')) {
+        const thingImportedWithNamedImport = importedModule.reexports.get(analyzedName).getImport();
+        const thingImportedWithDefaultImport = importedModule.reexports.get('default').getImport();
+
+        // Case: both imports point to the same file and they both refer to the same symbol in this file.
+        if (
+          thingImportedWithNamedImport.path === thingImportedWithDefaultImport.path
+          && thingImportedWithNamedImport.local === thingImportedWithDefaultImport.local
+        ) {
+          // #1594: the imported module exports the same thing via a default export and a named export
+          return;
+        }
       }
+
+      context.report(
+        defaultSpecifier,
+        `Using exported name '${defaultSpecifier[nameKey].name}' as identifier for default ${nameKey === 'local' ? `import` : `export`}.`,
+      );
+
     }
+
     return {
       ImportDefaultSpecifier: checkDefault.bind(null, 'local'),
       ExportDefaultSpecifier: checkDefault.bind(null, 'exported'),

tests/files/no-named-as-default/exports.js

@@ -0,0 +1,4 @@
+const variable = 1;
+
+export { variable };
+export default variable;

tests/files/no-named-as-default/misleading-re-exports.js

@@ -0,0 +1,2 @@
+export { variable as something } from './exports';
+export { something as default } from './something';

tests/files/no-named-as-default/no-default-export.js

@@ -0,0 +1 @@
+export const foobar = 4;

tests/files/no-named-as-default/re-exports.js

@@ -0,0 +1,2 @@
+export { something as default } from "./something";
+export { something } from "./something";

tests/files/no-named-as-default/something.js

@@ -0,0 +1 @@
+export const something = 42;

tests/src/rules/no-named-as-default.js

@@ -12,49 +12,105 @@ ruleTester.run('no-named-as-default', rule, {
     test({ code: 'import bar, { foo } from "./empty-folder";' }),
 
     // es7
-    test({ code: 'export bar, { foo } from "./bar";',
-      parser: parsers.BABEL_OLD }),
-    test({ code: 'export bar from "./bar";',
-      parser: parsers.BABEL_OLD }),
+    test({
+      code: 'export bar, { foo } from "./bar";',
+      parser: parsers.BABEL_OLD,
+    }),
+    test({
+      code: 'export bar from "./bar";',
+      parser: parsers.BABEL_OLD,
+    }),
 
     // #566: don't false-positive on `default` itself
-    test({ code: 'export default from "./bar";',
-      parser: parsers.BABEL_OLD }),
+    test({
+      code: 'export default from "./bar";',
+      parser: parsers.BABEL_OLD,
+    }),
 
     // es2022: Arbitrary module namespae identifier names
     testVersion('>= 8.7', () => ({
       code: 'import bar, { foo } from "./export-default-string-and-named"',
       parserOptions: { ecmaVersion: 2022 },
     })),
 
+    // #1594: Allow importing as default if object is exported both as default and named
+    test({ code: 'import something from "./no-named-as-default/re-exports.js";' }),
+    test({
+      code: 'import { something } from "./no-named-as-default/re-exports.js";',
+    }),
+    test({
+      code: 'import myOwnNameForVariable from "./no-named-as-default/exports.js";',
+    }),
+    test({
+      code: 'import { variable } from "./no-named-as-default/exports.js";',
+    }),
+    test({
+      code: 'import variable from "./no-named-as-default/misleading-re-exports.js";',
+    }),
+    test({
+      // incorrect import
+      code: 'import foobar from "./no-named-as-default/no-default-export.js";',
+    }),
+    // same tests, but for exports
+    test({
+      code: 'export something from "./no-named-as-default/re-exports.js";',
+      parser: parsers.BABEL_OLD,
+    }),
+    test({
+      code: 'export { something } from "./no-named-as-default/re-exports.js";',
+    }),
+    test({
+      code: 'export myOwnNameForVariable from "./no-named-as-default/exports.js";',
+      parser: parsers.BABEL_OLD,
+    }),
+    test({
+      code: 'export { variable } from "./no-named-as-default/exports.js";',
+    }),
+    test({
+      code: 'export variable from "./no-named-as-default/misleading-re-exports.js";',
+      parser: parsers.BABEL_OLD,
+    }),
+    test({
+      code: 'export foobar from "./no-named-as-default/no-default-export.js";',
+      parser: parsers.BABEL_OLD,
+    }),
+
     ...SYNTAX_CASES,
   ),
 
   invalid: [].concat(
     test({
       code: 'import foo from "./bar";',
       errors: [{
-        message: 'Using exported name \'foo\' as identifier for default export.',
-        type: 'ImportDefaultSpecifier' }] }),
+        message: 'Using exported name \'foo\' as identifier for default import.',
+        type: 'ImportDefaultSpecifier',
+      }],
+    }),
     test({
       code: 'import foo, { foo as bar } from "./bar";',
       errors: [{
-        message: 'Using exported name \'foo\' as identifier for default export.',
-        type: 'ImportDefaultSpecifier' }] }),
+        message: 'Using exported name \'foo\' as identifier for default import.',
+        type: 'ImportDefaultSpecifier',
+      }],
+    }),
 
     // es7
     test({
       code: 'export foo from "./bar";',
       parser: parsers.BABEL_OLD,
       errors: [{
         message: 'Using exported name \'foo\' as identifier for default export.',
-        type: 'ExportDefaultSpecifier' }] }),
+        type: 'ExportDefaultSpecifier',
+      }],
+    }),
     test({
       code: 'export foo, { foo as bar } from "./bar";',
       parser: parsers.BABEL_OLD,
       errors: [{
         message: 'Using exported name \'foo\' as identifier for default export.',
-        type: 'ExportDefaultSpecifier' }] }),
+        type: 'ExportDefaultSpecifier',
+      }],
+    }),
 
     test({
       code: 'import foo from "./malformed.js"',
@@ -68,18 +124,44 @@ ruleTester.run('no-named-as-default', rule, {
     testVersion('>= 8.7', () => ({
       code: 'import foo from "./export-default-string-and-named"',
       errors: [{
-        message: 'Using exported name \'foo\' as identifier for default export.',
+        message: 'Using exported name \'foo\' as identifier for default import.',
         type: 'ImportDefaultSpecifier',
       }],
       parserOptions: { ecmaVersion: 2022 },
     })),
     testVersion('>= 8.7', () => ({
       code: 'import foo, { foo as bar } from "./export-default-string-and-named"',
       errors: [{
-        message: 'Using exported name \'foo\' as identifier for default export.',
+        message: 'Using exported name \'foo\' as identifier for default import.',
         type: 'ImportDefaultSpecifier',
       }],
       parserOptions: { ecmaVersion: 2022 },
     })),
+
+    // #1594: Allow importing as default if object is exported both as default and named
+    test({
+      code: 'import something from "./no-named-as-default/misleading-re-exports.js";',
+      parser: parsers.BABEL_OLD,
+      errors: [{
+        message: 'Using exported name \'something\' as identifier for default import.',
+        type: 'ImportDefaultSpecifier',
+      }],
+    }),
+    // The only cases that are not covered by the fix
+    test({
+      code: 'import variable from "./no-named-as-default/exports.js";',
+      errors: [{
+        message: 'Using exported name \'variable\' as identifier for default import.',
+        type: 'ImportDefaultSpecifier',
+      }],
+    }),
+    test({
+      code: 'export variable from "./no-named-as-default/exports.js";',
+      parser: parsers.BABEL_OLD,
+      errors: [{
+        message: 'Using exported name \'variable\' as identifier for default export.',
+        type: 'ExportDefaultSpecifier',
+      }],
+    }),
   ),
 });