fix(core): resolve issue with rest params in type inference (#7270)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: arendjr

.changeset/resting-rhinos-reverberate.md

@@ -0,0 +1,5 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#6172](https://github.com/biomejs/biome/issues/6172): Resolved an issue with inferring types for rest parameters. This issue caused rest-parameter types to be incorrect, and in some cases caused extreme performance regressions in files that contained many methods with rest-parameter definitions.

crates/biome_js_analyze/src/services/typed.rs

@@ -14,7 +14,7 @@ use std::sync::Arc;
 ///
 /// This service is used for retrieving [`Type`] instances for arbitrary
 /// expressions or function definitions from the module graph.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct TypedService {
     resolver: Option<Arc<ModuleResolver>>,
 }

crates/biome_js_type_info/src/local_inference.rs

@@ -15,12 +15,12 @@ use biome_js_syntax::{
     JsFunctionDeclaration, JsFunctionExpression, JsGetterObjectMember, JsInitializerClause,
     JsLogicalExpression, JsLogicalOperator, JsMethodObjectMember, JsNewExpression,
     JsObjectBindingPattern, JsObjectExpression, JsParameters, JsPropertyClassMember,
-    JsPropertyObjectMember, JsReferenceIdentifier, JsReturnStatement, JsSetterObjectMember,
-    JsSyntaxNode, JsSyntaxToken, JsUnaryExpression, JsUnaryOperator, JsVariableDeclaration,
-    JsVariableDeclarator, TsDeclareFunctionDeclaration, TsExternalModuleDeclaration,
-    TsInterfaceDeclaration, TsModuleDeclaration, TsReferenceType, TsReturnTypeAnnotation,
-    TsTypeAliasDeclaration, TsTypeAnnotation, TsTypeArguments, TsTypeList, TsTypeParameter,
-    TsTypeParameters, TsTypeofType, inner_string_text, unescape_js_string,
+    JsPropertyObjectMember, JsReferenceIdentifier, JsRestParameter, JsReturnStatement,
+    JsSetterObjectMember, JsSyntaxNode, JsSyntaxToken, JsUnaryExpression, JsUnaryOperator,
+    JsVariableDeclaration, JsVariableDeclarator, TsDeclareFunctionDeclaration,
+    TsExternalModuleDeclaration, TsInterfaceDeclaration, TsModuleDeclaration, TsReferenceType,
+    TsReturnTypeAnnotation, TsTypeAliasDeclaration, TsTypeAnnotation, TsTypeArguments, TsTypeList,
+    TsTypeParameter, TsTypeParameters, TsTypeofType, inner_string_text, unescape_js_string,
 };
 use biome_rowan::{AstNode, SyntaxResult, Text, TokenText};
 
@@ -833,6 +833,7 @@ impl TypeData {
                         name,
                         ty: TypeReference::unknown(),
                         is_optional: false,
+                        is_rest: false,
                     })])
                 }
                 Ok(AnyJsArrowFunctionParameters::JsParameters(params)) => {
@@ -1507,6 +1508,7 @@ impl FunctionParameter {
                     .map(|ty| TypeReference::from_any_ts_type(resolver, scope_id, &ty))
                     .unwrap_or_default(),
                 is_optional: false,
+                is_rest: false,
             }),
         }
     }
@@ -1534,6 +1536,7 @@ impl FunctionParameter {
                 name,
                 ty: resolver.reference_to_owned_data(ty),
                 is_optional: param.question_mark_token().is_some(),
+                is_rest: false,
             })
         } else {
             let bindings = param
@@ -1554,6 +1557,50 @@ impl FunctionParameter {
         }
     }
 
+    pub fn from_js_rest_parameter(
+        resolver: &mut dyn TypeResolver,
+        scope_id: ScopeId,
+        param: &JsRestParameter,
+    ) -> Self {
+        let name = param
+            .binding()
+            .ok()
+            .as_ref()
+            .and_then(|binding| binding.as_any_js_binding())
+            .and_then(|binding| binding.as_js_identifier_binding())
+            .and_then(|identifier| identifier.name_token().ok())
+            .map(|token| token.token_text_trimmed().into());
+        let ty = param
+            .type_annotation()
+            .and_then(|annotation| annotation.ty().ok())
+            .map(|ty| TypeData::from_any_ts_type(resolver, scope_id, &ty))
+            .unwrap_or_default();
+        if let Some(name) = name {
+            Self::Named(NamedFunctionParameter {
+                name,
+                ty: resolver.reference_to_owned_data(ty),
+                is_optional: false,
+                is_rest: true,
+            })
+        } else {
+            let bindings = param
+                .binding()
+                .ok()
+                .and_then(|binding| {
+                    FunctionParameterBinding::bindings_from_any_js_binding_pattern_of_type(
+                        resolver, scope_id, &binding, &ty,
+                    )
+                })
+                .unwrap_or_default();
+            Self::Pattern(PatternFunctionParameter {
+                bindings,
+                ty: resolver.reference_to_owned_data(ty),
+                is_optional: false,
+                is_rest: true,
+            })
+        }
+    }
+
     pub fn params_from_js_parameters(
         resolver: &mut dyn TypeResolver,
         scope_id: ScopeId,

crates/biome_js_type_info/src/type_data.rs

@@ -556,6 +556,9 @@ pub struct NamedFunctionParameter {
 
     /// Whether the parameter is optional or not.
     pub is_optional: bool,
+
+    /// Whether this is a rest parameter (`...`) or not.
+    pub is_rest: bool,
 }
 
 /// A function parameter that is bound to either one or more positional

crates/biome_js_type_info/src/type_store.rs

@@ -25,7 +25,7 @@ use crate::{Resolvable, ResolvedTypeId, TypeData, TypeId, TypeReference, TypeRes
 /// some amount of deduplication is preferred. The reason for this is that type
 /// flattening can reduce disjoint types to the same type, at which point we
 /// store both to preserve their indices.
-#[derive(Debug, Default)]
+#[derive(Default)]
 pub struct TypeStore {
     types: Vec<Arc<TypeData>>,
     table: HashTable<usize>,
@@ -254,9 +254,9 @@ impl TypeStore {
     }
 }
 
-impl From<TypeStore> for Box<[Arc<TypeData>]> {
+impl From<TypeStore> for Vec<Arc<TypeData>> {
     fn from(store: TypeStore) -> Self {
-        store.types.into()
+        store.types
     }
 }
 

crates/biome_module_graph/benches/module_graph.rs

@@ -37,11 +37,10 @@ const INDEX_D_TS_CASES: &[(&str, &[u8])] = &[
         "@next/font/google/index.d.ts",
         include_bytes!("./next_font_google.d.ts") as &[u8],
     ),
-    // FIXME: enable it once the perf reaches a decent number
-    // (
-    //     "RedisCommander.d.ts",
-    //     include_bytes!("./RedisCommander.d.ts") as &[u8],
-    // )
+    (
+        "RedisCommander.d.ts",
+        include_bytes!("./RedisCommander.d.ts") as &[u8],
+    ),
 ];
 
 fn index_d_ts_cases() -> impl Iterator<Item = &'static str> {

crates/biome_module_graph/src/js_module_info.rs

@@ -174,24 +174,28 @@ pub struct JsModuleInfoInner {
 
     /// Re-exports that apply to all symbols from another module, without
     /// assigning a name to them.
-    pub blanket_reexports: Box<[JsReexport]>,
+    pub blanket_reexports: Vec<JsReexport>,
 
     /// Collection of all the declarations in the module.
-    pub(crate) bindings: Box<[JsBindingData]>,
+    pub(crate) bindings: Vec<JsBindingData>,
 
     /// Parsed expressions, mapped from their range to their type ID.
     pub(crate) expressions: FxHashMap<TextRange, ResolvedTypeId>,
 
     /// All scopes in this module.
     ///
     /// The first entry is expected to be the global scope.
-    pub(crate) scopes: Box<[JsScopeData]>,
+    pub(crate) scopes: Vec<JsScopeData>,
 
     /// Lookup tree to find scopes by text range.
     pub(crate) scope_by_range: Lapper<u32, ScopeId>,
 
     /// Collection of all types in the module.
-    pub(crate) types: Box<[Arc<TypeData>]>,
+    ///
+    /// We do not store these using our `TypeStore`, because once the module
+    /// info is constructed, no new types can be registered in it, and we have
+    /// no use for a hash table anymore.
+    pub(crate) types: Vec<Arc<TypeData>>,
 }
 
 #[derive(Debug, Default)]

crates/biome_module_graph/src/js_module_info/collector.rs

@@ -4,8 +4,8 @@ use biome_js_semantic::{SemanticEvent, SemanticEventExtractor};
 use biome_js_syntax::{
     AnyJsCombinedSpecifier, AnyJsDeclaration, AnyJsExportDefaultDeclaration, AnyJsExpression,
     AnyJsImportClause, JsForVariableDeclaration, JsFormalParameter, JsIdentifierBinding,
-    JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, JsVariableDeclaration, TsIdentifierBinding,
-    TsTypeParameter, TsTypeParameterName, inner_string_text,
+    JsRestParameter, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, JsVariableDeclaration,
+    TsIdentifierBinding, TsTypeParameter, TsTypeParameterName, inner_string_text,
 };
 use biome_js_type_info::{
     BindingId, FunctionParameter, GLOBAL_RESOLVER, GLOBAL_UNKNOWN_ID, GenericTypeParameter,
@@ -52,8 +52,8 @@ pub(super) struct JsModuleInfoCollector {
     /// Re-used from the semantic model in `biome_js_semantic`.
     extractor: SemanticEventExtractor,
 
-    /// Formal parameters.
-    formal_parameters: FxHashMap<JsSyntaxNode, FunctionParameter>,
+    /// Function parameters, both formal parameters as well as rest parameters.
+    function_parameters: FxHashMap<JsSyntaxNode, FunctionParameter>,
 
     /// Variable declarations.
     variable_declarations: FxHashMap<JsSyntaxNode, Box<[(Text, TypeReference)]>>,
@@ -174,7 +174,12 @@ impl JsModuleInfoCollector {
         } else if let Some(param) = JsFormalParameter::cast_ref(node) {
             let scope_id = *self.scope_stack.last().expect("there must be a scope");
             let parsed_param = FunctionParameter::from_js_formal_parameter(self, scope_id, &param);
-            self.formal_parameters
+            self.function_parameters
+                .insert(param.syntax().clone(), parsed_param);
+        } else if let Some(param) = JsRestParameter::cast_ref(node) {
+            let scope_id = *self.scope_stack.last().expect("there must be a scope");
+            let parsed_param = FunctionParameter::from_js_rest_parameter(self, scope_id, &param);
+            self.function_parameters
                 .insert(param.syntax().clone(), parsed_param);
         } else if let Some(decl) = JsVariableDeclaration::cast_ref(node) {
             let scope_id = *self.scope_stack.last().expect("there must be a scope");
@@ -543,6 +548,10 @@ impl JsModuleInfoCollector {
 
         self.infer_all_types(&scope_by_range);
         self.resolve_all_and_downgrade_project_references();
+
+        // Purging before flattening will save us from duplicate work during
+        // flattening. We'll purge again after for a final cleanup.
+        self.purge_redundant_types();
         self.flatten_all();
         self.purge_redundant_types();
 
@@ -553,13 +562,12 @@ impl JsModuleInfoCollector {
 
     fn infer_all_types(&mut self, scope_by_range: &Lapper<u32, ScopeId>) {
         for index in 0..self.bindings.len() {
-            let binding_id = BindingId::new(index);
-            let binding = &self.bindings[binding_id.index()];
+            let binding = &self.bindings[index];
             if let Some(node) = self.binding_node_by_start.get(&binding.range.start()) {
                 let name = binding.name.clone();
                 let scope_id = scope_id_for_range(scope_by_range, binding.range);
                 let ty = self.infer_type(&node.clone(), &name, scope_id);
-                self.bindings[binding_id.index()].ty = ty;
+                self.bindings[index].ty = ty;
             }
         }
     }
@@ -596,7 +604,11 @@ impl JsModuleInfoCollector {
                     .find_map(|(name, ty)| (name == binding_name).then(|| ty.clone()))
                     .unwrap_or_default();
             } else if let Some(param) = JsFormalParameter::cast_ref(&ancestor)
-                .and_then(|param| self.formal_parameters.get(param.syntax()))
+                .and_then(|param| self.function_parameters.get(param.syntax()))
+                .or_else(|| {
+                    JsRestParameter::cast_ref(&ancestor)
+                        .and_then(|param| self.function_parameters.get(param.syntax()))
+                })
             {
                 return match param {
                     FunctionParameter::Named(named) => named.ty.clone(),
@@ -1031,10 +1043,10 @@ impl JsModuleInfo {
             static_import_paths: collector.static_import_paths,
             dynamic_import_paths: collector.dynamic_import_paths,
             exports: Exports(exports),
-            blanket_reexports: collector.blanket_reexports.into(),
-            bindings: collector.bindings.into(),
+            blanket_reexports: collector.blanket_reexports,
+            bindings: collector.bindings,
             expressions: collector.parsed_expressions,
-            scopes: collector.scopes.into(),
+            scopes: collector.scopes,
             scope_by_range,
             types: collector.types.into(),
         }))

crates/biome_module_graph/src/js_module_info/module_resolver.rs

@@ -42,7 +42,6 @@ const MODULE_0_ID: ResolverId = ResolverId::from_level(TypeResolverLevel::Thin);
 /// statements.
 ///
 /// The module resolver is typically consumed through the `Typed` service.
-#[derive(Debug)]
 pub struct ModuleResolver {
     module_graph: Arc<ModuleGraph>,