fix(core): improve inference with conditionals#7393
Merged
arendjr merged 1 commit intobiomejs:mainfrom Sep 4, 2025
Merged
Conversation
|
Contributor
Walkthrough
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
arendjr
commented
Sep 4, 2025
|
|
||
| use crate::{FixKind, Rule, RuleKey}; | ||
| use std::any::{Any, TypeId}; | ||
| use std::fmt::Debug; |
Contributor
Author
There was a problem hiding this comment.
Sorry, this is an unrelated change. But it doesn't hurt, I guess 😅
arendjr
commented
Sep 4, 2025
| let mut conditional = ConditionalType::Unknown; | ||
| for ty in ty.flattened_union_variants(resolver) { | ||
| let next = derive_from_reference(&ty); | ||
| let next = derive_from_resolved_reference(&ty); |
Contributor
Author
There was a problem hiding this comment.
This is the fix that I'm hoping will solve #7324.
arendjr
commented
Sep 4, 2025
| Some(TypeData::union_of(resolver, types)) | ||
| } | ||
| _ => None, | ||
| _ => Some(resolved.clone()), |
Contributor
Author
There was a problem hiding this comment.
This is an unrelated fix that I discovered writing the test_reference_to_falsy_subset_of(). So even though the tests didn't reproduce #7324, they weren't for nothing :)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
crates/biome_js_type_info/src/type.rs (1)
190-201: Numeric literal equality: mind float quirksDirect f64 equality is fine for integers (your current tests: 0 and 1). For future decimals/exponent forms, consider normalising via numeric value rather than string form and documenting that NaN/Infinity aren’t supported as literal types, or add a small helper to compare canonicalised numeric-literal values.
crates/biome_js_type_info/src/lib.rs (1)
14-14: Limit public exports fromconditionals
Glob re-export exposes four symbols:
ConditionalTypereference_to_falsy_subset_ofreference_to_non_nullish_subset_ofreference_to_truthy_subset_ofReplace
pub use conditionals::*;with an explicit list of only the items actually needed externally.
crates/biome_js_type_info/tests/conditionals.rs (2)
23-24: Assert single binding explicitly to avoid accidental shape changesMake the expectation clear and avoid an O(n)
remove(0).- let (var_name, var_ty) = bindings.into_vec().remove(0); - assert_eq!(var_name.text(), "foo"); + let bindings = bindings.into_vec(); + assert_eq!(bindings.len(), 1, "expected a single binding"); + let (var_name, var_ty) = bindings.into_iter().next().unwrap(); + assert_eq!(var_name.text(), "foo");
30-35: Nice targeted coverageThe falsy-subset path is exercised end-to-end. Optionally assert that the empty string literal is included as well for completeness.
crates/biome_module_graph/tests/spec_tests.rs (1)
1627-1690: Great regression guard; consider asserting empty string tooThis captures the cross-module union and the falsy mapping well. Given
ReactNodeincludesstring, also asserting""would tighten the net.assert!(ty.has_variant(|ty| ty.is_boolean_literal(false))); assert!(ty.has_variant(|ty| ty.is_number_literal(0.))); assert!(ty.has_variant(|ty| ty.is_number_literal(1.))); + assert!(ty.has_variant(|ty| ty.is_string_literal("")));crates/biome_js_type_info/src/conditionals.rs (1)
57-68: Good centralisation; tiny polish on Cow handlingNice catch splitting raw vs already-applied references. For readability and to avoid taking
&Cow<_>(relying on deref-coercion), passas_ref()and avoid shadowing the name.- let derive_from_reference = |reference: &TypeReference| -> ConditionalType { - let reference = ty.apply_module_id_to_reference(reference); - derive_from_resolved_reference(&reference) - }; + let derive_from_reference = |reference: &TypeReference| -> ConditionalType { + let applied = ty.apply_module_id_to_reference(reference); + derive_from_resolved_reference(applied.as_ref()) + };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_js_type_info/tests/snapshots/test_reference_to_falsy_subset_of.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
crates/biome_analyze/src/options.rs(0 hunks)crates/biome_js_type_info/src/conditionals.rs(3 hunks)crates/biome_js_type_info/src/helpers.rs(1 hunks)crates/biome_js_type_info/src/lib.rs(1 hunks)crates/biome_js_type_info/src/type.rs(2 hunks)crates/biome_js_type_info/tests/conditionals.rs(1 hunks)crates/biome_module_graph/tests/spec_tests.rs(1 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_analyze/src/options.rs
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_type_info/src/type.rscrates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/tests/conditionals.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_js_type_info/src/helpers.rs
crates/biome_js_type_info/src/**/*.rs
📄 CodeRabbit inference engine (crates/biome_js_type_info/CONTRIBUTING.md)
crates/biome_js_type_info/src/**/*.rs: Represent links between types using TypeReference (not Arc) to avoid cross-module retention and recursive structures; store type data in linear vectors
Use TypeData::Unknown for unimplemented inference and TypeData::UnknownKeyword for the explicit TypeScript unknown keyword; treat them semantically the same but keep them distinct for measurement
Files:
crates/biome_js_type_info/src/type.rscrates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_js_type_info/src/helpers.rs
crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs
📄 CodeRabbit inference engine (crates/biome_js_type_info/CONTRIBUTING.md)
Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Files:
crates/biome_js_type_info/src/type.rscrates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_js_type_info/src/helpers.rs
crates/biome_js_type_info/{src,biome_module_graph/src}/**/*.rs
📄 CodeRabbit inference engine (crates/biome_js_type_info/CONTRIBUTING.md)
When pattern-matching on ResolvedTypeData via as_raw_data(), ensure any nested TypeReferences are subsequently resolved using the correct ResolverId; never use the raw data with a resolver without applying the right ResolverId to avoid panics
Files:
crates/biome_js_type_info/src/type.rscrates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_js_type_info/src/helpers.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_type_info/src/type.rscrates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/tests/conditionals.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_js_type_info/src/helpers.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_type_info/tests/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Use TypeData::Unknown for unimplemented inference and TypeData::UnknownKeyword for the explicit TypeScript unknown keyword; treat them semantically the same but keep them distinct for measurement
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Represent links between types using TypeReference (not Arc) to avoid cross-module retention and recursive structures; store type data in linear vectors
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,biome_module_graph/src}/**/*.rs : When pattern-matching on ResolvedTypeData via as_raw_data(), ensure any nested TypeReferences are subsequently resolved using the correct ResolverId; never use the raw data with a resolver without applying the right ResolverId to avoid panics
Learnt from: arendjr
PR: biomejs/biome#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.
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference may resolve qualifiers to local declarations or globals (TypeReference::Resolved), mark imported bindings as TypeReference::Import, and set unresolved to TypeReference::Unknown; it must not look beyond the current module
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/scoped_resolver.rs : Full inference resolves TypeReference::Import across modules into TypeReference::Resolved using the module graph; do not cache full-inference results
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/local_inference.rs : Local inference must not perform type resolution; it should only create TypeReference::Qualifier entries from syntax
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/flattening.rs : Flatten types only after references are resolved; e.g., addition of two TypeData::Number flattens to TypeData::Number, otherwise often to TypeData::String
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_js_type_info/src/lib.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Applied to files:
crates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/flattening.rs : Flatten types only after references are resolved; e.g., addition of two TypeData::Number flattens to TypeData::Number, otherwise often to TypeData::String
Applied to files:
crates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
PR: biomejs/biome#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/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Represent links between types using TypeReference (not Arc) to avoid cross-module retention and recursive structures; store type data in linear vectors
Applied to files:
crates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,biome_module_graph/src}/**/*.rs : When pattern-matching on ResolvedTypeData via as_raw_data(), ensure any nested TypeReferences are subsequently resolved using the correct ResolverId; never use the raw data with a resolver without applying the right ResolverId to avoid panics
Applied to files:
crates/biome_js_type_info/src/lib.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Use TypeData::Unknown for unimplemented inference and TypeData::UnknownKeyword for the explicit TypeScript unknown keyword; treat them semantically the same but keep them distinct for measurement
Applied to files:
crates/biome_js_type_info/tests/conditionals.rscrates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_js_type_info/src/helpers.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Applied to files:
crates/biome_js_type_info/tests/conditionals.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_js_type_info/tests/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/scoped_resolver.rs : Full inference resolves TypeReference::Import across modules into TypeReference::Resolved using the module graph; do not cache full-inference results
Applied to files:
crates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/local_inference.rs : Local inference must not perform type resolution; it should only create TypeReference::Qualifier entries from syntax
Applied to files:
crates/biome_js_type_info/src/conditionals.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference may resolve qualifiers to local declarations or globals (TypeReference::Resolved), mark imported bindings as TypeReference::Import, and set unresolved to TypeReference::Unknown; it must not look beyond the current module
Applied to files:
crates/biome_js_type_info/src/conditionals.rscrates/biome_module_graph/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable
Applied to files:
crates/biome_js_type_info/src/conditionals.rs
🧬 Code graph analysis (4)
crates/biome_js_type_info/tests/conditionals.rs (3)
crates/biome_js_type_info/src/conditionals.rs (1)
reference_to_falsy_subset_of(277-303)crates/biome_js_type_info/tests/utils.rs (2)
assert_type_data_snapshot(22-55)get_variable_declaration(272-286)crates/biome_js_type_info/src/local_inference.rs (1)
typed_bindings_from_js_variable_declaration(1403-1416)
crates/biome_js_type_info/src/conditionals.rs (3)
crates/biome_js_type_info/src/type_data.rs (3)
reference(337-339)ty(539-544)next(783-793)crates/biome_js_type_info/src/resolver.rs (4)
ty(512-514)resolved(725-725)resolved(732-778)resolved(786-800)crates/biome_js_type_info/src/flattening/unions.rs (1)
next(70-96)
crates/biome_module_graph/tests/spec_tests.rs (3)
crates/biome_js_type_info/src/type.rs (4)
default(48-53)has_variant(108-114)is_boolean_literal(133-141)is_number_literal(191-201)crates/biome_test_utils/src/lib.rs (1)
get_added_paths(196-217)crates/biome_module_graph/src/js_module_info/module_resolver.rs (1)
for_module(76-91)
crates/biome_js_type_info/src/helpers.rs (1)
crates/biome_js_syntax/src/expr_ext.rs (1)
is_undefined(75-77)
⏰ 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). (25)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: autofix
🔇 Additional comments (5)
crates/biome_js_type_info/src/type.rs (1)
132-141: Boolean literal check looks goodThe implementation follows existing literal patterns and is safely non-resolving. No issues spotted.
crates/biome_js_type_info/src/helpers.rs (1)
586-586: Consistent matcher addition
is_undefinedaligns with the existing matcher macros and unlocks tidy call-sites (e.g., in tests). Looks good.crates/biome_module_graph/tests/spec_tests.rs (1)
1627-1690: No misuse of apply_module_id_to_reference around flattened unions; ready to mergecrates/biome_js_type_info/src/conditionals.rs (2)
112-114: Correct: don’t re-apply module IDs for flattened union variantsSwitching to
derive_from_resolved_referencehere aligns with the invariant thatflattened_union_variants()already returns module-aware references. This should prevent the double-application bug behind the crash.
434-435: Fallback now retains unhandled shapes — please confirm intended semanticsReturning
Some(resolved.clone())in the Retained path changes behaviour from “drop” to “pass-through”. Likely right (avoids over-stripping), but it could widen results subtly. Please confirm this keepsreference_to_{truthy,falsy,non_nullish}_subset_ofstable on edge cases (e.g. unions containing generics/typeof).
CodSpeed Performance ReportMerging #7393 will not alter performanceComparing Summary
Footnotes |
kedevked
pushed a commit
to kedevked/biome
that referenced
this pull request
Sep 22, 2025
dyc3
pushed a commit
that referenced
this pull request
Oct 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hopefully fix #7324: Even though we're missing a complete reproduction, I noticed the partial reproductions we had both included a conditionals, so I double-checked the inference code related to those: I noticed an invalid
apply_module_id_to_reference()happening inderive_conditional_type(), because the references coming fromflattened_union_variants()already have correct module ID applied to them.flattened_union_variants()is also a very recent addition, so it could very well explain the regression.Test Plan
Tests added. Unfortunately the tests didn't reproduce the original problem, so I can only hope it resolves the issue.
Docs
N/A
Note I didn't even add a changeset as I'm not sure the fix will solve the linked issue.