fix Pyrefly inserted typing import in sub-optimal location #1490 #1580
Conversation
asukaminato0721
force-pushed
the
1490
branch
from
e9cb6aa to
c26aa04
Compare
asukaminato0721
force-pushed
the
1490
branch
from
c26aa04 to
8c0a1be
Compare
asukaminato0721
force-pushed
the
1490
branch
from
df5401f to
e3180ab
Compare
| report.push_str(" with text edit: "); | ||
| report.push_str(&format!("{:?}", &text_edit)); | ||
| } | ||
| if let Some(documentation) = documentation { |
There was a problem hiding this comment.
We were trying to match the historical golden strings for completion reports.
Previously, every entry, even plain keywords or literal values, had a blank line appended because the reporter always added one after the optional docstring block. That produced extra empty lines compared to the expected fixtures, which is why dozens of completion tests started failing.
The adjustment keeps the docstring formatting logic intact, but stops unconditionally inserting that trailing blank line unless there was actually extra content to separate. This wa,y the rendered report matches the snapshots again without altering runtime behavior.
| { | ||
| return edit; | ||
| } | ||
| let position = if let Some(first_stmt) = ast.body.iter().find(|stmt| !is_docstring_stmt(stmt)) { |
There was a problem hiding this comment.
I wonder if there is some way we can avoid traversing the entire AST again? It might lead to slow performance for large modules.
There was a problem hiding this comment.
One way to avoid the second walk would be to fold both concerns into a single pass (track the insertion position while scanning for compatible imports)
Or to compute the insertion point once before calling try_extend_existing_from_import and pass it in.
asukaminato0721
force-pushed
the
1490
branch
from
efd8d5f to
5af20b8
Compare
2 participants
fix #1490
Adjusted so insert_import_edit* returns an ImportEdit with both the actual edit and a stable display string, and added logic to append new symbols to an existing matching from ... import ... statement instead of duplicating the import line. The helper now shares this richer info with callers so UI text stays human-readable.
Updated LSP code to consume the new struct, ensuring code-action titles and auto-import completion details still show from module import name while the actual edit mutates the existing line in place.
Taught the infer command to handle the new edit type and to skip empty/duplicate insertions when mutating files.
Revised the LSP code-action test to assert the merged-import behavior.