fix Autocomplete parameters of class pattern in match-case situation #2830#2848
fix Autocomplete parameters of class pattern in match-case situation #2830#2848asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes LSP completion behavior for Python match/case class patterns by suggesting keyword names derived from the matched class’s instance attributes (e.g. dataclass fields in case A(...)).
Changes:
- Add completion logic to suggest
attr=items when completing insidecase <Class>(...). - Wire the new completion logic into the main completion flow (pattern-match keyword context + generic non-identifier context).
- Add a regression test ensuring dataclass fields (
a=,b=) are suggested incase A( ).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/lsp/wasm/completion.rs |
Adds match-class keyword completion generation and hooks it into completion dispatch. |
pyrefly/lib/test/lsp/completion.rs |
Adds a regression test covering dataclass-field keyword completions in class patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| &mut result, | ||
| in_string_literal, | ||
| ); | ||
| self.add_match_class_keyword_completions(handle, &nodes, &mut result); |
There was a problem hiding this comment.
add_match_class_keyword_completions is invoked in the generic identifier_at == None completion path, which means these attr= suggestions will appear anywhere the cursor is inside a PatternMatchClass node (including the pattern value position after =). This can surface invalid/noisy suggestions while the user is typing the subpattern (e.g. case A(a=|)), where attr= is not syntactically valid without inserting a comma. Consider gating this call to positions where a new keyword can start (e.g. when the previous non-whitespace character is ( or ,, or by checking AST/token context so the cursor is in the keyword-name slot, not inside the value subpattern).
| self.add_match_class_keyword_completions(handle, &nodes, &mut result); | |
| // Only offer match class keyword completions when the innermost | |
| // node at the cursor is a PatternMatchClass. This avoids | |
| // suggesting `attr=` in value positions such as `case A(a=|)`, | |
| // where a new keyword cannot legally start. | |
| if let Some(first) = nodes.first() | |
| && matches!(first, AnyNodeRef::PatternMatchClass(_)) | |
| { | |
| self.add_match_class_keyword_completions(handle, &nodes, &mut result); | |
| } |
| .map(|attr| { | ||
| RankedCompletion::new(CompletionItem { | ||
| label: format!("{}=", attr.name.as_str()), | ||
| detail: attr.ty.map(|ty| ty.to_string()), |
There was a problem hiding this comment.
The completion detail here uses Type::to_string(), while other completion paths in this file format types with as_lsp_string(LspDisplayMode::Hover) for consistency and readability. Consider using the same formatting helper so match-class keyword completions display types consistently with attribute completions.
| detail: attr.ty.map(|ty| ty.to_string()), | |
| detail: attr | |
| .ty | |
| .map(|ty| ty.as_lsp_string(LspDisplayMode::Hover)), |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #2830
adds class-pattern keyword completions from the matched class’s instance attributes, so case A(...): now suggests a= / b= for dataclass fields.
Test Plan
add test