impl code action to convert dict to TypedDict / dataclass / pydantic #2144#2483
impl code action to convert dict to TypedDict / dataclass / pydantic #2144#2483asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements a new LSP refactor code action that generates a model definition (TypedDict, dataclass, or Pydantic BaseModel) from a selected dict literal, including import insertion and basic field-type inference, and wires it into the server and test suite.
Changes:
- Added
dict_definitionquick-fix implementation to generate TypedDict/dataclass/Pydantic models from dict literals. - Exposed the new action through
Transactionand registered it in the non-wasm LSP server refactor pipeline. - Added test coverage for basic behavior, Any-import behavior, and rejection of non-literal keys.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pyrefly/lib/state/lsp/quick_fixes/dict_definition.rs |
Core implementation for dict-to-model code actions, including import insertion and type-to-annotation rendering. |
pyrefly/lib/state/lsp/quick_fixes/mod.rs |
Registers the new dict_definition quick-fix module. |
pyrefly/lib/state/lsp.rs |
Adds Transaction::dict_definition_code_actions API entrypoint. |
pyrefly/lib/lsp/non_wasm/server.rs |
Wires the new refactor into the server’s refactor code action collection (with telemetry timing). |
pyrefly/lib/test/lsp/code_actions.rs |
Adds tests validating the generated edits and titles for the new actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(first_stmt) = ast.body.iter().find(|stmt| !is_docstring_stmt(stmt)) { | ||
| first_stmt.range().start() | ||
| } else { | ||
| ast.range.end() | ||
| } |
There was a problem hiding this comment.
import_insertion_point currently inserts new imports before the first non-docstring statement. If the module begins with from __future__ import ..., this will insert imports above the future import, which is invalid in Python (future imports must come first after the module docstring/comments). Update the insertion logic to place new imports after any leading __future__ imports (and docstring), e.g. scan initial ast.body statements and advance the insertion position past all Stmt::ImportFrom where module == "__future__".
| if let Some(first_stmt) = ast.body.iter().find(|stmt| !is_docstring_stmt(stmt)) { | |
| first_stmt.range().start() | |
| } else { | |
| ast.range.end() | |
| } | |
| for stmt in &ast.body { | |
| // Skip the module docstring, if present. | |
| if is_docstring_stmt(stmt) { | |
| continue; | |
| } | |
| // Skip any leading `from __future__ import ...` statements, which must | |
| // remain the first imports after the docstring/comments. | |
| if let Stmt::ImportFrom(import_from) = stmt { | |
| if let Some(module) = &import_from.module { | |
| if module.id.as_str() == "__future__" { | |
| continue; | |
| } | |
| } | |
| } | |
| return stmt.range().start(); | |
| } | |
| ast.range.end() |
| if !seen_names.insert(key.clone()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When a dict literal has the same string key multiple times, Python keeps the last value, but this code keeps the first occurrence by continue-ing on duplicates. That can generate a model field type that doesn’t match runtime behavior. Consider either (a) overwriting the previously collected field with the later annotation, or (b) rejecting dict literals with duplicate keys so the action can’t produce a misleading schema.
| format!("class {name}"), | ||
| format!("def {name}"), | ||
| format!("{name} ="), | ||
| format!("{name}\t="), |
There was a problem hiding this comment.
name_conflicts only checks a few raw-text patterns (e.g. "{name} =") and will miss existing bindings like annotated assignments (Data: int = 1 / Data: int) or other syntactic forms where the name is already defined. That can cause unique_name to pick a class name that collides with an existing binding. Consider expanding the patterns to include annotated-assignment forms (e.g. "{name}:") or, more robustly, deriving conflicts from the parsed AST symbol names instead of substring matching.
| format!("{name}\t="), | |
| format!("{name}\t="), | |
| format!("{name}:"), | |
| format!("{name} :"), |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #2144
Implemented dict-to-model refactor code actions (TypedDict, dataclass, Pydantic) and wired them into LSP refactors, with imports and scoped insertion logic.
note: to keep simple, currently nest case is not considered（fallback to dict）. Will impl in the future pr.
Test Plan
Added coverage for the new actions.