Report duplicate declarations for import bindings kept in TypeScript output

src/js_parser/p.rs

@@ -432,6 +432,13 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
     pub enclosing_class_keyword: bun_ast::Range,
     pub import_items_for_namespace: HashMap<Ref, ImportItemForNamespaceMap>,
     pub is_import_item: RefMap,
+    /// Import bindings whose name was re-declared by a later declaration in the
+    /// same scope. TypeScript allows the collision because the import may be
+    /// type-only and elided (see `Scope::can_merge_symbol_kinds`), but if the
+    /// import binding survives import elision it would be printed next to the
+    /// other declaration, so `ImportScanner::scan` reports a duplicate
+    /// declaration error using the re-declaration's location stored here.
+    pub redeclared_import_bindings: HashMap<Ref, bun_ast::Loc>,
     pub named_imports: NamedImportsType<'a>,
     pub named_exports: bun_ast::ast_result::NamedExports,
     pub import_namespace_cc_map: Map<ImportNamespaceCallOrConstruct, bool>,
@@ -4989,6 +4996,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         if kind.is_function() && self.symbols[symbol_idx].kind.is_function() {
                             self.symbols[symbol_idx].remove_overwritten_function_declaration = true;
                         }
+
+                        // In TypeScript, an import binding may be silently re-declared
+                        // because the import might be type-only and elided (see
+                        // `Scope::can_merge_symbol_kinds`). Remember the collision so
+                        // `ImportScanner::scan` can report a duplicate declaration error
+                        // if the import binding is actually kept in the output.
+                        if TYPESCRIPT
+                            && self.symbols[symbol_idx].kind == js_ast::symbol::Kind::Import
+                        {
+                            self.redeclared_import_bindings.insert(existing.ref_, loc);
+                        }
                     }
                     MR::BecomePrivateGetSetPair => {
                         ref_ = existing.ref_;
@@ -9301,6 +9319,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             enclosing_class_keyword: bun_ast::Range::NONE,
             import_items_for_namespace: Default::default(),
             is_import_item: Default::default(),
+            redeclared_import_bindings: Default::default(),
             import_namespace_cc_map: Default::default(),
             scope_order_to_visit: &[],
             module_scope_directive_loc: bun_ast::Loc::default(),

src/js_parser/scan/scan_imports.rs

@@ -322,6 +322,47 @@
                         st.star_name_loc = None;
                     }
 
+                    // In TypeScript, a later declaration is allowed to re-declare the
+                    // name of an import binding on the assumption that the import is
+                    // type-only and will be elided (see `Scope::can_merge_symbol_kinds`).
+                    // Any such import binding that is still around at this point will be
+                    // printed next to the other declaration, so report the duplicate
+                    // declaration instead of emitting output that fails to re-parse.
+                    if is_typescript_enabled && !p.redeclared_import_bindings.is_empty() {
+                        let default_binding = st
+                            .default_name
+                            .map(|name| (name.ref_.expect("infallible: ref bound"), name.loc));
+                        let star_binding = st.star_name_loc.map(|loc| (st.namespace_ref, loc));
+                        let item_bindings = st.items.slice().iter().map(|item| {
+                            (
+                                item.name.ref_.expect("infallible: ref bound"),
+                                item.name.loc,
+                            )
+                        });
+
+                        for (name_ref, import_loc) in default_binding
+                            .into_iter()
+                            .chain(star_binding)
+                            .chain(item_bindings)
+                        {
+                            // `remove` so a second scan pass doesn't report it again.
+                            if let Some(redeclared_loc) =
+                                p.redeclared_import_bindings.remove(&name_ref)
+                            {
+                                // SAFETY: arena-owned slice valid for 'p.
+                                let name = p.symbols[name_ref.inner_index() as usize]
+                                    .original_name
+                                    .slice();
+                                p.log().add_symbol_already_declared_error(
+                                    p.source,
+                                    name,
+                                    redeclared_loc,
+                                    import_loc,
+                                );
+                            }
+                        }
+                    }
+
                     if st.default_name.is_some() {
                         record!()
                             .flags

test/bundler/transpiler/transpiler.test.js

@@ -927,6 +927,49 @@ class Test extends Bar {
       ts.expectPrinted_("export import Foo = Baz.Bar;", "export const Foo = Baz.Bar");
     });
 
+    it("re-declaring an import binding that is kept in the output is an error", () => {
+      const err = ts.expectParseError;
+
+      // TypeScript allows another declaration to take over the name of an
+      // import binding because the import may be type-only. These imports are
+      // not elided (trimUnusedImports defaults to false for Bun.Transpiler),
+      // so printing them would produce output that fails to re-parse. They
+      // must error instead.
+      err('import{Observable}from""\nimport{Observable} from "x"', '"Observable" has already been declared');
+      err('import { Foo } from "./x";\nexport class Foo {}', '"Foo" has already been declared');
+      err('import Foo from "./x";\nclass Foo {}', '"Foo" has already been declared');
+      err('import * as Foo from "./x";\nclass Foo {}', '"Foo" has already been declared');
+      err('import { Foo } from "./x";\nfunction Foo() {}', '"Foo" has already been declared');
+      err('import { Foo } from "./x";\nvar Foo = 1;', '"Foo" has already been declared');
+      err('import { Foo } from "./x";\nlet Foo = 1;', '"Foo" has already been declared');
+      err('import { Foo } from "./x";\nenum Foo {}', '"Foo" has already been declared');
+      err('import { Foo } from "./x";\nimport Foo = require("./y");', '"Foo" has already been declared');
+      err('import { Foo, Foo } from "./x";', '"Foo" has already been declared');
+    });
+
+    it("re-declaring an elided import binding is allowed", () => {
+      const exp = ts.expectPrinted_;
+
+      // Type-only imports and declarations that emit no code don't conflict
+      // with anything in the output, so they are still allowed to collide.
+      exp('import type { Foo } from "./x";\nclass Foo {}', "class Foo {\n}");
+      exp(
+        'import { type Foo, Bar } from "./x";\nclass Foo {}\nconsole.log(Bar);',
+        'import { Bar } from "./x";\n\nclass Foo {\n}\nconsole.log(Bar);\n',
+      );
+      exp('import { Foo } from "./x";\ndeclare class Foo {}\nnew Foo();', 'import { Foo } from "./x";\nnew Foo;\n');
+      exp('import { foo } from "./x";\nfunction foo(): void;', 'import { foo } from "./x";\n');
+
+      // When unused imports are trimmed (the default for the runtime and the
+      // bundler), the re-declared import is elided and there is no error.
+      const trimming = new Bun.Transpiler({ loader: "ts", trimUnusedImports: true });
+      expect(trimming.transformSync('import { Foo } from "./x";\nexport class Foo {}')).toBe("export class Foo {\n}\n");
+      expect(trimming.transformSync('import{Observable}from""\nimport{Observable} from "x"')).toBe("");
+      expect(trimming.transformSync('import { Foo } from "./x";\nnamespace Foo { export const x = 1 }')).toBe(
+        "var Foo;\n((Foo) => {\n  Foo.x = 1;\n})(Foo ||= {});\n",
+      );
+    });
+
     it("export = {foo: 123}", () => {
       ts.expectPrinted_("export = {foo: 123}", "module.exports = { foo: 123 }");
     });