fix empty case and dict case of key autocompletion #1886 #2838#2843
fix empty case and dict case of key autocompletion #1886 #2838#2843asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Improves LSP autocompletion for TypedDict construction in Python code, especially when parsing incomplete dict literals that Ruff represents as sets (e.g. {""}), and when constructing dicts via dict(...) kwargs.
Changes:
- Treat Ruff’s
{""}error-recovery shape (ExprSet) as a dict-key placeholder to enable key completion. - Recover contextual
TypedDicttype information from surrounding assignment/call sites when the literal itself doesn’t infer as aTypedDict. - Add
dict(...)-constructor kwargs completions for contextualTypedDictfields, plus tests covering these cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/state/lsp/dict_completions.rs |
Extends dict-key completion to handle ExprSet placeholders and adds contextual type recovery + dict(...) kwargs completion. |
pyrefly/lib/lsp/wasm/completion.rs |
Wires in the new dict(...)-constructor kwargs completion and exposes expected_call_argument_type for reuse. |
pyrefly/lib/test/lsp/completion.rs |
Adds regression tests for incomplete literal {""}, call-argument {""}, and dict( kwargs completion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // For key access we query the container expression; for literals we query the | ||
| // literal itself to pick up contextual TypedDict typing from assignments. | ||
| if let Some(base_type) = self.get_type_trace(handle, context.base_range()) | ||
| if let Some(base_type) = | ||
| self.contextual_typed_dict_type(handle, module, position, context.base_range()) | ||
| && let Some(typed_keys) = self.collect_typed_dict_keys(handle, base_type) |
There was a problem hiding this comment.
contextual_typed_dict_type is now used for both dict literals and key-access contexts. For key access (u[""] / d[""]), falling back to the enclosing call/assignment can produce incorrect TypedDict key suggestions even when the base expression is not a TypedDict (e.g. cfg: Config = data[""] could start suggesting Config fields for the key into data). Consider restricting the call/assignment fallback to the dict-literal construction path only (or gating it behind context.base_expr().is_none()), and keep key-access completion typed keys derived solely from the base expression’s own type trace.
| let Some(call) = | ||
| Ast::locate_node(module, position) | ||
| .into_iter() | ||
| .find_map(|node| match node { | ||
| AnyNodeRef::ExprCall(call) | ||
| if call.arguments.range.contains_inclusive(position) | ||
| && matches!( | ||
| call.func.as_ref(), | ||
| Expr::Name(name) if name.id.as_str() == "dict" | ||
| ) => | ||
| { |
There was a problem hiding this comment.
add_typed_dict_constructor_kwargs_completions triggers solely on Expr::Name("dict"). This will also fire if dict is shadowed (e.g., a local function/variable named dict), producing TypedDict-field completions in calls that aren’t the built-in dict constructor. Consider resolving the callee via bindings/exports (or otherwise confirming it refers to builtins.dict) before adding these completions.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #1886 #2838
The dict-key matcher now treats Ruff’s error-recovery {""} shape as a placeholder key position, then recovers the contextual TypedDict from the enclosing assignment/call when the literal itself only infers as set[str].
also added a specialized dict(...) kwargs path so contextual TypedDict fields show up as name= / age= completions in the existing kwargs flow.
Test Plan
add test