Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rename request handler #2462

Merged
merged 3 commits into from
Nov 21, 2023
Merged

feat: rename request handler #2462

merged 3 commits into from
Nov 21, 2023

Conversation

digama0
Copy link
Collaborator

@digama0 digama0 commented Aug 25, 2023

This implements a request handler for the textDocument/rename LSP request, enabling renames via F2. It handles both local renames (e.g. let x := 1; x to let y := 1; y) as well as global renames (definitions).

Unfortunately it does not work for "orphan" files outside a project, as it uses ilean data for the current file and this does not seem to be saved for orphan files. As a result, the test file does not work, although one can manually test the implementation against a project such as mathlib. (This issue already exists for the "references" request, e.g. ctrl click on the first x in let x := 1; x takes you to the second one only if you are not in an orphan file.)

@eric-wieser
Copy link
Contributor

eric-wieser commented Aug 25, 2023

How does this handle a rename that starts with

namespace Foo

def bar := sorry

end Foo

example := Foo.bar

and renames bar on the third line to _root_.Baz.bar?

@digama0
Copy link
Collaborator Author

digama0 commented Aug 25, 2023

It's not particularly clever about namespaces, it will replace all occurrences of things that resolve to Foo.bar with the text you give it. (And it's difficult to do much better than this given that the only information we have is def/ref information regarding identifiers in other files. We would need full elaboration results to know the set of open namespaces at a random point in a random other file.) So that example would be turned into:

namespace Foo

def _root_.Baz.bar := sorry

end Foo

example := _root_.Baz.bar

Probably you don't want to be using _root_ in renames. I would be inclined to just use the name without namespaces, or maybe the fully qualified name, and then manually fix any issues.

@eric-wieser
Copy link
Contributor

eric-wieser commented Aug 25, 2023

Does that mean that in

namespace Foo
def bar := sorry
end Foo

open Foo
example := bar

renaming bar to baz changes bar to Foo.baz?

@digama0
Copy link
Collaborator Author

digama0 commented Aug 25, 2023

renaming bar to baz in that example would just do exactly that, both occurrences of bar would change to baz. Namespaces are resolved in the input (because they have previously been processed by lean), meaning that it won't get confused about a different bar that resolves to something else, but the output is just naive text substitution, it just puts whatever you say as the replacement text.

@mhuisi mhuisi added the awaiting-author Waiting for PR author to address issues label Sep 12, 2023
@github-actions github-actions bot added the stale label Oct 20, 2023
@github-actions github-actions bot removed the stale label Nov 2, 2023
@eric-wieser
Copy link
Contributor

Just noting that this would be a super helpful feature for refactoring mathlib

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Nov 15, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Nov 15, 2023

  • ❗ Mathlib CI can not be attempted yet, as the 'nightly-testing-2023-11-15' branch does not exist there yet. We will retry when you push more commits. It may be necessary to rebase onto 'nightly' tomorrow. (2023-11-15 11:09:21)
  • ✅ Mathlib branch lean-pr-testing-2462 has successfully built against this PR. (2023-11-16 02:51:56) View Log
  • ✅ Mathlib branch lean-pr-testing-2462 has successfully built against this PR. (2023-11-21 10:11:50) View Log

@digama0 digama0 requested a review from mhuisi November 16, 2023 00:40
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 16, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Nov 16, 2023
@Kha Kha removed request for Kha and leodemoura November 16, 2023 09:12
Copy link
Contributor

@mhuisi mhuisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce the second issue I found anymore either. This PR is a great start, hopefully we will be able to resolve some of the other issues later!

@digama0 digama0 requested a review from mhuisi November 20, 2023 23:17
@digama0
Copy link
Collaborator Author

digama0 commented Nov 21, 2023

the error looks very strange, likely intermittent, and looks similar to this zulip issue

@mhuisi
Copy link
Contributor

mhuisi commented Nov 21, 2023

Yes, the failing test is most likely intermittent. Sebastian is looking into it.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 21, 2023
@mhuisi mhuisi merged commit b97b0ad into leanprover:master Nov 21, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jan 25, 2024
This PR adds support for the "call hierarchy" feature of LSP that allows
quickly navigating both inbound and outbound call sites of functions. In
this PR, "call" is taken to mean "usage", so inbound and outbound
references of all kinds of identifiers (e.g. functions or types) can be
navigated. To implement the call hierarchy feature, this PR implements
the LSP requests `textDocument/prepareCallHierarchy`,
`callHierarchy/incomingCalls` and `callHierarchy/outgoingCalls`.

<details>
  <summary>Showing the call hierarchy (click to show image)</summary>
  

![show_call_hierarchy](https://github.com/leanprover/lean4/assets/10852073/add13943-013c-4d0a-a2d4-a7c57ad2ae26)
  
</details>

<details>
  <summary>Incoming calls (click to show image)</summary>
  

![incoming_calls](https://github.com/leanprover/lean4/assets/10852073/9a803cb4-6690-42b4-9c5c-f301f76367a7)
  
</details>

<details>
  <summary>Outgoing calls (click to show image)</summary>
  

![outgoing_calls](https://github.com/leanprover/lean4/assets/10852073/a7c4f193-51ab-4365-9473-0309319b1cfe)
  
</details>

It is based on #3159, which should be merged before this PR.

To route the parent declaration name through to the language server, the
`.ilean` format is adjusted, breaking backwards compatibility with
version 1 of the ILean format and yielding version 2.

This PR also makes the following more minor adjustments:
- `Lean.Server.findModuleRefs` now also combines the identifiers of
constants and FVars and prefers constant over FVars for the combined
identifier. This is necessary because e.g. declarations declared using
`where` yield both a constant (for usage outside of the function) and an
FVar (for usage inside of the function) with the same range, whereas we
would typically like all references to refer to the former. This also
fixes a bug introduced in #2462 where renaming a declaration declared
using `where` would not rename usages outside of the function, as well
as a bug in the unused variable linter where `where` declarations would
be reported as unused even if they were being used outside of the
function.
- The function converting `Lean.Server.RefInfo` to `Lean.Lsp.RefInfo`
now also computes the `Lean.DeclarationRanges` for parent declaration
names via `MetaM` and must hence be in `IO` now.
- Add a utility function `Array.groupByKey` to `HashMap.lean`.
- Stylistic refactoring of `Watchdog.lean` and `LanguageFeatures.lean`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author Waiting for PR author to address issues builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactoring support in vs code
4 participants