impl use function by asukaminato0721 · Pull Request #2035 · facebook/pyrefly

asukaminato0721 · 2026-01-08T09:11:43Z

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

Copilot

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.

meta-codesync · 2026-02-03T14:11:33Z

@jvansch1 has imported this pull request. If you are a Meta employee, you can view this in D92157038.

jvansch1

Good start! I have a couple suggestions for some things that we can change.

Also, for these code action PRs it would be helpful if you could either include some instructions for how we can test these in our own IDE(we mostly use VScode) or a video showing how they work.

jvansch1 · 2026-02-03T14:20:05Z

@@ -0,0 +1,895 @@
+/*


I think most of the functions in this file need a docstring. A lot of these functions are quite large and it is not immediately clear what their purpose is.

jvansch1 · 2026-02-03T14:22:39Z

+    matches
+}
+
+fn match_expr(


Can we break this function up into smaller, easier to read functions. This function is over 300 lines and I think it will be easier to maintain if we can break this into smaller functions which are easier to reason about.

asukaminato0721 · 2026-02-03T19:28:47Z

Screencast_20260204_042421_c.webm

jvansch1 · 2026-02-09T14:08:29Z

@asukaminato0721 Looks like there is a merge conflict here. Once this gets updated I will do another review of this.

jvansch1 · 2026-02-09T15:04:40Z

+    selection: TextRange,
+) -> Option<Vec<LocalRefactorCodeAction>> {
+    let module_info = transaction.get_module_info(handle)?;
+    if transaction.is_third_party_module(&module_info, handle)


We don't have to do it in this PR, but it seems like this a check we are doing often. We check transaction.is_third_party_module(&target_info, &target_handle) && !transaction.is_source_file(&target_info, &target_handle) twice in this function and we also do it in lib/state/lsp.rs. At this point we can probably extract this out to a helper function.

Again not something that I think needs to be done in the PR but something that would improve code quality a bit.

jvansch1 · 2026-02-09T15:05:49Z

+    name: String,
+    params: Vec<String>,
+    param_set: HashSet<String>,
+    param_counts: HashMap<String, usize>,


Do we need all these representations of params? I can see how having the counts is useful, but wonder if we really need to have both params as a vector and as a set.

@asukaminato0721 Still wondering about this.

jvansch1 · 2026-02-09T15:07:36Z

+        let Stmt::Import(StmtImport { names, .. }) = stmt else {
+            continue;
+        };
+        for alias in names {


This is very similar to the block on line 405. Maybe a small helper function should be introduced to reduce repeated code? I know we are comparing a module in one case and a function in another case, so if it becomes too complicated to make it one function then I think it is fine to leave as is.

jvansch1 · 2026-02-09T15:11:00Z

+
+/// Matches dict expressions by comparing key/value pairs.
+fn match_dict_expr(
+    lhs: &ExprDict,


It looks like we have a lot of functions that do very similar things here with the main difference being the type of lhs and rhs. Maybe we could make this a generic function that has a match statement that could properly handle the logic for different types of lhs and rhs?

A single generic would likely recreate a big match internally

I think a match might be preferable In this case. I guess we either need to have

A lot of smaller functions like this

One huge function containing all the match arms

I don't think either great but if we had to choose one I think what we have here is better since at least we have named functions which describe what is happening. Overall there is a done of duplicate logic so it would be great to see if we could get rid of that if possible.

jvansch1

Can we also add some tests for the following cases:

A function with zero arguments
A function that has an argument with a default value.

jvansch1 · 2026-02-10T14:25:26Z

+
+/// Matches dict expressions by comparing key/value pairs.
+fn match_dict_expr(
+    lhs: &ExprDict,


I think a match might be preferable In this case. I guess we either need to have

A lot of smaller functions like this

One huge function containing all the match arms

I don't think either great but if we had to choose one I think what we have here is better since at least we have named functions which describe what is happening. Overall there is a done of duplicate logic so it would be great to see if we could get rid of that if possible.

jvansch1 · 2026-02-10T14:26:07Z

+    name: String,
+    params: Vec<String>,
+    param_set: HashSet<String>,
+    param_counts: HashMap<String, usize>,


@asukaminato0721 Still wondering about this.

jvansch1 · 2026-02-11T14:49:29Z

@asukaminato0721 I did see an issue with import when using this functionality. It seems like we are using the full import name rather than just importing the function we need. It would be nice if we could fix this before we merge this in.

import_issue_use_function.mov

jvansch1 · 2026-03-04T18:16:42Z

@asukaminato0721 I am going to try and finalize this review as well. Could you resolve the merge conflict when you have a chance?

asukaminato0721 · 2026-03-05T03:07:43Z

done

asukaminato0721 · 2026-03-22T07:20:01Z

done

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions · 2026-05-22T19:59:34Z

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions · 2026-05-23T19:27:55Z

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

meta-cla Bot added the cla signed label Jan 8, 2026

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

Copilot started reviewing on behalf of asukaminato0721 January 8, 2026 09:12 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

Comment thread pyrefly/lib/state/lsp/quick_fixes/use_function.rs Outdated

This comment has been minimized.

Sign in to view

stroxler assigned jvansch1 Jan 10, 2026

asukaminato0721 force-pushed the 364-1 branch 2 times, most recently from d6d8e85 to 4e819f1 Compare January 15, 2026 03:47

This comment has been minimized.

Sign in to view

asukaminato0721 force-pushed the 364-1 branch from 4e819f1 to bd55bd0 Compare January 22, 2026 18:42

This comment has been minimized.

Sign in to view

jvansch1 suggested changes Feb 3, 2026

View reviewed changes

asukaminato0721 force-pushed the 364-1 branch 2 times, most recently from b923145 to 54fe777 Compare February 3, 2026 19:11

This comment has been minimized.

Sign in to view

asukaminato0721 requested a review from jvansch1 February 9, 2026 14:43

This comment has been minimized.

Sign in to view

jvansch1 suggested changes Feb 9, 2026

View reviewed changes

asukaminato0721 force-pushed the 364-1 branch from e5fa47b to d43669d Compare February 9, 2026 16:57

This comment has been minimized.

Sign in to view

asukaminato0721 requested a review from jvansch1 February 10, 2026 06:08

jvansch1 suggested changes Feb 10, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

asukaminato0721 requested a review from jvansch1 February 11, 2026 03:26

asukaminato0721 force-pushed the 364-1 branch from 0895e58 to 35715f2 Compare February 11, 2026 16:41

asukaminato0721 force-pushed the 364-1 branch 2 times, most recently from 3d2fac8 to 1445f44 Compare March 5, 2026 02:25

This comment has been minimized.

Sign in to view

asukaminato0721 force-pushed the 364-1 branch from 1445f44 to f0afb0c Compare March 22, 2026 06:58

This comment has been minimized.

Sign in to view

asukaminato0721 and others added 11 commits May 20, 2026 14:46

impl use function

d47e483

Apply suggestion from @Copilot

08d5211

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

.

2eb3ab0

update

8c67847

update

5be000f

update

16b6c20

from x import y style

b356ebf

.

a7c2466

fix, add test

a08077c

fix one problem for now

7830c21

fix test

a4bf1e7

asukaminato0721 force-pushed the 364-1 branch from f0afb0c to a4bf1e7 Compare May 20, 2026 05:53

github-actions Bot added the size/xl label May 20, 2026

This comment has been minimized.

Sign in to view

github-actions Bot added size/xl and removed size/xl labels May 22, 2026

Merge branch 'main' into 364-1

7284e85

asukaminato0721 force-pushed the 364-1 branch from 2a5ce75 to 7284e85 Compare May 23, 2026 19:08

github-actions Bot added size/xl and removed size/xl labels May 23, 2026

Uh oh!

Conversation

asukaminato0721 commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

meta-codesync Bot commented Feb 3, 2026

jvansch1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

asukaminato0721 commented Feb 3, 2026

jvansch1 commented Feb 9, 2026

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

jvansch1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

jvansch1 commented Feb 11, 2026

jvansch1 commented Mar 4, 2026

This comment has been minimized.

asukaminato0721 commented Mar 5, 2026

This comment has been minimized.

asukaminato0721 commented Mar 22, 2026

This comment has been minimized.

github-actions Bot commented May 22, 2026

github-actions Bot commented May 23, 2026

Labels

4 participants

asukaminato0721 commented Jan 8, 2026 •

edited

Loading