fix remove or downgrade the already appear branch in match case #3015#3179
fix remove or downgrade the already appear branch in match case #3015#3179asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Improves LSP attribute completion ranking inside Python match value patterns by demoting enum members that were already used in earlier case arms (fixes #3015).
Changes:
- Add a new
is_deprioritizedranking flag and incorporate it intosort_textgeneration. - Detect
matchvalue-pattern attribute completion, collect previously-matched enum members, and mark corresponding completions as deprioritized. - Add a regression test for the new ranking behavior in
matchcases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pyrefly/lib/lsp/wasm/completion.rs |
Adds deprioritization support to completion ranking and wires match-case enum-member demotion into the attribute-completion path. |
pyrefly/lib/test/lsp/completion.rs |
Adds a regression test intended to verify that already-matched enum members are ranked lower during completion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let report = get_batched_lsp_operations_report_allow_error( | ||
| &[("main", code)], | ||
| |state, handle, position| { | ||
| let mut report = String::new(); | ||
| for item in state | ||
| .transaction() | ||
| .completion(handle, position, ImportFormat::Absolute, true, None) | ||
| .into_iter() | ||
| .filter(|item| matches!(item.label.as_str(), "AA" | "BB")) | ||
| { | ||
| report.push_str(&item.label); | ||
| report.push('\n'); | ||
| } | ||
| report | ||
| }, | ||
| ); | ||
|
|
||
| let bb_index = report.find("BB\n"); | ||
| let aa_index = report.find("AA\n"); | ||
| assert!( | ||
| bb_index.is_some() && aa_index.is_some(), | ||
| "Expected completions for AA and BB." | ||
| ); | ||
| assert!( | ||
| bb_index.unwrap() < aa_index.unwrap(), | ||
| "Expected the unmatched enum member to sort first." | ||
| ); |
There was a problem hiding this comment.
This test’s ordering assertion is unreliable because get_batched_lsp_operations_report_allow_error prepends a code frame and the source itself contains AA/BB (e.g. BB = auto() and case A.AA:). As a result, report.find("BB\n") / report.find("AA\n") can match the code frame rather than the completion output, making the test pass even if completion ranking is wrong. Consider asserting against the full expected report (like other tests in this file), or add an unambiguous sentinel header in the closure output and restrict the search to that section before comparing ordering.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #3015
now detect when attribute completion is happening inside a match value pattern, collect enum members already covered by earlier case arms, and demote those members in the completion ranking.
The ranking hook is wired and applied in the attribute-completion path.
Test Plan
add test