fix(core): implement constructor parameter property tracking #8346
Conversation
🦋 Changeset detectedLatest commit: 91c5cb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR adds constructor-parameter handling to the type inference layer so TypeScript parameter properties are tracked as typed class members. It introduces a public Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_js_type_info/src/local_inference.rs (2)
2243-2262: Verify the indexed loop pattern.The loop iterates over
0..num_memberswhile potentially pushing tomembers, but since you only read from indices belownum_members, this is safe. However, consider using iterators withfilterandflat_mapfor a more idiomatic Rust style, or alternatively collect constructor members separately before extending.- let num_members = members.len(); - for i in 0..num_members { - let member = &members[i]; - if member.is_constructor() - && let Some(member_ty) = resolver.resolve_and_get(&member.ty) - && let TypeData::Constructor(constructor) = member_ty.as_raw_data() - { - for param in &constructor.parameters { - if let Some(_accessibility) = param.accessibility - && let FunctionParameter::Named(named_param) = ¶m.parameter - { - // TODO: Assign accessibility to type members. - members.push(Self { - kind: TypeMemberKind::Named(named_param.name.clone()), - ty: param.parameter.ty().clone(), - }); - } - } - } - } + let extra_members: Vec<_> = members + .iter() + .filter(|member| member.is_constructor()) + .filter_map(|member| resolver.resolve_and_get(&member.ty)) + .filter_map(|member_ty| match member_ty.as_raw_data() { + TypeData::Constructor(constructor) => Some(constructor), + _ => None, + }) + .flat_map(|constructor| { + constructor.parameters.iter().filter_map(|param| { + if param.accessibility.is_some() { + if let FunctionParameter::Named(named_param) = ¶m.parameter { + // TODO: Assign accessibility to type members. + return Some(Self { + kind: TypeMemberKind::Named(named_param.name.clone()), + ty: param.parameter.ty().clone(), + }); + } + } + None + }) + }) + .collect(); + members.extend(extra_members);
1510-1514: Consider logging or handling bogus parameters.Returning
Self::default()forJsBogusParametersilently discards potentially useful diagnostic information. This is fine for now, but you might want to track this for observability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue8292.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/constructed-carriers-corrolate.md(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue8292.ts(1 hunks)crates/biome_js_type_info/src/local_inference.rs(10 hunks)crates/biome_js_type_info/src/type_data.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md: Create changesets for user-facing changes usingjust new-changeset; use headers with####or#####only; keep descriptions concise (1-3 sentences) and focus on user-facing changes
Use past tense when describing what was done ('Added new feature'), present tense when describing Biome behavior ('Biome now supports'); end sentences with a full stop
For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use adiffcode block
Files:
.changeset/constructed-carriers-corrolate.md
crates/biome_js_type_info/**/*.rs
📄 CodeRabbit inference engine (crates/biome_js_type_info/CONTRIBUTING.md)
crates/biome_js_type_info/**/*.rs: No module may copy or clone data from another module in the module graph, not even behind anArc
UseTypeReferenceinstead ofArcfor types that reference other types to avoid stale cache issues when modules are replaced
Store type data in linear vectors instead of using recursive data structures withArcfor improved data locality and performance
UseTypeReferencevariants (Qualifier,Resolved,Import,Unknown) to represent different phases of type resolution
UseTypeData::Unknownto indicate when type inference falls short or is not implemented
Distinguish betweenTypeData::UnknownandTypeData::UnknownKeywordto measure inference effectiveness versus explicit user-provided unknown types
When usingResolvedTypeData, track theResolverIdto ensure subsequent resolver calls use the correct context
Always apply the correctResolverIdwhen retrieving raw type data fromResolvedTypeData.as_raw_data()to prevent panics during subsequent resolution calls
Files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
**/*.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For Node.js package development, build WebAssembly bindings and JSON-RPC bindings; run tests against compiled files after implementing features or bug fixes
Files:
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue8292.ts
crates/biome_js_type_info/**/local_inference.rs
📄 CodeRabbit inference engine (crates/biome_js_type_info/CONTRIBUTING.md)
Implement local inference in dedicated modules to derive type definitions from expressions without context of surrounding scopes
Files:
crates/biome_js_type_info/src/local_inference.rs
🧠 Learnings (20)
📚 Learning: 2025-11-28T09:08:10.077Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : Create changesets for user-facing changes using `just new-changeset`; use headers with `####` or `#####` only; keep descriptions concise (1-3 sentences) and focus on user-facing changes
Applied to files:
.changeset/constructed-carriers-corrolate.md
📚 Learning: 2025-11-28T09:08:10.077Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a `diff` code block
Applied to files:
.changeset/constructed-carriers-corrolate.md
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph
Applied to files:
crates/biome_js_type_info/src/type_data.rscrates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.{js,ts,tsx,jsx,json,css} : Test rules using snapshot tests via the `insta` library with test cases in `tests/specs/<group>/<rule_name>/` directories prefixed by `invalid` or `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue8292.ts
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/js_module_info/collector.rs : Implement module-level (thin) inference to resolve `TypeReference::Qualifier` variants by looking up declarations in module scopes and handling import statements
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/local_inference.rs : Implement local inference in dedicated modules to derive type definitions from expressions without context of surrounding scopes
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : When using `ResolvedTypeData`, track the `ResolverId` to ensure subsequent resolver calls use the correct context
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Always apply the correct `ResolverId` when retrieving raw type data from `ResolvedTypeData.as_raw_data()` to prevent panics during subsequent resolution calls
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/flattening.rs : Implement type flattening to simplify `TypeofExpression` variants once all component types are resolved
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When formatting AST nodes, use mandatory tokens from the AST instead of hardcoding token strings (e.g., use `node.l_paren_token().format()` instead of `token("(")`)
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : For tokens that are not mandatory, use helper functions instead of hardcoding
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to 'fix' the code; if a token/node is known to be mandatory but is missing, return `None` instead
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `declare_node_union!` macro to query multiple node types at once by joining them into an enum with `Any*Like` naming convention
Applied to files:
crates/biome_js_type_info/src/local_inference.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/issue8292.ts (1)
1-15: LGTM!Test correctly exercises the scenario from issue #8292: a floating promise call via a constructor parameter property. This will validate that the type tracking now works for
this.field.returnsPromise().crates/biome_js_type_info/src/type_data.rs (3)
461-468: LGTM!The
ConstructorParameterstruct cleanly wrapsFunctionParameterwith optional accessibility, enabling parameter property tracking without disrupting existing function parameter handling.
554-563: LGTM!Sensible default implementation for
FunctionParameter— returns a pattern parameter with unknown type and empty bindings. This supports fallback paths in constructor parameter conversion.
1511-1518: LGTM!
TypeMemberAccessibilityenum withPublicas the default aligns with TypeScript semantics where unspecified accessibility defaults to public.crates/biome_js_type_info/src/local_inference.rs (3)
1817-1832: LGTM!Clean handling of
JsConstructorClassMember— the constructor is properly converted to aTypeMemberwithTypeMemberKind::Constructor.
2524-2538: LGTM!The
from_modifier_listmethod correctly extracts accessibility from TypeScript property parameter modifiers, defaulting toPublicwhen no explicit modifier is found.
2540-2573: LGTM!The helper functions
constructor_params_from_js_constructor_paramsandconstructor_params_from_js_paramsprovide clean conversion paths for constructor parameters, properly wrappingFunctionParameterwith accessibility metadata.
CodSpeed Performance ReportMerging #8346 will not alter performanceComparing Summary
Footnotes
|
Summary
Fixed #8292: Implement tracking of types of TypeScript constructor parameter properties.
This resolves certain false negatives in
noFloatingPromisesand other typed rules.Example
Test Plan
Test added.
Docs
N/A