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

TypeScript identifier resolver incorrectly handles block scopes #7967

Closed
eandre opened this issue Sep 18, 2023 · 5 comments · Fixed by #8403
Closed

TypeScript identifier resolver incorrectly handles block scopes #7967

eandre opened this issue Sep 18, 2023 · 5 comments · Fixed by #8403
Assignees
Labels
Milestone

Comments

@eandre
Copy link

eandre commented Sep 18, 2023

Describe the bug

I'm using swc_ecma_transforms_base::resolver to determine which declaration a given identifier refers to.

I'm defining multiple TypeScript interfaces with the same name, each within its own block (using a top-level BlockStmt). However, swc_ecma_transforms_base::resolver maps them to the same swc_ecma_ast::Id. See example below

Input code

TypeScript:

{
    interface Foo {}
}

{
    interface Foo {}
}

Rust:

    let globals = Globals::new();
    GLOBALS.set(&globals, || {
        let lexer = Lexer::new(
            Syntax::Typescript(Default::default()),
            EsVersion::Es2022,
            StringInput::from(file.as_ref()),
            None,
        );
        let mut parser = Parser::new_from(lexer);
        let ast = parser.parse_module().unwrap();

        let mut resolver = swc_ecma_transforms_base::resolver(Mark::new(), Mark::new(), true);
        let ast = ast.fold_with(&mut resolver);

        // Here I expect the two separate `Foo` idents to have different ids,
        // since they're in different blocks.
    });

Config

Using the Rust library.

Playground link

View the AST tab:
https://play.swc.rs/?version=1.3.74&code=H4sIAAAAAAAAA6vmUgCCzLyS1KK0xORUBbf8fIXqWq5aLq5qHDIAgwgOgDIAAAA%3D&config=H4sIAAAAAAAAA1VPOw7DIAzdOQXy3KFi6NA79BCIOhERAYQdqSjK3QsJpM1mv4%2Ff8yqkhIkMPOVaxrJEnQjTuReEsmf9KQhwjkgm2chw6yxTpQbtCHdoOxhgnUbk6kJSd6WaA1wIhN3RsNl6O%2BT%2FTBPmmJDoKqxS7UeH10TRUmEO72Un2y%2B179HgAT9RDzsPg6VXd3JaUGxfBMLf3xcBAAA%3D

Expected behavior

I expect the two Identifier AST nodes inside the TsInterfaceDeclaration nodes to have different ctxt values inside their spans.

Actual behavior

The two Identifier nodes have the same ctxt value 2, which means they get mapped to the same swc_ecma_ast::Id even though they're different types.

Version

1.3.85

@eandre eandre added the C-bug label Sep 18, 2023
@kdy1
Copy link
Member

kdy1 commented Sep 18, 2023

I think those are same

@kdy1 kdy1 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2023
@eandre
Copy link
Author

eandre commented Sep 18, 2023

I don't understand what you mean by "those are the same", can you elaborate?

Type definitions in TypeScript are block-scoped. For example:

foo.ts:

{
  interface Foo {}
}

let foo: Foo;
$ tsc --noEmit foo.ts
foo.ts:5:8 - error TS2304: Cannot find name 'Foo'.

5 let foo: Foo;
           ~~~
Found 1 error in foo.ts:5

I believe this should be treated the same as other definitions so that each interface declaration gets its own ctxt, similarly to class declarations and variable declarations, since the scoping rules are the same.

The swc_ecma_ast::Ident docs show a very similar example using variable declarations, which is being handled correctly:

/// ```ts
/// let a = 5
/// {
/// let a = 3;
/// }
/// ```
/// In the code above, there are two variables with the symbol a.

Why should interface declarations be treated differently?

@eandre
Copy link
Author

eandre commented Sep 20, 2023

Ping @kdy1, I'm very confused why this was closed. Can you elaborate on my questions above? I'm happy to contribute the change if you agree with my reasoning.

@kdy1 kdy1 reopened this Sep 21, 2023
@0f-0b
Copy link

0f-0b commented Nov 13, 2023

This also affects using declarations (playground).

{ using foo = null }
{ using foo = null }

@kdy1 kdy1 added this to the Planned milestone Nov 13, 2023
@kdy1 kdy1 self-assigned this Nov 13, 2023
@kdy1 kdy1 closed this as completed in #8403 Dec 8, 2023
kdy1 pushed a commit that referenced this issue Dec 8, 2023
@kdy1 kdy1 modified the milestones: Planned, v1.3.101 Dec 18, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Jan 17, 2024

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Jan 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

4 participants