Rebecca Chen

Change AnswersSolver::error to take ErrorKind directly [9/16] Summary: Pyrefly's error reporting API has grown unwieldy. ErrorCollector currently has two methods for emitting errors (add and add_with_annotations_and_quick_fixes) whose signatures grow every time we add a new optional decoration like annotations or quick fixes. This stack introduces a builder pattern (design: P2307835290, implementation plan: P2307836987) that replaces these methods with an error_builder() returning a chainable ErrorBuilder, making error construction explicit and extensible. This commit changes AnswersSolver::error from taking ErrorInfo to taking ErrorKind directly. After commits 6-7 migrated all ErrorInfo::new and ErrorInfo::Context call sites away, only ErrorInfo::Kind(k) callers remain — this is a purely mechanical replacement. The method now delegates to error_builder internally. All ~250 callers are updated to pass ErrorKind directly, and unused ErrorInfo imports are removed. Reviewed By: fangyi-zhou Differential Revision: D103990096 fbshipit-source-id: e74b693775121033dde6a58c4a8d76ae09779124

Move residual-related functions that only operate on Type + TypeHeap to the types crate Summary: This stack: reorganizes some of the callable residual code to make it clearer which parts of the logic are tightly coupled to the solver, which will make it easier for me to make solver changes. This diff: moves Solver functions that access nothing from the Solver except `self.heap` into the types crate. They are turned into methods on the appropriate types that accept a `&TypeHeap` argument. No functional change. Reviewed By: yangdanny97 Differential Revision: D106045944 fbshipit-source-id: bb7abe3efa6dca4bacff3bcabd00fb7812359f7a

Migrate direct ErrorCollector::add calls to builder [10/16] Summary: Pyrefly's error reporting API has grown unwieldy. ErrorCollector currently has two methods for emitting errors (add and add_with_annotations_and_quick_fixes) whose signatures grow every time we add a new optional decoration like annotations or quick fixes. This stack introduces a builder pattern (design: P2307835290, implementation plan: P2307836987) that replaces these methods with an error_builder() returning a chainable ErrorBuilder, making error construction explicit and extensible. This commit migrates all ~48 direct errors.add(range, ErrorInfo::Kind(k), vec1![msg]) calls to use the builder. Single-line messages become errors.error_builder(range, kind, msg).emit(). Multi-line Vec1 messages are split into header + with_detail() chains. Dynamic Vec1 built with pushes are converted to conditional builder chains. Files modified across alt/, binding/, module/, and state/. Unused ErrorInfo and vec1 imports are removed. Reviewed By: fangyi-zhou Differential Revision: D103990088 fbshipit-source-id: 5f2a534ec943b5e50e8fb6bbbd18da5696417841

Support exhaustiveness checks on SelfType Summary: I noticed while looking into https://github.com/facebook/pyrefly/issues/2514 that we emit a couple of spurious errors caused by not realizing when a `match` on `self` is exhaustive. Reviewed By: grievejia Differential Revision: D95323925 fbshipit-source-id: 13eb6955b6cf280138f8ca3ba03c6b1423e2fb9e

Keep specialization errors arising from hints to overloaded calls Summary: Fixes a bug where we would throw away a valid specialization error. Reviewed By: grievejia Differential Revision: D100931635 fbshipit-source-id: ae7edae78b43456281c37445d201432ba72dd46e

Detect Final status changes in incremental export diffing Summary: `Exports::changed_exports` was not comparing `final_names` between old and new definitions, so adding or removing a `Final` annotation on an export would not be detected as a metadata change. This meant downstream modules depending on finality (via `ExportOrigin`) would not be re-checked, leading to stale errors. Add `final_names` to the metadata comparison in `changed_exports`. Reviewed By: grievejia Differential Revision: D104303060 fbshipit-source-id: 1c44c905bd4e8f33b705a47f83ed168fe24b45c2

Unify var traversal in solver.rs Summary: * Adds a new `resolve_vars` function that handles var traversal for both `expand_vars` and `deep_force`, allowing us to clean up some duplication and redundant helpers. * Moves dimension canonicalization into `simplify_mut` so we can get rid of `simplify_forced_type`. * Removes a couple no-longer-relevant TODOs. No functional change. Reviewed By: yangdanny97 Differential Revision: D106076328 fbshipit-source-id: 63d4277da296a6380cca01a06f8308ab19a625b3

Backfill missing release notes Summary: Backfills missing release notes for 0.58.0 - 0.63.0. Once this diff is approved, I'll edit these into the corresponding releases. 0.59.0 already has release notes on GitHub, so all I did for that version was copy the existing notes into the release_notes directory. There was already an md file for 0.60.0, but it hadn't been copied to the GitHub release, and the content looked a little questionable anyway, so I re-generated it. Reviewed By: ndmitchell Differential Revision: D103356172 fbshipit-source-id: a4e073ed3a8a14b0994a05a5fbdc19a27e31b2f9

Simplify `finish_quantified_*` cluster of Solver functions Summary: Previously, the solver had five finish_quantified_* functions: three pub, one pub(crate), and one private. This diff gets us down to two: one pub and one private. It also adds a `finish_quantified` wrapper on AnswersSolver to reduce boilerplate, like we do already do with `is_subset_eq` and friends. This *is* a functional change: we now always create a Subset object and do overload pruning when calling `finish_quantified`. CI does not find any change to user-facing behavior. Reviewed By: yangdanny97 Differential Revision: D106052351 fbshipit-source-id: 8307e2ff6f1941b013d6b1d320073d48acea962e

Create `pyrefly/lib/alt/types/instance.rs` Summary: Moves `InstanceKind` and `Instance` into their own file in preparation for reusing them. No functional change. Reviewed By: grievejia Differential Revision: D105456425 fbshipit-source-id: 8c7b1765afe9ee156684c472e1c84d16da814aa4

Restore VariableNode when restoring var state Summary: Fixes a partial inference bug. Reviewed By: migeed-z Differential Revision: D100367789 fbshipit-source-id: 5f4bba65b940b53ec503cfd1f64333e1e57c2299

Fix all clippy warnings Summary: Looks like we got upgraded to a new Rust version. Warnings fixed: - **collapsible_match** (16 instances): Collapsed `if` conditions inside match arms into match guards in `ast.rs`, `folding.rs`, `simplify.rs`, `code_lens.rs`, `hover.rs`, `inlay_hints.rs`, `convert.rs`, `inline_variable.rs`, `pytest_fixture.rs`, `lsp.rs`, `extract.rs` - **unnecessary_sort_by** (3): Replaced `sort_by(|a, b| a.x.cmp(&b.x))` with `sort_by_key(|a| a.x)` in `report.rs`, `semantic_tokens.rs` - **while_let_loop** (3): Converted `loop { let Some(...) = ... else { break }; ... }` to `while let Some(...) = ... { ... }` in `server.rs`, `state.rs` - **dead_code** (1): Removed unused method `enclosing_class_and_metadata_keys` in `scope.rs` - **manual_checked_ops** (1): Replaced manual division-by-zero check with `checked_div` in `server.rs` - **useless_conversion** (1): Removed unnecessary `.into_iter()` in `dict_completions.rs` - **iter_kv_map** (1): Replaced `.iter().flat_map(|(h, _)| ...)` with `.keys().flat_map(|h| ...)` in `state.rs` - **Redundant guard** (1): Removed duplicate condition check in `simplify.rs` where the match guard already checked the same condition as the inner `if` Reviewed By: stroxler Differential Revision: D103764683 fbshipit-source-id: 0911b3141a9fe6abb171a82ea05153bcb02ca0f7

Read generic residuals from the call context Summary: Context: this stack migrates generic residuals from being stored on Variable to being stored on CallContext. This has several advantages: * This makes generic residuals look a lot more like overload residuals, which will enable code sharing in a later diff. * Separating residuals and variables makes the solver (a little) more modular. * At least to me, it makes more sense for the conceptual unit to be a *set* of residuals associated with a call, rather than a single residual describing a standalone property of a variable. This diff: switches `generalize_class_targs` and `finish_quantified_with_pruning` to read generic residual information from `GenericResidualCapture` on `CallContext` instead of the `residuals` field on `Variable::Quantified`. Remove the dual-write from the previous diff. One wrinkle is that during Callable subset checking, contravariant parameter comparisons negate the `argument_side`, which disables `record_deferred_residual_target_vars` for parameter-side vars. Those vars only appear in `witness_vars` indirectly — through union-find unification with vars that ARE in `witness_vars`. To handle this, we have to resolves vars through the union-find root before matching against `witness_vars`. Also stores generic residuals in overload branch snapshots, so `materialize_overload_residual_branch_value` can determine whether a snapshotted `Quantified` var should produce a `callable_residual_generic` type. Reviewed By: stroxler Differential Revision: D106163696 fbshipit-source-id: 1570cdb954f63fe6d829b94b5118178538b661f1

Collect lower bounds on Variable::Quantified and Variable::Unwrap (#2795) Summary: Pull Request resolved: https://github.com/facebook/pyrefly/pull/2795 Changes Variable::Quantified and Variable::Unwrap to collect lower bounds rather than solving immediately to the first encountered type. Because a Quantified can be solved to an Unwrap and vice versa, I had to update their behavior together to keep tests passing. This fixes a group of bugs related to eager pinning of type parameters. I left TODOs for remaining work. Note that we appear to regress on a couple of generics tests, but what's actually happening is that we stop reporting a false positive, but we still don't report the error that we're supposed to (pre-existing). The false positive previously fooled us into thinking the tests were passing. Fixes https://github.com/facebook/pyrefly/issues/345. Fixes https://github.com/facebook/pyrefly/issues/924. Fixes https://github.com/facebook/pyrefly/issues/2515. Fixes https://github.com/facebook/pyrefly/issues/2519. Fixes https://github.com/facebook/pyrefly/issues/2616. Ref https://github.com/facebook/pyrefly/issues/105. Reviewed By: stroxler Differential Revision: D96439545 fbshipit-source-id: e29b5582a5598b537ffad3962faff4f014bea8c6

Remove unnecessary parameters from `FuncMetadata::def` Summary: `FuncMetadata::def` took (module, cls, func, def_index), but really only needs to take cls and func. The module can be computed from the class, and def_index was None at every call site. Note: callers were previously inconsistent about whether we used the module from the class or the current module, but we really should be using the module from the class. Reviewed By: migeed-z Differential Revision: D103759635 fbshipit-source-id: ce16b175376a592406b7a43fe555680ffd1144d9

Merge lower bounds before unifying vars Summary: Variable unification preserves one of the variables and destroys the other, so make sure both have all lower bounds. For https://github.com/facebook/pyrefly/issues/105. Reviewed By: yangdanny97 Differential Revision: D96548067 fbshipit-source-id: e9ea002a4aa5c5aa3020768f8763372b0c3097db

Add ErrorBuilder and ErrorCollector::error_builder() [1/16] Summary: Pyrefly's error reporting API has grown unwieldy. ErrorCollector currently has two methods for emitting errors (add and add_with_annotations_and_quick_fixes) whose signatures grow every time we add a new optional decoration like annotations or quick fixes. This stack introduces a builder pattern (design: P2307835290, implementation plan: P2307836987) that replaces these methods with an error_builder() returning a chainable ErrorBuilder, making error construction explicit and extensible. This commit adds the new ErrorBuilder struct and ErrorCollector::error_builder() method alongside the existing API. ErrorBuilder is a #[must_use] builder with chainable with_detail, with_annotation, with_quick_fix, and with_context methods, terminated by emit(). When the collector's style is Never, the builder is inactive and all operations are no-ops. Reviewed By: migeed-z Differential Revision: D103990089 fbshipit-source-id: 0f8e8751b91c9d9b586b766cecb9479522e3ada8

Move `finalize_callable_residuals_at_boundary` and helpers into the types crate Summary: This stack: reorganizes some of the callable residual code to make it clearer which parts of the logic are tightly coupled to the solver, which will make it easier for me to make solver changes. This diff: moves more residual-related functions out of Solver and onto Type, now that their dependency on Solver::simplify_mut has been removed. I also took the opportunity to condense down from one public function + two helpers to one public function + one helper. With this move, a bunch of things no longer need to be `pub`. No functional change. Reviewed By: yangdanny97 Differential Revision: D106046346 fbshipit-source-id: 63ddeb4071b781b88ead83124db8a2e5e054a7f2

Store generic residuals on the call context in addition to vars Summary: Context: this stack migrates generic residuals from being stored on Variable to being stored on CallContext. This has several advantages: * This makes generic residuals look a lot more like overload residuals, which will enable code sharing in a later diff. * Separating residuals and variables makes the solver (a little) more modular. * At least to me, it makes more sense for the conceptual unit to be a *set* of residuals associated with a call, rather than a single residual describing a standalone property of a variable. This diff: store generic residuals on call context *as well as* on vars - that is, they are duplicatively stored in both places for now. Adds: - `GenericWitnessCapture` struct on `CallContext` (mirroring overload captures) - Accessor methods: `persist_generic_witness_capture`, `take_generic_witness_captures`, `generic_captured_vars` - `Drop` assertion to catch leaks - `with_outside_context` disconnects the new field (matching overload captures) - `record_generic_residuals_for_witness` dual-writes to both the call context and variable storage - `finish_quantified` drains the new captures (unused for now) All tests pass with no behavior change. The next diff will switch where consumers read generic residuals from. Reviewed By: stroxler Differential Revision: D106163685 fbshipit-source-id: 2e78461aea5c4f9636f123c79e9e966b105ebe3a

Resolve ambiguous overload calls by selecting the most general return type among the matched overloads (#2764) Summary: Previously, if a call to an overloaded function matched more than one overload and the return types of the matched overloads weren't all equivalent, we fell back to a return type of `Any`, as the spec says to do. With this diff, we instead try to select the "most general" return type among the matched overloads, by checking if there exists a return type such that all materializations of every other return type are assignable to it. Some examples of what this means: ```python overload def f(...) -> A[int]: ... overload def f(...) -> A[Any]: ... # Old return type: Any # New return type: A[Any] f(<ambiguous arguments>) ``` ```python overload def f(...) -> bool: ... overload def f(...) -> int: ... # Old return type: Any # New return type: int f(<ambiguous arguments>) ``` ```python overload def f(...) -> A[int]: ... overload def f(...) -> A[str]: ... # Old return type: Any # New return type: Any (neither return type is more general than the other) f(<ambiguous arguments>) ``` We now pass more of numpy and scipy-stubs' assert_type tests: | project | assert_type failures at head | assert_type failures at previous diff | assert_type failures at current diff | | numpy | 134 | 186 | 127 | | scipy-stubs | 49 | 101 | 19 | The trade-off is that we report a lot of new errors on mypy_primer projects. I investigated a bunch of them, and they're not wrong, per se, but some are arguably low-value. Examples: ``` def f(x: Sequence[str], idx) -> str: # The possible types of `x[idx]` are str (if idx is an int) # and Sequence[str] (if idx is a slice). str <: Sequence[str], # so we pick the latter as the return type. # This is technically correct...but idx is probably meant to be an int. return x[idx] # error! ``` ``` def f(x: bool, y) -> bool: # Similar to above, this is technically correct because y may be an int, # but y is probably meant to be a bool. return x & y # error! ``` Pull Request resolved: https://github.com/facebook/pyrefly/pull/2764 The LLM classifier seems to have gotten really confused on this one XD The detailed analysis says that Pyrefly is now "stricter than the established ecosystem standard", but we pass more of numpy and scipy-stubs' `assert_type` assertions than before, meaning this diff brings us closer to the ecosystem standard. Because the PR includes the whole stack, it's also complaining about errors introduced in previous diffs. Reviewed By: grievejia Differential Revision: D95667476 fbshipit-source-id: db2048424afdd3a8607cc65d2818b188760c0b37