Rename file when renaming module

[scottming]

This PR is an implementation of renaming the file while renaming the module.

It first determines whether the module meets certain hard constraints, such as having no parent, then performs some conventional checks. If both are satisfied, it proceeds to return the file renaming.

https://github.com/lexical-lsp/lexical/assets/12830256/3d9d0221-fe3f-43be-8a51-3121b375d65d

[scohen]

In the end, don't we just want the module name to match the file path? I was thinking this could be accomplished by indexing the file with the Module extractor, then, for each module, getting its name, and if it's not in the right file, doing one of the following:

If there is one module in the file:

1. If the file has the correct name, then do nothing
2. If the file name or path is incorrect, determine the right path and issue a RenameFIle, then emit edits for the file with the new contents.

If there is more than one module in the file:

1. Determine if any of the modules have the correct name
2. If one does, emit an edit for the current file with the others removed
3. For all modules in the current file that don't have the right name, follow the above steps.

There's a caveat here, since there can be global aliases / imports, etc. in the file, and if there are, they need to be relocated to the new files as well.

[scottming]

In the end, don't we just want the module name to match the file path?

Yes, essentially, that's exactly what we need to do.

I was thinking this could be accomplished by indexing the file with the Module extractor

If we can achieve that, it would certainly be ideal, but these steps also sound quite challenging.

[scohen]

Why do you think it's challenging?

[scottming]

Why do you think it's challenging?

Perhaps I was overthinking it at the time, feeling that there were too many conventions. However, if we start with some simple conventions, like in this PR, then the complexity should be manageable.

[scohen]

Why do we care if there are parents or siblings before we rename a file?

[scottming]

Why do we care if there are parents or siblings before we rename a file?

When there are some sibling modules at the first level, we usually don't use one of those modules as a mapping for the file name.

Regarding the issue of parent modules, consider examples like state modules inside GenServer modules.

There are tests for these cases.

[scohen]

Regarding the issue of parent modules, consider examples like state modules inside GenServer modules.

This is why I was suggesting using the indexer, you can index the file with the module extractor, and see if there are multiple modules in the file (as well as get their hierarchy). If there are, you would treat the file as the parent module, and only rename that.

There are multiple cases for this, of course. You can have a file whose name has no bearing on the modules contained inside as well.

[scottming]

This is why I was suggesting using the indexer, you can index the file with the module extractor, and see if there are multiple modules in the file (as well as get their hierarchy)

To be honest, I'm not sure exactly how to do it. It sounds like I need to add some fields to Indexer.Entry, right? And append this information when Entry.block_definition is called. Do you want me to take care of this in this PR?

[scohen]

It sounds like I need to add some fields to Indexer.Entry, right?

No, I don't think so. You just need to run the indexer on the document, giving it only the module extractor, and it will return all the module definitions in the document. You can then look at the entries to find out if they're nested (you can get all the top-level modules by collecting those modules whose block_id is :root. any others are nested.

to index using a subset of extractors, do the following

document
modules =
  |> Indexer.Source.index([Indexer.Extractors.Module])
  |> Enum.reject(& &1.type == :metadata)

[scohen]

Did using Build.with_lock not work for this case?

[scohen]

@scottming tried renaming Lexical.Ast to Lexical.AST again in both vscode and emacs, and both failed in different ways, but both brought down the language server.

I don't think this approach of opening things temporarily will work --at least not with the default timeouts.

[scohen]

I tried this branch this weekend. Renamed Lexical.Ast to Lexical.AST.

Fails pretty hard in emacs. Lots of errors about documents not being open. I can get you stack traces if you want

Works in vscode, but I see a bunch of errors like the following:

17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.046 [error] Formatter failed "Cannot format while renaming"
17:33:37.047 [error] Formatter failed "Cannot format while renaming"
17:33:37.047 [error] Formatter failed "Cannot format while renaming"
17:33:37.047 [error] Formatter failed "Cannot format while renaming"
17:33:37.048 [error] Formatter failed "Cannot format while renaming"
17:33:37.048 [error] Formatter failed "Cannot format while renaming"
17:33:37.048 [error] Formatter failed "Cannot format while renaming"

The formatter should detect these errors, but you've made it tougher by making the reason a string.

[scohen]

Here are the errors I encountered in emacs:

2024-04-15T17:29:32.801242-07:00 error: Error in process <0.4908.5> on node 'project-lexical-29783@127.0.0.1' with exit value:, {#{'__exception__' => true,'__struct__' => 'Elixir.Module.Types.Error',message => <<"found error while checking  (UndefinedFunctionError) function Lexical.Ast.Analysis.__struct__/0 is undefined (module Lexical.Ast.Analysis is not available)">>},[
  {'Elixir.Lexical.Ast.Analysis','__struct__',[],[]},
{'Elixir.Module.Types.Of',struct,3,[{file,"lib/module/types/of.ex"},{line,131}]},
{'Elixir.Module.Types.Pattern',of_shared,4,[{file,"lib/module/types/pattern.ex"},{line,754}]},{'Elixir.Module.Types.Pattern',of_pattern,3,[{file,"lib/module/types/pattern.ex"},{line,47}]},{'Elixir.Module.Types.Helpers',do_map_reduce_ok,3,[{file,"lib/module/types/helpers.ex"},{line,93}]},{'Elixir.Module.Types.Pattern',of_head,4,[{file,"lib/module/types/pattern.ex"},{line,12}]},{'Elixir.Module.Types',warnings_from_clause,6,[{file,"lib/module/types.ex"},{line,62}]},{'Elixir.Module.Types','-warnings/5-fun-0-',8,[{file,"lib/module/types.ex"},{line,22}]}]}

Lots of these:

17:29:33.161 [info] No formatter found in /Users/steve/Projects/lexical/apps/common/test/lexical/ast/detection/alias_test.exs error was false
17:29:33.162 [info] No formatter found in /Users/steve/Projects/lexical/apps/common/test/lexical/ast/detection error was false
17:29:33.165 [info] No formatter found in /Users/steve/Projects/lexical/apps/common/test/lexical/ast error was false
17:29:33.165 [info] No formatter found in /Users/steve/Projects/lexical/apps/common/test/lexical error was false
17:29:33.166 [info] No formatter found in /Users/steve/Projects/lexical/apps/common/test error was false
17:29:33.169 [info] found formatter in /Users/steve/Projects/lexical/apps/common

Then TONS of these:

18:12:48.311 [error] Failed to handle Elixir.LXical.Protocol.Notifications.DidClose, {{:error, :not_open}, %LXical.Server.State{configuration: %LXical.Server.Configuration{project: %LXical.Project{root_uri: "file:///Users/steve/Projects/lexical", mix_exs_uri: "file:///Users/steve/Projects/lexical/mix.exs", mix_project?: true, mix_env: nil, mix_target: nil, env_variables: %{}, project_module: nil, entropy: 4195, project_config: []}, support: %LXical.Server.Configuration.Support{code_action_dynamic_registration: true, hierarchical_symbols: true, snippet: true, deprecated: true, tags: false, signature_help: #Protocol.Types.SignatureHelp.ClientCapabilities<[dynamic_registration: true, signature_information: #Protocol.Types.SignatureHelp.ClientCapabilities.SignatureInformation<[parameter_information: #Protocol.Types.SignatureHelp.ClientCapabilities.ParameterInformation<[label_offset_support: true]>]>]>, work_done_progress: true}, client_name: "emacs", additional_watched_extensions: nil, dialyzer_enabled?: false}, initialized?: true, shutdown_received?: false, in_flight_requests: %{"12" => {%LXical.Protocol.Requests.CodeLensRefresh{lsp: %LXical.Protocol.Requests.CodeLensRefresh.LSP{id: "12", jsonrpc: "2.0", method: "workspace/codeLens/refresh"}, id: "12", jsonrpc: "2.0", method: "workspace/codeLens/refresh"}, #Function<0.7576677/2 in LXical.Server.server_request/1>}}}}
18:12:48.311 [error] Could not handle message LXical.Protocol.Notifications.DidClose :ok

I also notice that you're emitting file renames even if the file names haven't changed. This could also be messing stuff. up. Examples:

    {
      "kind": "rename",
      "newUri": "file:///Users/steve/Projects/lexical/apps/common/lib/lexical/ast/detection/pipe.ex",
      "oldUri": "file:///Users/steve/Projects/lexical/apps/common/lib/lexical/ast/detection/pipe.ex",
      "options": {
        "overwrite": true
      }
    },
    {
      "kind": "rename",
      "newUri": "file:///Users/steve/Projects/lexical/apps/common/lib/lexical/ast/detection/spec.ex",
      "oldUri": "file:///Users/steve/Projects/lexical/apps/common/lib/lexical/ast/detection/spec.ex",
      "options": {
        "overwrite": true
      }
    },

I think these are unnecessary, and will likely cause emacs to freak out.

Emacs then opens all the documents and does code lens requests on them, followed by a bunch of didOpen requests, followed by didChange requests. I then start seeing cancellation requests for the codeLens requests intermixed with the didChange notifications. It appears that emacs sends the cancellation request immediately after the code lens request.

Emacs does seem to be requesting notifications in the following order: didOpen -> didChange -> didSave -> didClose If this is the case, why do we need to open things temporarily?

[scottming]

I also notice that you're emitting file renames even if the file names haven't changed. This could also be messing stuff. up.

Good catch, Fixed.

Emacs does seem to be requesting notifications in the following order: didOpen -> didChange -> didSave -> didClose If this is the case, why do we need to open things temporarily?

So, are you suggesting that in this PR, we should replace 'Store.open_temporarily' with 'Store.open' everywhere?

[scohen]

So, are you suggesting that in this PR, we should replace 'Store.open_temporarily' with 'Store.open' everywhere?

Logically, all editors should be doing the same actions, so yes. Are they not?