Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Jan 8, 2026

Summary

Fixes part of #364

Pattern matcher and refactor edits targets single‑return, non‑decorated, non‑async top‑level functions with only positional params and return expressions that reference only those params (no and/or/ternary or chained comparisons to avoid evaluation‑order changes).

Hooked into LSP code actions and exposed as refactor.rewrite .

https://rope.readthedocs.io/en/latest/overview.html#use-function-refactoring

Test Plan

Added a multi‑module test and updated LSP capability expectations

@meta-cla meta-cla bot added the cla signed label Jan 8, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 8, 2026 09:11
Copilot AI review requested due to automatic review settings January 8, 2026 09:11
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

This PR implements a "use function" refactoring feature that identifies expressions matching a function's return pattern and offers to replace them with function calls. The pattern matcher targets single-return, non-decorated, non-async top-level functions with only positional parameters, avoiding boolean operators, ternary expressions, and chained comparisons to prevent evaluation-order changes.

Key changes:

  • Adds pattern matching logic to find and replace expressions that structurally match a function's return expression
  • Intelligently handles imports across modules, reusing existing imports or adding new ones while avoiding name conflicts
  • Exposes the refactoring as a new refactor.rewrite code action kind in the LSP

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pyrefly/lib/state/lsp/quick_fixes/use_function.rs New file implementing the core pattern matching and refactoring logic
pyrefly/lib/test/lsp/code_actions.rs Adds comprehensive tests covering cross-module refactoring, import reuse, shadowing, repeated parameters, and edge cases
pyrefly/lib/state/lsp/quick_fixes/mod.rs Registers the new use_function module
pyrefly/lib/state/lsp.rs Adds transaction method to access use_function code actions
pyrefly/lib/lsp/non_wasm/server.rs Integrates use_function into code actions handler and advertises REFACTOR_REWRITE capability
pyrefly/lib/test/lsp/lsp_interaction/basic.rs Updates LSP capability expectations to include refactor.rewrite
crates/pyrefly_config/src/config.rs Removes unnecessary lifetime annotation from function signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

This comment has been minimized.

@github-actions
Copy link

Diff from mypy_primer, showing the effect of this PR on open source code:

jinja (https://github.com/pallets/jinja)
- ERROR tests/test_async.py:647:31-62: Cannot set item in `dict[str, ((obj: Sized, /) -> int) | ((s: Any, /) -> Markup) | ((s: Any, /) -> str) | ((env: Environment, s: str, length: int = 255, killwords: bool = False, end: str = '...', leeway: int | None = None) -> str) | ((environment: Environment, obj: Any, name: str) -> Undefined | Any) | ((environment: Environment, s: str, width: int = 79, break_long_words: bool = True, wrapstring: str | None = None, break_on_hyphens: bool = True) -> str) | ((eval_ctx: EvalContext, d: Mapping[str, Any], autospace: bool = True) -> str) | ((eval_ctx: EvalContext, s: str, old: str, new: str, count: int | None = None) -> str) | ((eval_ctx: EvalContext, value: str, trim_url_limit: int | None = None, nofollow: bool = False, target: str | None = None, rel: str | None = None, extra_schemes: Iterable[str] | None = None) -> str) | ((eval_ctx: EvalContext, value: Any, indent: int | None = None) -> Markup) | ((s: str) -> int) | ((s: str) -> str) | ((s: str, width: int | str = 4, first: bool = False, blank: bool = False) -> str) | ((value: HasHTML | str) -> Markup) | ((value: HasHTML | str) -> str) | ((value: Iterable[tuple[str, Any]] | Mapping[str, Any] | str) -> str) | ((value: float | int | str, binary: bool = False) -> str) | ((value: float, precision: int = 0, method: Literal['ceil', 'common', 'floor'] = 'common') -> float) | ((value: str) -> Markup) | ((value: str, chars: str | None = None) -> str) | ((value: str, width: int = 80) -> str) | ((value: str, *args: Any, **kwargs: Any) -> str) | ((value: Any) -> str) | ((value: Any, default: float = ...) -> float) | ((value: Any, default: int = 0, base: int = 10) -> int) | ((*args: Unknown, **kwargs: Unknown) -> Unknown) | Overload[(context: Context, value: AsyncIterable[Any] | Iterable[Any], name: str, *args: Any, **kwargs: Any) -> Iterable[Any], (context: Context, value: AsyncIterable[Any] | Iterable[Any], *, attribute: str = ..., default: Any | None = None) -> Iterable[Any]] | Overload[(value: str) -> str, [V](value: Iterable[Unknown]) -> Iterable[Unknown]] | [K, V](value: Mapping[Unknown, Unknown], case_sensitive: bool = False, by: Literal['key', 'value'] = 'key', reverse: bool = False) -> list[tuple[Unknown, Unknown]] | [K, V](value: Mapping[Unknown, Unknown] | Undefined) -> Iterator[tuple[Unknown, Unknown]] | [V](value: Iterable[Unknown], linecount: int, fill_with: Unknown | None = None) -> Iterator[list[Unknown]] | [V](value: Unknown, default_value: Unknown = ..., boolean: bool = False) -> Unknown | [V](environment: Environment, seq: Reversible[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], case_sensitive: bool = False, attribute: int | str | None = None) -> Undefined | Unknown | [V](context: Context, seq: Sequence[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], reverse: bool = False, case_sensitive: bool = False, attribute: int | str | None = None) -> list[Unknown] | [_T](x: SupportsAbs[Unknown], /) -> Unknown]` [unsupported-operation]
+ ERROR tests/test_async.py:647:31-62: Cannot set item in `dict[str, ((obj: Sized, /) -> int) | ((s: Any, /) -> Markup) | ((s: Any, /) -> str) | ((env: Environment, s: str, length: int = 255, killwords: bool = False, end: str = '...', leeway: int | None = None) -> str) | ((environment: Environment, obj: Any, name: str) -> Undefined | Any) | ((environment: Environment, s: str, width: int = 79, break_long_words: bool = True, wrapstring: str | None = None, break_on_hyphens: bool = True) -> str) | ((eval_ctx: EvalContext, d: Mapping[str, Any], autospace: bool = True) -> str) | ((eval_ctx: EvalContext, s: str, old: str, new: str, count: int | None = None) -> str) | ((eval_ctx: EvalContext, value: str, trim_url_limit: int | None = None, nofollow: bool = False, target: str | None = None, rel: str | None = None, extra_schemes: Iterable[str] | None = None) -> str) | ((eval_ctx: EvalContext, value: Any, indent: int | None = None) -> Markup) | ((s: str) -> int) | ((s: str) -> str) | ((s: str, width: int | str = 4, first: bool = False, blank: bool = False) -> str) | ((value: HasHTML | str) -> Markup) | ((value: HasHTML | str) -> str) | ((value: Iterable[tuple[str, Any]] | Mapping[str, Any] | str) -> str) | ((value: float | int | str, binary: bool = False) -> str) | ((value: float, precision: int = 0, method: Literal['ceil', 'common', 'floor'] = 'common') -> float) | ((value: str) -> Markup) | ((value: str, chars: str | None = None) -> str) | ((value: str, width: int = 80) -> str) | ((value: str, *args: Any, **kwargs: Any) -> str) | ((value: Any) -> str) | ((value: Any, default: float = ...) -> float) | ((value: Any, default: int = 0, base: int = 10) -> int) | ((*args: Unknown, **kwargs: Unknown) -> Unknown) | Overload[(context: Context, value: AsyncIterable[Any] | Iterable[Any], name: str, *args: Any, **kwargs: Any) -> Iterable[Any], (context: Context, value: AsyncIterable[Any] | Iterable[Any], *, attribute: str = ..., default: Any | None = None) -> Iterable[Any]] | Overload[(value: str) -> str, [V](value: Iterable[Unknown]) -> Iterable[Unknown]] | [K, V](value: Mapping[Unknown, Unknown] | Undefined) -> Iterator[tuple[Unknown, Unknown]] | [K, V](value: Mapping[Unknown, Unknown], case_sensitive: bool = False, by: Literal['key', 'value'] = 'key', reverse: bool = False) -> list[tuple[Unknown, Unknown]] | [V](value: Iterable[Unknown], linecount: int, fill_with: Unknown | None = None) -> Iterator[list[Unknown]] | [V](value: Unknown, default_value: Unknown = ..., boolean: bool = False) -> Unknown | [V](environment: Environment, seq: Reversible[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], case_sensitive: bool = False, attribute: int | str | None = None) -> Undefined | Unknown | [V](context: Context, seq: Sequence[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], reverse: bool = False, case_sensitive: bool = False, attribute: int | str | None = None) -> list[Unknown] | [_T](x: SupportsAbs[Unknown], /) -> Unknown]` [unsupported-operation]
- ERROR tests/test_regression.py:87:34-66: Cannot set item in `dict[str, ((obj: Sized, /) -> int) | ((s: Any, /) -> Markup) | ((s: Any, /) -> str) | ((env: Environment, s: str, length: int = 255, killwords: bool = False, end: str = '...', leeway: int | None = None) -> str) | ((environment: Environment, obj: Any, name: str) -> Undefined | Any) | ((environment: Environment, s: str, width: int = 79, break_long_words: bool = True, wrapstring: str | None = None, break_on_hyphens: bool = True) -> str) | ((eval_ctx: EvalContext, d: Mapping[str, Any], autospace: bool = True) -> str) | ((eval_ctx: EvalContext, s: str, old: str, new: str, count: int | None = None) -> str) | ((eval_ctx: EvalContext, value: str, trim_url_limit: int | None = None, nofollow: bool = False, target: str | None = None, rel: str | None = None, extra_schemes: Iterable[str] | None = None) -> str) | ((eval_ctx: EvalContext, value: Any, indent: int | None = None) -> Markup) | ((s: str) -> int) | ((s: str) -> str) | ((s: str, width: int | str = 4, first: bool = False, blank: bool = False) -> str) | ((value: HasHTML | str) -> Markup) | ((value: HasHTML | str) -> str) | ((value: Iterable[tuple[str, Any]] | Mapping[str, Any] | str) -> str) | ((value: float | int | str, binary: bool = False) -> str) | ((value: float, precision: int = 0, method: Literal['ceil', 'common', 'floor'] = 'common') -> float) | ((value: str) -> Markup) | ((value: str, chars: str | None = None) -> str) | ((value: str, width: int = 80) -> str) | ((value: str, *args: Any, **kwargs: Any) -> str) | ((value: Any) -> str) | ((value: Any, default: float = ...) -> float) | ((value: Any, default: int = 0, base: int = 10) -> int) | ((*args: Unknown, **kwargs: Unknown) -> Unknown) | Overload[(context: Context, value: AsyncIterable[Any] | Iterable[Any], name: str, *args: Any, **kwargs: Any) -> Iterable[Any], (context: Context, value: AsyncIterable[Any] | Iterable[Any], *, attribute: str = ..., default: Any | None = None) -> Iterable[Any]] | Overload[(value: str) -> str, [V](value: Iterable[Unknown]) -> Iterable[Unknown]] | [K, V](value: Mapping[Unknown, Unknown], case_sensitive: bool = False, by: Literal['key', 'value'] = 'key', reverse: bool = False) -> list[tuple[Unknown, Unknown]] | [K, V](value: Mapping[Unknown, Unknown] | Undefined) -> Iterator[tuple[Unknown, Unknown]] | [V](value: Iterable[Unknown], linecount: int, fill_with: Unknown | None = None) -> Iterator[list[Unknown]] | [V](value: Unknown, default_value: Unknown = ..., boolean: bool = False) -> Unknown | [V](environment: Environment, seq: Reversible[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], case_sensitive: bool = False, attribute: int | str | None = None) -> Undefined | Unknown | [V](context: Context, seq: Sequence[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], reverse: bool = False, case_sensitive: bool = False, attribute: int | str | None = None) -> list[Unknown] | [_T](x: SupportsAbs[Unknown], /) -> Unknown]` [unsupported-operation]
+ ERROR tests/test_regression.py:87:34-66: Cannot set item in `dict[str, ((obj: Sized, /) -> int) | ((s: Any, /) -> Markup) | ((s: Any, /) -> str) | ((env: Environment, s: str, length: int = 255, killwords: bool = False, end: str = '...', leeway: int | None = None) -> str) | ((environment: Environment, obj: Any, name: str) -> Undefined | Any) | ((environment: Environment, s: str, width: int = 79, break_long_words: bool = True, wrapstring: str | None = None, break_on_hyphens: bool = True) -> str) | ((eval_ctx: EvalContext, d: Mapping[str, Any], autospace: bool = True) -> str) | ((eval_ctx: EvalContext, s: str, old: str, new: str, count: int | None = None) -> str) | ((eval_ctx: EvalContext, value: str, trim_url_limit: int | None = None, nofollow: bool = False, target: str | None = None, rel: str | None = None, extra_schemes: Iterable[str] | None = None) -> str) | ((eval_ctx: EvalContext, value: Any, indent: int | None = None) -> Markup) | ((s: str) -> int) | ((s: str) -> str) | ((s: str, width: int | str = 4, first: bool = False, blank: bool = False) -> str) | ((value: HasHTML | str) -> Markup) | ((value: HasHTML | str) -> str) | ((value: Iterable[tuple[str, Any]] | Mapping[str, Any] | str) -> str) | ((value: float | int | str, binary: bool = False) -> str) | ((value: float, precision: int = 0, method: Literal['ceil', 'common', 'floor'] = 'common') -> float) | ((value: str) -> Markup) | ((value: str, chars: str | None = None) -> str) | ((value: str, width: int = 80) -> str) | ((value: str, *args: Any, **kwargs: Any) -> str) | ((value: Any) -> str) | ((value: Any, default: float = ...) -> float) | ((value: Any, default: int = 0, base: int = 10) -> int) | ((*args: Unknown, **kwargs: Unknown) -> Unknown) | Overload[(context: Context, value: AsyncIterable[Any] | Iterable[Any], name: str, *args: Any, **kwargs: Any) -> Iterable[Any], (context: Context, value: AsyncIterable[Any] | Iterable[Any], *, attribute: str = ..., default: Any | None = None) -> Iterable[Any]] | Overload[(value: str) -> str, [V](value: Iterable[Unknown]) -> Iterable[Unknown]] | [K, V](value: Mapping[Unknown, Unknown] | Undefined) -> Iterator[tuple[Unknown, Unknown]] | [K, V](value: Mapping[Unknown, Unknown], case_sensitive: bool = False, by: Literal['key', 'value'] = 'key', reverse: bool = False) -> list[tuple[Unknown, Unknown]] | [V](value: Iterable[Unknown], linecount: int, fill_with: Unknown | None = None) -> Iterator[list[Unknown]] | [V](value: Unknown, default_value: Unknown = ..., boolean: bool = False) -> Unknown | [V](environment: Environment, seq: Reversible[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], case_sensitive: bool = False, attribute: int | str | None = None) -> Undefined | Unknown | [V](context: Context, seq: Sequence[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], reverse: bool = False, case_sensitive: bool = False, attribute: int | str | None = None) -> list[Unknown] | [_T](x: SupportsAbs[Unknown], /) -> Unknown]` [unsupported-operation]
- ERROR tests/test_security.py:182:30-33: Cannot set item in `dict[str, ((obj: Sized, /) -> int) | ((s: Any, /) -> Markup) | ((s: Any, /) -> str) | ((env: Environment, s: str, length: int = 255, killwords: bool = False, end: str = '...', leeway: int | None = None) -> str) | ((environment: Environment, obj: Any, name: str) -> Undefined | Any) | ((environment: Environment, s: str, width: int = 79, break_long_words: bool = True, wrapstring: str | None = None, break_on_hyphens: bool = True) -> str) | ((eval_ctx: EvalContext, d: Mapping[str, Any], autospace: bool = True) -> str) | ((eval_ctx: EvalContext, s: str, old: str, new: str, count: int | None = None) -> str) | ((eval_ctx: EvalContext, value: str, trim_url_limit: int | None = None, nofollow: bool = False, target: str | None = None, rel: str | None = None, extra_schemes: Iterable[str] | None = None) -> str) | ((eval_ctx: EvalContext, value: Any, indent: int | None = None) -> Markup) | ((s: str) -> int) | ((s: str) -> str) | ((s: str, width: int | str = 4, first: bool = False, blank: bool = False) -> str) | ((value: HasHTML | str) -> Markup) | ((value: HasHTML | str) -> str) | ((value: Iterable[tuple[str, Any]] | Mapping[str, Any] | str) -> str) | ((value: float | int | str, binary: bool = False) -> str) | ((value: float, precision: int = 0, method: Literal['ceil', 'common', 'floor'] = 'common') -> float) | ((value: str) -> Markup) | ((value: str, chars: str | None = None) -> str) | ((value: str, width: int = 80) -> str) | ((value: str, *args: Any, **kwargs: Any) -> str) | ((value: Any) -> str) | ((value: Any, default: float = ...) -> float) | ((value: Any, default: int = 0, base: int = 10) -> int) | ((*args: Unknown, **kwargs: Unknown) -> Unknown) | Overload[(context: Context, value: AsyncIterable[Any] | Iterable[Any], name: str, *args: Any, **kwargs: Any) -> Iterable[Any], (context: Context, value: AsyncIterable[Any] | Iterable[Any], *, attribute: str = ..., default: Any | None = None) -> Iterable[Any]] | Overload[(value: str) -> str, [V](value: Iterable[Unknown]) -> Iterable[Unknown]] | [K, V](value: Mapping[Unknown, Unknown], case_sensitive: bool = False, by: Literal['key', 'value'] = 'key', reverse: bool = False) -> list[tuple[Unknown, Unknown]] | [K, V](value: Mapping[Unknown, Unknown] | Undefined) -> Iterator[tuple[Unknown, Unknown]] | [V](value: Iterable[Unknown], linecount: int, fill_with: Unknown | None = None) -> Iterator[list[Unknown]] | [V](value: Unknown, default_value: Unknown = ..., boolean: bool = False) -> Unknown | [V](environment: Environment, seq: Reversible[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], case_sensitive: bool = False, attribute: int | str | None = None) -> Undefined | Unknown | [V](context: Context, seq: Sequence[Unknown]) -> Undefined | Unknown | [V](environment: Environment, value: Iterable[Unknown], reverse: bool = False, case_sensitive: bool = False, attribute: int | str | None = None) -> list[Unknown] | [_T](x: SupportsAbs[Unknown], /) -> Unknown]` [unsupported-operation]

... (truncated 9 lines) ...

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.

2 participants