impl Implemented encapsulate_field #2207

[asukaminato0721]

Summary

Fixes part of #2207

Implemented encapsulate_field as a new local rewrite refactor. It now offers a code action on attribute selections, inserts unique get_<field> / set_<field> methods, rewrites reads to getter calls, rewrites assignments and += to setter calls, and rejects unsupported tuple-assignment targets. It’s wired into the transaction and LSP code-action pipeline.

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

Test Plan

Added focused tests for read/write rewriting, unique accessor naming, and tuple-target rejection.

[Copilot]

Pull request overview

Adds a new local “Encapsulate field” refactor to the Pyrefly LSP pipeline, generating getter/setter methods on a class field and rewriting local reads/writes to use those accessors.

Changes:

• Implemented encapsulate_field local refactor (generate accessor methods + rewrite reads/writes and +=).
• Wired the refactor into Transaction and the non-wasm LSP server code-action path.
• Added focused LSP code-action tests for read/write rewriting, unique naming, and tuple-target rejection.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pyrefly/lib/state/lsp/quick_fixes/encapsulate_field.rs	New refactor implementation (edits generation + method insertion).
pyrefly/lib/state/lsp/quick_fixes/mod.rs	Exposes the new quick-fix module.
pyrefly/lib/state/lsp.rs	Adds Transaction::encapsulate_field_code_actions entrypoint.
pyrefly/lib/lsp/non_wasm/server.rs	Adds the refactor to the server’s timed refactor action pipeline.
pyrefly/lib/test/lsp/code_actions.rs	Adds tests for the new refactor behavior.

---

💡 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]

handled_stmt_ranges is checked before classifying the occurrence, so once a write in a statement is handled (usually the LHS, since references are sorted by start position), all later references in the same statement are skipped. This leaves un-rewritten reads in RHS expressions (e.g. self.value = self.value becomes self.set_value(self.value)), breaking encapsulation. Consider classifying first, then only dedup write occurrences per statement (or track handled write-target ranges instead of entire statement ranges).

[Copilot]

The definition filter attempts to exclude methods/functions/classes via definition.metadata.symbol_kind(), but DefinitionMetadata::Attribute always reports SymbolKind::Attribute, even when the resolved attribute is actually a method/property. As a result, this refactor can be offered on obj.method and will rewrite obj.method() into something invalid like obj.get_method()(). Suggest explicitly rejecting attribute definitions whose definition_range is within a Stmt::FunctionDef/Stmt::ClassDef (via Ast::locate_node), or otherwise verifying the definition comes from an assignment/AnnAssign to a field rather than a callable/class member.

[Copilot]

Current tests don’t cover the case where a write statement also contains reads of the same attribute (e.g. self.value = self.value or self.value += self.value). Given references are processed in source order, this scenario currently risks leaving RHS reads un-rewritten; adding a regression test here would prevent this from reappearing.

Suggested change

	#[test]
	#[test]
	fn encapsulate_field_rewrites_rhs_reads_in_write() {
	let code = r#"
	class Counter:
	def __init__(self, value):
	self.value = value
	def copy(self):
	self.value = self.value
	# ^
	def double(self):
	self.value += self.value
	"#;
	let updated =
	apply_first_encapsulate_field_action(code).expect("expected encapsulate field action");
	let expected = r#"
	class Counter:
	def __init__(self, value):
	self.value = value
	def copy(self):
	self.set_value(self.get_value())
	def double(self):
	self.set_value(self.get_value() + self.get_value())
	def get_value(self):
	return self.value
	def set_value(self, value):
	self.value = value
	# ^
	"#;
	assert_eq!(expected.trim(), updated.trim());
	}
	#[test]

[Copilot]

Consider adding a test ensuring encapsulate-field is not offered for method attributes (e.g. cursor on obj.method / obj.method()), since the refactor currently treats any resolved DefinitionMetadata::Attribute as a field and could generate invalid rewrites for callables/properties.

Suggested change

	}
	}
	#[test]
	fn encapsulate_field_not_offered_for_method_attribute() {
	let code = r#"
	class Counter:
	def __init__(self, value):
	self.value = value
	def inc(self):
	self.value += 1
	def use(counter: Counter):
	method_ref = counter.inc
	# ^
	"#;
	assert_no_encapsulate_field_action(code);
	}
	#[test]
	fn encapsulate_field_not_offered_for_method_call() {
	let code = r#"
	class Counter:
	def __init__(self, value):
	self.value = value
	def inc(self):
	self.value += 1
	def use(counter: Counter):
	counter.inc()
	# ^
	"#;
	assert_no_encapsulate_field_action(code);
	}

[asukaminato0721]

@jvansch1 this can be reviewed

[jvansch1]

@asukaminato0721 Looks like there are a lot of failing checks. Once these are fixed I can do a review.

[jvansch1]

@asukaminato0721 As part of the description of this PR could you also add either a video showing this new functionality or instructions for how to test it?

[asukaminato0721]

Screencast_20260421_012325.webm

[github-actions]

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