[asukaminato0721]

Summary

Fixes part of #2207

ref rope.refactor.method_object: Transform a function or a method to a method object.

https://rope.readthedocs.io/en/latest/library.html#list-of-refactorings

Test Plan

[Copilot]

Pull request overview

Implements a new LSP refactoring code action to convert a selected (sync) Python function/method into a “method object” pattern, and wires it into the server code-action pipeline with accompanying tests.

Changes:

• Add method_object quick-fix/refactor implementation and expose it via Transaction.
• Register the new refactor in the non-wasm LSP server’s refactor code actions.
• Add LSP code-action tests for basic function/method conversions and eligibility rules.

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 a helper to apply the new refactor and introduces tests for method-object transformations.
pyrefly/lib/state/lsp/quick_fixes/mod.rs	Exports the new method_object quick-fix module.
pyrefly/lib/state/lsp/quick_fixes/method_object.rs	Implements the method-object refactor (rewrite body + insert generated class).
pyrefly/lib/state/lsp.rs	Exposes method_object_code_actions on Transaction.
pyrefly/lib/lsp/non_wasm/server.rs	Registers the new refactor code action with telemetry timing.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

function_body_range() starts at the first statement in the function body, which means a leading docstring will be removed from the original function and instead become the __call__ docstring in the generated method-object class. This changes the refactored function’s __doc__ (and can affect tooling that reads docstrings). Consider detecting/removing the docstring stmt from the moved body and keeping it in the wrapper function (e.g., keep the docstring line(s) and only replace the rest of the body, or prepend the docstring before the generated return NewMethodObject(...)() statement).

Suggested change

	let first_body_stmt = selected.function_def.body.first()?;
	let body_range = function_body_range(source, selected.function_def)?;
	let body = &selected.function_def.body;
	let first_body_stmt = body.first()?;
	let full_body_range = function_body_range(source, selected.function_def)?;
	// Detect and preserve a leading docstring statement, if present.
	// We consider the first statement a docstring if its source snippet
	// begins with a string literal (with optional prefixes such as r, u, f, or b).
	let mut has_leading_docstring = false;
	if let Some(first_stmt) = body.first() {
	let start = first_stmt.range().start();
	let end = first_stmt.range().end();
	let start_usize = start.to_usize();
	let end_usize = end.to_usize();
	if let Some(snippet) = source.get(start_usize..end_usize) {
	let trimmed = snippet.trim_start();
	let is_string_literal = trimmed.starts_with('"')
	|| trimmed.starts_with('\'')
	|| trimmed.starts_with("r\"")
	|| trimmed.starts_with("r'")
	|| trimmed.starts_with("R\"")
	|| trimmed.starts_with("R'")
	|| trimmed.starts_with("u\"")
	|| trimmed.starts_with("u'")
	|| trimmed.starts_with("U\"")
	|| trimmed.starts_with("U'")
	|| trimmed.starts_with("f\"")
	|| trimmed.starts_with("f'")
	|| trimmed.starts_with("F\"")
	|| trimmed.starts_with("F'")
	|| trimmed.starts_with("b\"")
	|| trimmed.starts_with("b'")
	|| trimmed.starts_with("B\"")
	|| trimmed.starts_with("B'");
	if is_string_literal {
	has_leading_docstring = true;
	}
	}
	}
	// Adjust the body range so that we do not move or replace the docstring.
	// If the function only has a docstring, avoid performing the refactor.
	let body_range = if has_leading_docstring {
	if body.len() == 1 {
	return None;
	}
	let non_docstring_start = body[1].range().start();
	TextRange::new(non_docstring_start, full_body_range.end())
	} else {
	full_body_range
	};

[Copilot]

For instance methods, collect_parameter_infos() keeps stored_name == "self" but renames the init parameter to host, which leads to generated code like self.self = host and later self.self.base. This is syntactically valid but very confusing and produces an oddly named attribute that’s easy to misread. Prefer storing the receiver under a dedicated attribute name (e.g. self.host/self._host) and update the reference-rewrite logic accordingly so method bodies become self.host.base etc.

[asukaminato0721]

@jvansch1 this can be reviewed

[meta-codesync]

This pull request has been imported. If you are a Meta employee, you can view this in D113979820. (Because this pull request was imported automatically, there will not be any future comments.)

[codspeed]

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 10 untouched benchmarks

---

_{Comparing asukaminato0721:2207-method-obj (e39f159) with main (0109ebf)}

[codspeed]

Unable to generate the flame graphs

The performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists.

[github-actions]

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

[stroxler]

I'm going to close this PR, it's too old and attached to a stale diff which makes it harder to review.

I'm actually not sure whether it's worth reopening - you can if you want to, but the value of these automated refactors is probably much lower now than when the issue was filed, because coding agents mean most of this tooling isn't really used anymore; thus far, agents generally ignore automated refactors even when available to them.

---

If you do decide to open a fresh PR (I would consider not doing so in this case if I were you), here are some suggestions to improve it beyond the fact that it doesn't compile today:

• One-line functions such as def f(x): return x are transformed into invalid code because the replacement range includes the def line.
• Nested functions lose captured variables because the generated class is moved to module scope; nonlocal can make the result syntactically invalid.
• Parameter rewriting is based on go-to-definition rather than lexical bindings. Reassigned parameters can remain bare names and cause UnboundLocalError or NameError.
• Moving method bodies into another class breaks zero-argument super(), __class__, and private-name mangling such as self.__value.
• Reindenting the moved source changes the contents of multiline string literals.
• Generated names are not collision-safe:
  • Positional-only self produces def __init__(self, self, ...).
  • Existing or nested self bindings can refer to the wrong object.
  • A parameter or import named NewMethodObject collides with the generated class.
• The original function’s docstring moves into __call__, changing function.__doc__.
• Debug f-strings change observable output: f"{a=}" becomes f"{self.a=}".
• The tests cover only a simple function, a simple method, and selection eligibility; they miss these scope, binding, and source-preservation cases.