Skip to content

fix empty case and dict case of key autocompletion #1886 #2838#2843

Open
asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
asukaminato0721:1886
Open

fix empty case and dict case of key autocompletion #1886 #2838#2843
asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
asukaminato0721:1886

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Mar 21, 2026

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

@meta-cla meta-cla bot added the cla signed label Mar 21, 2026
@asukaminato0721 asukaminato0721 changed the title fix We get completion if there's an existing key-value in the dictionary literal, but completion doesn't work if it's an empty literal (ie, {""} doesn't complete, {"": ""} does). fix empty case and dict case of key autocompletion #1886 Mar 21, 2026
@asukaminato0721 asukaminato0721 changed the title fix empty case and dict case of key autocompletion #1886 fix empty case and dict case of key autocompletion #1886 #2838 Mar 21, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review March 21, 2026 03:38
Copilot AI review requested due to automatic review settings March 21, 2026 03:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TypedDict type information from surrounding assignment/call sites when the literal itself doesn’t infer as a TypedDict.
  • Add dict(...)-constructor kwargs completions for contextual TypedDict fields, 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.

Comment on lines 463 to 467
// 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)
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +500 to +510
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"
) =>
{
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSP: Provide key autocompletion on annotated TypedDict

2 participants