Skip to content

Commit 39e4fd5

Browse files
authored
refactor(module_graph): preserve same-name function overloads as a set (#10585)
1 parent 844b1be commit 39e4fd5

10 files changed

Lines changed: 202 additions & 21 deletions

File tree

crates/biome_js_semantic/src/events.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ impl SemanticEventExtractor {
576576
self.push_binding(hoisted_scope_id, BindingName::Value(name), info);
577577
}
578578
AnyJsBindingDeclaration::JsFunctionDeclaration(_) => {
579-
declaration_kind = JsDeclarationKind::HoistedValue;
579+
declaration_kind = JsDeclarationKind::Function;
580580
let is_in_strict_mode = self
581581
.scopes
582582
.last()
@@ -588,8 +588,12 @@ impl SemanticEventExtractor {
588588
};
589589
self.push_binding(hoisted_scope_id, BindingName::Value(name), info);
590590
}
591-
AnyJsBindingDeclaration::TsDeclareFunctionDeclaration(_)
592-
| AnyJsBindingDeclaration::TsDeclareFunctionExportDefaultDeclaration(_)
591+
AnyJsBindingDeclaration::TsDeclareFunctionDeclaration(_) => {
592+
declaration_kind = JsDeclarationKind::Function;
593+
hoisted_scope_id = self.scope_index_to_hoist_declarations(1);
594+
self.push_binding(hoisted_scope_id, BindingName::Value(name), info);
595+
}
596+
AnyJsBindingDeclaration::TsDeclareFunctionExportDefaultDeclaration(_)
593597
| AnyJsBindingDeclaration::JsFunctionExportDefaultDeclaration(_) => {
594598
declaration_kind = JsDeclarationKind::HoistedValue;
595599
hoisted_scope_id = self.scope_index_to_hoist_declarations(1);

crates/biome_js_semantic/src/semantic_model/binding.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,19 @@ pub enum JsDeclarationKind {
2525
/// Declares both a type and a value.
2626
Enum,
2727

28+
/// A `function` declaration or a `declare function` overload signature.
29+
///
30+
/// Declares only a value, and is hoisted to the function scope.
31+
Function,
32+
2833
/// A generic type parameter, declared in angle brackets.
2934
///
3035
/// For example: `<T>`.
3136
///
3237
/// Declares only a type.
3338
Generic,
3439

35-
/// A `function` or `var` declaration.
40+
/// A `var` declaration.
3641
///
3742
/// Declares only a value, and is hoisted to the function scope.
3843
HoistedValue,
@@ -110,6 +115,7 @@ impl JsDeclarationKind {
110115
self,
111116
Self::Class
112117
| Self::Enum
118+
| Self::Function
113119
| Self::HoistedValue
114120
| Self::Import
115121
| Self::Namespace
@@ -129,14 +135,14 @@ impl JsDeclarationKind {
129135
if let Some(declaration) = AnyJsDeclaration::cast_ref(&ancestor) {
130136
return match declaration {
131137
AnyJsDeclaration::JsClassDeclaration(_) => Self::Class,
132-
AnyJsDeclaration::JsFunctionDeclaration(_) => Self::HoistedValue,
138+
AnyJsDeclaration::JsFunctionDeclaration(_) => Self::Function,
133139
AnyJsDeclaration::JsVariableDeclaration(decl) => match decl.variable_kind() {
134140
Ok(JsVariableKind::Const | JsVariableKind::Let) => Self::Value,
135141
Ok(JsVariableKind::Using) => Self::Using,
136142
Ok(JsVariableKind::Var) => Self::HoistedValue,
137143
Err(_) => Self::Unknown,
138144
},
139-
AnyJsDeclaration::TsDeclareFunctionDeclaration(_) => Self::HoistedValue,
145+
AnyJsDeclaration::TsDeclareFunctionDeclaration(_) => Self::Function,
140146
AnyJsDeclaration::TsEnumDeclaration(_) => Self::Enum,
141147
AnyJsDeclaration::TsExternalModuleDeclaration(_) => Self::Module,
142148
AnyJsDeclaration::TsInterfaceDeclaration(_) => Self::Interface,
@@ -246,7 +252,8 @@ impl TsBindingReference {
246252
match self {
247253
Self::ValueType(binding_id)
248254
| Self::TypeAndValueType(binding_id)
249-
| Self::NamespaceAndValueType(binding_id) => binding_id,
255+
| Self::NamespaceAndValueType(binding_id)
256+
| Self::Type(binding_id) => binding_id,
250257
Self::Merged {
251258
ty,
252259
value_ty,
@@ -255,14 +262,15 @@ impl TsBindingReference {
255262
.or(namespace_ty)
256263
.or(ty)
257264
.expect("a merged reference must have at least two fields set to `Some`"),
258-
Self::Type(binding_id) => binding_id,
259265
}
260266
}
261267

262268
/// Creates a union from this binding reference with another.
263269
///
264270
/// If both bindings refer to the same kind of type, the binding ID(s) from
265-
/// `other` takes precedence.
271+
/// `other` take precedence. Same-name function overloads are tracked
272+
/// separately in the scope's overload map, so here two functions simply
273+
/// merge last-wins like any other pair of values.
266274
pub fn union_with(self, other: Self) -> Self {
267275
match (self, other) {
268276
(Self::Type(own_binding_id), Self::ValueType(other_binding_id)) => {

crates/biome_js_semantic/src/semantic_model/builder.rs

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use biome_js_syntax::{
66
use biome_jsdoc_comment::JsdocComment;
77
use biome_rowan::SyntaxNodePtr;
88
use rustc_hash::{FxHashMap, FxHashSet};
9+
use std::collections::hash_map::Entry;
910

1011
/// Builds the [SemanticModel] consuming [SemanticEvent] and [JsSyntaxNode].
1112
/// For a good example on how to use it see [semantic_model].
@@ -167,6 +168,7 @@ impl SemanticModelBuilder {
167168
children: vec![],
168169
bindings: vec![],
169170
bindings_by_name: FxHashMap::default(),
171+
overloads_by_name: FxHashMap::default(),
170172
read_references: vec![],
171173
write_references: vec![],
172174
is_closure,
@@ -235,13 +237,39 @@ impl SemanticModelBuilder {
235237
declaration_kind,
236238
);
237239

238-
scope
239-
.bindings_by_name
240-
.entry(name)
241-
.and_modify(|existing| {
242-
*existing = existing.union_with(binding_reference);
243-
})
244-
.or_insert(binding_reference);
240+
// Update `bindings_by_name` (declaration merging), keeping
241+
// the previous reference so we can detect same-name
242+
// function overloads on collision.
243+
let previous = match scope.bindings_by_name.entry(name.clone()) {
244+
Entry::Occupied(mut existing) => {
245+
let previous = *existing.get();
246+
*existing.get_mut() = previous.union_with(binding_reference);
247+
Some(previous)
248+
}
249+
Entry::Vacant(slot) => {
250+
slot.insert(binding_reference);
251+
None
252+
}
253+
};
254+
255+
// Record an overload set only when a function follows a
256+
// same-name function (rare). The common case of a unique
257+
// name adds nothing beyond the check above, keeping the
258+
// hot build path allocation- and rehash-free.
259+
if matches!(declaration_kind, JsDeclarationKind::Function)
260+
&& let Some(previous) = previous
261+
{
262+
let previous_id = previous.value_ty_or_ty();
263+
if self.bindings[previous_id.index()].declaration_kind
264+
== JsDeclarationKind::Function
265+
{
266+
scope
267+
.overloads_by_name
268+
.entry(name)
269+
.or_insert_with(|| smallvec::smallvec![previous_id])
270+
.push(binding_id);
271+
}
272+
}
245273
}
246274
}
247275

crates/biome_js_semantic/src/semantic_model/scope.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use super::*;
22
use biome_js_syntax::TextRange;
33
use biome_rowan::TokenText;
44
use rustc_hash::FxHashMap;
5+
use smallvec::SmallVec;
56
use std::sync::Arc;
67

78
#[derive(Debug)]
@@ -17,6 +18,10 @@ pub(crate) struct SemanticModelScopeData {
1718
// Map pointing to the [bindings] vec of each bindings by its name,
1819
// tracking the Type/Value/Namespace distinction for TypeScript declaration merging
1920
pub(crate) bindings_by_name: FxHashMap<TokenText, TsBindingReference>,
21+
// Same-name `function` / `declare function` declarations hoisted to this scope,
22+
// in source order, keyed by name. Tracked separately from `bindings_by_name` so
23+
// overload sets survive declaration merging with a same-name type/namespace.
24+
pub(crate) overloads_by_name: FxHashMap<TokenText, SmallVec<[BindingId; 2]>>,
2025
// All read references of a scope
2126
pub(crate) read_references: Vec<ReferenceId>,
2227
// All write references of a scope
@@ -131,6 +136,21 @@ impl Scope {
131136
data.bindings_by_name.get(name).copied()
132137
}
133138

139+
/// Returns every set of same-name function overloads declared in this
140+
/// scope, each in source order. Only names with two or more `function` /
141+
/// `declare function` declarations are returned; a single function is not
142+
/// an overload set.
143+
///
144+
/// It **does not** return overloads of parent scopes.
145+
pub fn overload_sets(&self) -> Vec<Vec<BindingId>> {
146+
self.data.scopes[self.id.index()]
147+
.overloads_by_name
148+
.values()
149+
.filter(|set| set.len() > 1)
150+
.map(|set| set.to_vec())
151+
.collect()
152+
}
153+
134154
/// Checks if the current scope is one of the ancestor of "other". Given
135155
/// that [Scope::ancestors] return "self" as the first scope,
136156
/// this function returns true for:

crates/biome_js_semantic/src/semantic_model/tests.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
mod test {
33
use crate::{
44
BindingExtensions, CanBeImportedExported, SemanticFlavor, SemanticModelOptions,
5-
SemanticScopeExtensions, semantic_model,
5+
SemanticScopeExtensions, TsBindingReference, semantic_model,
66
};
77
use biome_js_parser::JsParserOptions;
88
use biome_js_syntax::{
@@ -19,6 +19,51 @@ mod test {
1919
}
2020
}
2121

22+
/// Regression: same-name function overloads must keep their full set even
23+
/// when a same-name type/interface/namespace merges into the value's name.
24+
/// Previously the overload set lived inside `TsBindingReference` and was
25+
/// collapsed to the last signature the moment a type merged in.
26+
#[test]
27+
pub fn overload_sets_survive_declaration_merging() {
28+
let r = biome_js_parser::parse(
29+
"function f(a: number): number;\n\
30+
function f(a: string): string;\n\
31+
function f(a: number | string): number | string { return a; }\n\
32+
type f = number;\n",
33+
JsFileSource::ts(),
34+
JsParserOptions::default(),
35+
);
36+
let model = semantic_model(&r.tree(), SemanticModelOptions::default());
37+
let scope = model.global_scope();
38+
39+
// The full overload set (three signatures) is preserved.
40+
let overload_sets = scope.overload_sets();
41+
assert_eq!(overload_sets.len(), 1);
42+
assert_eq!(overload_sets[0].len(), 3);
43+
44+
// Name resolution still merges the value slot with the type slot.
45+
assert!(matches!(
46+
scope.get_binding_reference("f"),
47+
Some(TsBindingReference::Merged {
48+
ty: Some(_),
49+
value_ty: Some(_),
50+
..
51+
})
52+
));
53+
}
54+
55+
/// A single function is not an overload set.
56+
#[test]
57+
pub fn single_function_is_not_an_overload_set() {
58+
let r = biome_js_parser::parse(
59+
"function f(a: number): number { return a; }\n",
60+
JsFileSource::ts(),
61+
JsParserOptions::default(),
62+
);
63+
let model = semantic_model(&r.tree(), SemanticModelOptions::default());
64+
assert!(model.global_scope().overload_sets().is_empty());
65+
}
66+
2267
#[test]
2368
pub fn ok_semantic_model() {
2469
let r = biome_js_parser::parse(

crates/biome_js_semantic/src/snapshots/format_js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ Scope {
126126
Scope: ScopeId(2)
127127
Imported: false
128128
Exported: true
129-
Kind: HoistedValue
129+
Kind: Function
130130
ident: add
131131
JsDoc Comments: Adds two numbers.
132132
@param x - first operand
@@ -205,7 +205,7 @@ Scope {
205205
Scope: ScopeId(9)
206206
Imported: false
207207
Exported: true
208-
Kind: HoistedValue
208+
Kind: Function
209209
ident: outer
210210
}
211211
Scope {

crates/biome_js_semantic/src/snapshots/format_ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ Scope {
138138
Scope: ScopeId(5)
139139
Imported: false
140140
Exported: true
141-
Kind: HoistedValue
141+
Kind: Function
142142
ident: greet
143143
JsDoc Comments: Creates a greeting.
144144
@param cfg - the configuration

crates/biome_module_graph/src/js_module_info/collector.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,29 @@ impl JsModuleInfoCollector {
409409
self.bindings[index].ty = ty;
410410
}
411411
}
412+
413+
// A set of same-name function overloads becomes an object with one call
414+
// signature per declaration, placed on the binding that name resolution
415+
// returns for the set (its last one) so a call site can select among them.
416+
let carriers: Vec<(usize, Vec<TypeMember>)> = semantic_model
417+
.scopes()
418+
.flat_map(|scope| scope.overload_sets())
419+
.map(|set| {
420+
let signatures = set
421+
.iter()
422+
.map(|id| TypeMember {
423+
kind: TypeMemberKind::CallSignature,
424+
ty: self.bindings[id.index()].ty.clone(),
425+
})
426+
.collect();
427+
let representative = set.last().expect("overload set has 2+ entries").index();
428+
(representative, signatures)
429+
})
430+
.collect();
431+
for (representative, signatures) in carriers {
432+
let ty = self.reference_to_owned_data(TypeData::object_with_members(signatures.into()));
433+
self.bindings[representative].ty = ty;
434+
}
412435
}
413436

414437
fn has_writable_reference(&self, semantic_model: &SemanticModel, range: TextRange) -> bool {

crates/biome_module_graph/src/js_module_info/scope.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ impl FusedIterator for ScopeBindingsIter {}
154154
#[cfg(test)]
155155
mod tests {
156156
use super::*;
157+
use biome_js_semantic::JsDeclarationKind;
157158

158159
#[test]
159160
fn binding_reference_merging() {
@@ -213,4 +214,56 @@ mod tests {
213214
TsBindingReference::NamespaceAndValueType(BindingId::new(0))
214215
);
215216
}
217+
218+
#[test]
219+
fn function_declarations_are_plain_value_references() {
220+
// Functions are ordinary value references here; same-name overloads are
221+
// accumulated separately in the scope's `overloads_by_name` map, so the
222+
// `TsBindingReference` itself stays a small, `Copy` POD enum.
223+
assert_eq!(
224+
TsBindingReference::from_binding_and_declaration_kind(
225+
BindingId::new(0),
226+
JsDeclarationKind::Function,
227+
),
228+
TsBindingReference::ValueType(BindingId::new(0))
229+
);
230+
assert_eq!(
231+
TsBindingReference::from_binding_and_declaration_kind(
232+
BindingId::new(0),
233+
JsDeclarationKind::HoistedValue,
234+
),
235+
TsBindingReference::ValueType(BindingId::new(0))
236+
);
237+
assert_eq!(
238+
TsBindingReference::from_binding_and_declaration_kind(
239+
BindingId::new(0),
240+
JsDeclarationKind::Value,
241+
),
242+
TsBindingReference::ValueType(BindingId::new(0))
243+
);
244+
}
245+
246+
#[test]
247+
fn same_name_functions_merge_last_wins() {
248+
// Two same-name functions collapse to the last (implementation) signature
249+
// in `bindings_by_name`; the full overload set lives in `overloads_by_name`.
250+
assert_eq!(
251+
TsBindingReference::ValueType(BindingId::new(0))
252+
.union_with(TsBindingReference::ValueType(BindingId::new(1))),
253+
TsBindingReference::ValueType(BindingId::new(1))
254+
);
255+
256+
// A function merging with a same-name type still produces a `Merged`
257+
// reference, so name resolution keeps both the value and the type slot
258+
// even though the overload set is tracked elsewhere.
259+
assert_eq!(
260+
TsBindingReference::ValueType(BindingId::new(0))
261+
.union_with(TsBindingReference::Type(BindingId::new(1))),
262+
TsBindingReference::Merged {
263+
ty: Some(BindingId::new(1)),
264+
value_ty: Some(BindingId::new(0)),
265+
namespace_ty: None,
266+
}
267+
);
268+
}
216269
}

crates/biome_service/src/snapshots/biome_service__workspace__tests__debug_semantic_model.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Scope {
3535
Scope: ScopeId(2)
3636
Imported: false
3737
Exported: false
38-
Kind: HoistedValue
38+
Kind: Function
3939
ident: foo
4040
}
4141
Scope {

0 commit comments

Comments
 (0)