Conversation
asukaminato0721
changed the title
There was a problem hiding this comment.
Pull request overview
Adds an “Unwrap block” refactor code action to the Pyrefly LSP, enabling users to remove an enclosing block statement (e.g., if, else, for/while without else, with) and reindent the block body into the parent scope.
Changes:
- Implemented
unwrap_blockquick-fix/refactor logic, including AST traversal and text edit construction. - Wired the new refactor into
Transactionand the non-wasm LSP server’s code-action collection. - Added initial LSP code-action tests for unwrap-block (basic
if,else, and loop-else rejection).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pyrefly/lib/test/lsp/code_actions.rs |
Adds unwrap-block helpers and a few targeted tests. |
pyrefly/lib/state/lsp/quick_fixes/unwrap_block.rs |
New unwrap-block refactor implementation (target detection + edit generation). |
pyrefly/lib/state/lsp/quick_fixes/mod.rs |
Exposes the new unwrap_block module. |
pyrefly/lib/state/lsp.rs |
Adds Transaction::unwrap_block_code_actions API entrypoint. |
pyrefly/lib/lsp/non_wasm/server.rs |
Includes unwrap-block in refactor code-action computation/telemetry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let line = source[line_start..line_end].trim_end_matches('\n'); | ||
| if !line.trim().is_empty() { | ||
| let indent_len = line | ||
| .chars() | ||
| .take_while(|ch| *ch == ' ' || *ch == '\t') | ||
| .map(char::len_utf8) | ||
| .sum::<usize>(); | ||
| if line.get(..indent_len) == Some(outer_indent) && line.rfind(':').is_some() { | ||
| let colon = line.rfind(':').expect("checked above"); | ||
| return TextSize::try_from(line_start + colon).ok(); | ||
| } |
There was a problem hiding this comment.
find_suite_colon uses line.rfind(':') on the full line text, so a trailing comment containing a colon (e.g. if x: # note: ...) can cause the refactor to pick the wrong colon position and never offer the action at the actual suite colon. Consider stripping/ignoring the comment portion when searching for the suite colon (or using a token-based approach), and avoid requiring an exact indent match so continued headers (e.g. with \ line continuations) still work when the : is on a more-indented header line.
| assert_no_unwrap_block_action(code, selection); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
The unwrap-block implementation supports more constructs than the current tests cover (e.g., with, while without else, and multi-line headers). Adding a few targeted tests for these cases (and for a header with a trailing comment containing :) would help prevent regressions in the colon/indent detection logic.
| #[test] | |
| #[test] | |
| fn unwrap_block_with_statement() { | |
| let code = r#" | |
| def f(): | |
| with open("x.txt") as f: | |
| print(f.read()) | |
| "#; | |
| let selection = find_nth_range(code, ":", 2); | |
| let updated = | |
| apply_first_unwrap_block_action(code, selection).expect("expected unwrap-block action"); | |
| let expected = r#" | |
| def f(): | |
| print(f.read()) | |
| "#; | |
| assert_eq!(expected.trim(), updated.trim()); | |
| } | |
| #[test] | |
| fn unwrap_block_while_without_else() { | |
| let code = r#" | |
| def f(): | |
| while True: | |
| print("tick") | |
| "#; | |
| let selection = find_nth_range(code, ":", 2); | |
| let updated = | |
| apply_first_unwrap_block_action(code, selection).expect("expected unwrap-block action"); | |
| let expected = r#" | |
| def f(): | |
| print("tick") | |
| "#; | |
| assert_eq!(expected.trim(), updated.trim()); | |
| } | |
| #[test] | |
| fn unwrap_block_multiline_header() { | |
| let code = r#" | |
| def f(): | |
| if ( | |
| True | |
| ): | |
| print("then") | |
| "#; | |
| let selection = find_nth_range(code, ":", 2); | |
| let updated = | |
| apply_first_unwrap_block_action(code, selection).expect("expected unwrap-block action"); | |
| let expected = r#" | |
| def f(): | |
| print("then") | |
| "#; | |
| assert_eq!(expected.trim(), updated.trim()); | |
| } | |
| #[test] | |
| fn unwrap_block_header_comment_with_colon() { | |
| let code = r#" | |
| def f(): | |
| if True: # comment with : | |
| print("then") | |
| "#; | |
| let selection = find_nth_range(code, ":", 2); | |
| let updated = | |
| apply_first_unwrap_block_action(code, selection).expect("expected unwrap-block action"); | |
| let expected = r#" | |
| def f(): | |
| print("then") | |
| "#; | |
| assert_eq!(expected.trim(), updated.trim()); | |
| } | |
| #[test] |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
part of #2207
impl unwrap block
astral-sh/ruff#24552
Test Plan
add test