fix Renaming a kwarg does not work across files. #1583

[asukaminato0721]

Summary

Fixes #1583

Updated cross-module rename logic so keyword-argument references are included when the parameter definition lives in a different file. This now resolves keyword args by name, then refines against the definition module’s AST to match the exact parameter.

Test Plan

Added a cross-file rename test and fixtures to cover kwarg renames across modules

[Copilot]

Pull request overview

Fixes #1583 by extending workspace rename/reference discovery so that keyword-argument usages are included even when the parameter definition is in a different module, and adds an LSP interaction test/fixtures covering the cross-file kwarg rename scenario.

Changes:

• Update cross-module reference collection to include kwarg references tied back to a parameter definition in another file.
• Add a new rename interaction test plus a minimal multi-file fixture (defs.py/uses.py) for kwarg renames across files.

Reviewed changes

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

Show a summary per file

File	Description
pyrefly/lib/state/lsp.rs	Extends external-definition reference collection to also find/refine kwarg references against the definition module AST.
pyrefly/lib/test/lsp/lsp_interaction/rename.rs	Adds an integration-style LSP rename test for cross-file kwarg renames.
pyrefly/lib/test/lsp/lsp_interaction/test_files/rename_kwargs_across_files/defs.py	Defines a function with a parameter intended to be renamed.
pyrefly/lib/test/lsp/lsp_interaction/test_files/rename_kwargs_across_files/uses.py	Calls the function using keyword arguments to validate cross-file rename edits.
pyrefly/lib/test/lsp/lsp_interaction/test_files/rename_kwargs_across_files/pyrefly.toml	Fixture config to make the local directory importable for the test scenario.

Comments suppressed due to low confidence (1)

pyrefly/lib/test/lsp/lsp_interaction/rename.rs:210

• This test opens both defs.py and uses.py before issuing the rename. If the intended behavior is to support workspace-wide rename for kwarg references in files that aren’t currently opened (which is how other rename tests validate cross-file edits), consider adding a variant that does not did_open("uses.py") to ensure the new kwarg reference logic works when the referencing module’s AST isn’t already resident.

    interaction.client.did_open("defs.py");
    interaction.client.did_open("uses.py");

---

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

[Copilot]

keyword_argument_references_from_parameter_definition ultimately depends on collect_local_keyword_arguments_by_name(handle, ...), which returns an empty list when get_ast(handle) is unavailable. In this codebase, ASTs can be evicted after later pipeline steps (see ComputeGuard::evict_ast in state/module.rs), so for unopened modules (or modules whose AST was evicted) cross-file kwarg references will still be missed and rename won’t update those files. Consider adding the same fallback used for definition_ast: if get_ast(handle) is None, re-parse the referencing module from get_module_info(handle)?.contents() and use that AST to collect keyword args, so rename works even when the file wasn’t opened.

[meta-codesync]

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

[kinto0]

Review automatically exported from Phabricator review in Meta.

[github-actions]

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

[kinto0]

commented here