Skip to content

perf(inference): reduce type widening #8374

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.

Sign up for GitHub

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

Open
ematipico wants to merge 2 commits into
base: main
Choose a base branch
from
+57 −13
Open

perf(inference): reduce type widening #8374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
622c663
perf(inference): reduce type widening
ematipico Dec 5, 2025
9937a9c
create less types
ematipico Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/biome_js_type_info/src/resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,26 @@ pub trait TypeResolver {
// #endregion
}

pub struct UnionCollector {
types: Vec<TypeReference>,
}

impl UnionCollector {
pub fn new() -> Self {
Self { types: Vec::new() }
}

pub fn add(&mut self, ty: TypeReference) {
self.types.push(ty);
}

pub fn finish(self) -> Cow<'static, TypeData> {
Cow::Owned(TypeData::Union(Box::new(Union(
self.types.into_boxed_slice(),
))))
}
}
Comment on lines +740 to +758
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Implement Default trait and handle the empty-union edge case.

The Clippy lint correctly identifies that new() should be replaced with a Default implementation—this is causing the build to fail. Additionally, finish() will create an empty union if no types were added, which is likely incorrect (an empty union is uninhabitable).

Apply this diff to fix the Clippy error:

+impl Default for UnionCollector {
+    fn default() -> Self {
+        Self { types: Vec::new() }
+    }
+}
+
 impl UnionCollector {
-    pub fn new() -> Self {
-        Self { types: Vec::new() }
-    }
-
     pub fn add(&mut self, ty: TypeReference) {
         self.types.push(ty);
     }
 
     pub fn finish(self) -> Cow<'static, TypeData> {
+        if self.types.is_empty() {
+            return Cow::Owned(TypeData::unknown());
+        }
         Cow::Owned(TypeData::Union(Box::new(Union(
             self.types.into_boxed_slice(),
         ))))
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Actions: Pull request

[error] 745-747: Clippy: you should consider adding a Default implementation for UnionCollector. This lint was treated as an error due to -D warnings, causing the build to fail while running 'cargo lint'.

🤖 Prompt for AI Agents 
In crates/biome_js_type_info/src/resolver.rs around lines 740 to 758, replace
the manual new() with a Default impl for UnionCollector to satisfy Clippy and
change finish() to handle the empty-union edge case: implement Default for
UnionCollector (initializing types = Vec::new()), update or remove new() to call
Default if kept, and modify finish() so that if self.types.is_empty() it returns
Cow::Owned(TypeData::Never) (or the crate's uninhabitable variant) instead of
constructing an empty Union; otherwise return the Union as before.


/// Trait to be implemented by `TypeData` and its subtypes to aid the resolver.
pub trait Resolvable: Sized {
/// Returns the resolved version of this type.
Expand Down
50 changes: 37 additions & 13 deletions crates/biome_module_graph/src/js_module_info/collector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use biome_js_type_info::{
BindingId, FunctionParameter, GLOBAL_RESOLVER, GLOBAL_UNKNOWN_ID, GenericTypeParameter,
MAX_FLATTEN_DEPTH, Module, Namespace, Resolvable, ResolvedTypeData, ResolvedTypeId, ScopeId,
TypeData, TypeId, TypeImportQualifier, TypeMember, TypeMemberKind, TypeReference,
TypeReferenceQualifier, TypeResolver, TypeResolverLevel, TypeStore,
TypeReferenceQualifier, TypeResolver, TypeResolverLevel, TypeStore, UnionCollector,
};
use biome_jsdoc_comment::JsdocComment;
use biome_rowan::{AstNode, Text, TextRange, TextSize, TokenText};
Expand Down Expand Up @@ -665,6 +665,22 @@ impl JsModuleInfoCollector {
TypeReference::unknown()
}

fn get_scope_by_range(&self, range: TextRange) -> Option<&ScopeId> {
let start: u32 = range.start().into();
let end: u32 = range.end().into();

// Search through all scope intervals to find the one that contains this range
self.scope_range_by_start
.values()
.flat_map(|intervals| intervals.iter())
.filter(|interval| {
// The interval must fully contain the range
interval.start <= start && end <= interval.stop
})
.max_by_key(|interval| interval.val)
.map(|interval| &interval.val)
}

/// Widen the type of binding from its writable references.
fn widen_binding_from_writable_references(
&mut self,
Expand All @@ -674,25 +690,33 @@ impl JsModuleInfoCollector {
) -> TypeReference {
let references = self.get_writable_references(binding);
let mut ty = ty.clone();
let mut union_collector = UnionCollector::new();
for reference in references {
let Some(node) = self.binding_node_by_start.get(&reference.range_start) else {
continue;
};
for ancestor in node.ancestors().skip(1) {
if let Some(assignment) = JsAssignmentExpression::cast_ref(&ancestor)
&& let Ok(right) = assignment.right()
{
let data = TypeData::from_any_js_expression(self, scope_id, &right);
let assigned_type = self.reference_to_owned_data(data);
ty = ResolvedTypeId::new(
self.level(),
self.union_with(ty.clone(), assigned_type),
)
.into();
}
let Some(reference_scope) = self.get_scope_by_range(node.text_trimmed_range()) else {
continue;
};

// We don't want to widen types inside the same scope
if binding.scope_id == *reference_scope {
continue;
}
let assignment = node
.ancestors()
.skip(1)
.find_map(|ancestor| JsAssignmentExpression::cast_ref(&ancestor))
.and_then(|assignment| assignment.right().ok());
if let Some(right) = assignment {
let data = TypeData::from_any_js_expression(self, scope_id, &right);
let assigned_type = self.reference_to_owned_data(data);
union_collector.add(assigned_type);
}
}

let id = self.register_type(union_collector.finish());
ty = ResolvedTypeId::new(self.level(), id).into();
ty
}

Expand Down
Loading