-
Notifications
You must be signed in to change notification settings - Fork 255
impl introduce-parameter refactor #2037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements a new "introduce-parameter" code action that extracts a selected expression into a function parameter and automatically updates all call sites. The implementation handles various Python function types including regular functions, methods, classmethods, and staticmethods, and correctly rejects refactoring when the expression uses local variables or when call sites use variadic arguments.
Key Changes:
- New introduce-parameter refactoring with single and "replace all occurrences" variants
- Automatic call site updates with proper parameter substitution
- Support for keyword-only parameter insertion when functions use
*argsor have keyword-only parameters - Rejection of unsafe refactorings (local variable usage, variadic call arguments)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pyrefly/lib/state/lsp/quick_fixes/introduce_parameter.rs |
Core implementation of introduce-parameter refactor with parameter insertion, expression template substitution, and call site updating |
pyrefly/lib/state/lsp/quick_fixes/mod.rs |
Module declaration for the new introduce_parameter module |
pyrefly/lib/state/lsp.rs |
Public API method to invoke introduce-parameter code actions |
pyrefly/lib/lsp/non_wasm/server.rs |
Integration of introduce-parameter actions into LSP server code action handler |
pyrefly/lib/test/lsp/code_actions.rs |
Comprehensive test suite covering basic functionality, method handling, edge cases, and rejection scenarios |
crates/pyrefly_config/src/config.rs |
Benign removal of unnecessary lifetime annotation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| KeywordOnly, | ||
| } | ||
|
|
||
| /// Builds introduce-parameter refactor actions for a selected expression. |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function documentation is incomplete compared to similar functions in the codebase. Consider expanding it to explain: 1) what the function does (extracts an expression into a parameter and updates call sites), 2) when it returns None (e.g., when the selection is empty, not an exact expression, uses local variables, in parameter list, uses variadic parameters), and 3) what actions are returned (single replacement vs. replace-all-occurrences). See extract_variable_code_actions for a good example of comprehensive documentation.
| /// Builds introduce-parameter refactor actions for a selected expression. | |
| /// Builds "introduce parameter" refactor code actions for the selected expression. | |
| /// | |
| /// This refactor extracts the selected expression into a new parameter of the | |
| /// enclosing function or method, and updates the corresponding call sites to | |
| /// pass the extracted expression as an argument. | |
| /// | |
| /// Returns `None` when no valid refactor is available. This includes cases | |
| /// where: | |
| /// - the selection is empty or contains only whitespace, | |
| /// - the selection does not correspond to an exact expression in the AST, | |
| /// - the selected expression cannot be safely turned into a parameter | |
| /// (for example, because it depends on local variables, is located in a | |
| /// parameter list, or conflicts with variadic parameters). | |
| /// | |
| /// When successful, returns a list of [`LocalRefactorCodeAction`]s. Typically | |
| /// this will contain actions for introducing a parameter at the current | |
| /// occurrence of the expression, and (when applicable) introducing a | |
| /// parameter for all matching occurrences within the same function. |
This comment has been minimized.
This comment has been minimized.
f103862 to
c50a255
Compare
Add a new introduce-parameter code action that inserts a parameter, replaces the selected expression, and updates call sites. Includes method/classmethod/staticmethod handling and rejects *args/**kwargs. Adds comprehensive LSP code action tests for the new behavior.
1618be0 to
c5f4aef
Compare
This comment has been minimized.
This comment has been minimized.
c5f4aef to
4e2c4c6
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Add a new introduce-parameter code action that inserts a parameter, replaces the selected expression, and updates call sites. Includes method/classmethod/staticmethod handling and rejects *args/**kwargs.
Fixes part of #364
https://www.jetbrains.com/help/pycharm/introduce-parameter.html#extract-constant-dialog
Test Plan
Adds comprehensive LSP code action tests for the new behavior.