No `SymbolId` for `declare class C {}`

[overlookmotel]

Semantic does not produce a SymbolId for C in declare class C {}.

I believe it should - all BindingIdentifiers should have a SymbolId, and we do produce SymbolIds for other TS-only identifiers e.g. type N = number;.

In this example, only A gets assigned a SymbolId:

class A {}
declare class B {}
export declare class C {}

Playground

Relevant code:

oxc/crates/oxc_semantic/src/binder.rs

Lines 103 to 118 in ff2a1d4

	fn bind(&self, builder: &mut SemanticBuilder) {
	if !self.declare {
	let Some(ident) = &self.id else { return };
	let symbol_id = builder.declare_symbol(
	ident.span,
	&ident.name,
	SymbolFlags::Class,
	if self.is_declaration() {
	SymbolFlags::ClassExcludes
	} else {
	SymbolFlags::empty()
	},
	);
	ident.symbol_id.set(Some(symbol_id));
	}
	}

[Boshen]

I couldn't figure out where in binder.ts permits

declare class F {}
function F() {}

but not

class F {}
function F() {}

I need to debug binder.ts :-/

[Dunqing]

We need this to remove declare binding from export {}. See rolldown/rolldown#3048 (comment)

[Dunqing]

Ditto for the declare function

[ShuiRuTian]

I had a try to fix declare class by #8038

Here is how binder.ts handles this(binder is rare to be changed, should be fine to assume the line is not changed):

The logic is not special. In fact, oxc chose to skip bind declare class somehow, delete this related logic.

Will fix function in another PR if no one is interested/has time for it :)

[ShuiRuTian]

Another try to bind for declare function is #8039

However, there is still a missed function type called TSEmptyBodyFunctionExpression, which confused me. What is this? I do not quite understand why it's only needed in eslint-typescript....

[ShuiRuTian]

Not sure whether this is the right place to discuss detials...

1. feels like Ambient should not be a member of SymbolFlags, a symbol might be decalred through many declarations, which might be ambient or not. I think it's why Typescript add such flag to just Node(a term used by tsc, which is similar to AST node)
2. A symbol might have multi valid declarations in TS, but current SymbolTable seems to be designed for JS.

In my opinion, they are problems need to be resolved for supporting TS, waiting for options from core team members...

also cc @EliLichtblau, for who introduced "Ambient" as a member of "SymbolFlags"

[EliLichtblau]

Yea - I wonder if ambient was in the definition of symbolFlags at the time I made the PR in tsc but I agree ambient should be associated with a node rather than a symbol