Change file name when change class name

[jacobggman]

Every time that I want to refactor a name of a class in my code I need to change the class name and also change the file name. I think this is a waste of time to change the name of the same thing twice, this is an another step to make refactor harder. For example, I have a class that have the name of NodeKey and I want to change the class name to FileKey I need to change the class name and the file name from node_key.dart to file_key.dart.

I thing that this process need to be automated and it will same time for many users and make refactoring easier. On Change Symble that change class name the extension will check if the file name have the same name as the changed class name (taking into consideration the name conventions of UpperCamelCase for class names and lowercase_with_underscores for file names) and if they have the same name change also the file name to the new name (with the name conventions).

Alternative solutions are: use existing extensions that does it, if you know any, or this is already possible in this extension and I don't know about it or make a new extension that does it.

I will be happy to implement this if it is possible :) If possible you know any file/files you think that I need to change to make this feature?

[DanTup]

I think it's a fair request. There are two places it could be implemented:

1. Directly in this extension, use LSP middleware to sniff when a rename happens and trigger the file rename
2. In the LSP servers rename handler which could then include the rename as part of the WorkspaceEdit it returns

I think the second one makes most sense (cc @bwilkerson for his opinion) since other LSP clients would also benefit. In either case, I think there needs to be a setting to control this (for ex. a nullable boolean, where null means prompt the user, and true/false would just always do that - the prompt could include options for "this time only"/"always"/"never" which would set the option accordingly). We can prompt the user from the server using LSP's window/showMessageRequest.

[jacobggman]

I also think the second approach is better. From what I understand the extension is manage the saving and the passing of the parameters for the handler, so the dart code doesn't need to save the "user preferences". Am I right?

Also, I fork the dart SDK repo to make the changes and basically I finish writing the code but I haven't tested it yet. I am not sure how do I test if what I did is working. I tried to build the analysis_server project, but it depends on those projects:

analysis_server -> analyzer_plugin -> analyzer_utilities -> analyzer

And I can't build analyzer_utilities:

Because test <1.3.0 requires SDK version >=1.8.0 <2.0.0-∞ and test >=1.16.0-nullsafety <1.16.0-nullsafety.8 requires SDK version >=2.10.0-0 <2.12.0, test <1.3.0-∞ or >=1.16.0-nullsafety <1.16.0-nullsafety.8 is forbidden.
And because test >=1.16.0-nullsafety.8 <1.16.0-nullsafety.12 depends on yaml ^2.0.0 and test >=1.3.0 <1.16.0-nullsafety depends on yaml ^2.0.0, test <1.16.0-nullsafety.12 requires yaml ^2.0.0.
And because test >=1.16.0-nullsafety.10 <1.16.1 depends on analyzer >=0.36.0 <0.42.0 and test >=1.16.1 <1.16.6 depends on analyzer >=0.39.5 <2.0.0, test <1.16.6 requires yaml ^2.0.0 or analyzer from hosted.
And because every version of analyzer from path depends on yaml ^3.0.0 and test >=1.16.6 depends on analyzer from hosted, analyzer from path is incompatible with test.
So, because analyzer_utilities depends on both analyzer from path and test any, version solving failed.
exit code 1

Also can you take a look at my code and check if what I did it fine? I am not sure if the file paths will be working and the way I got them and not sure about the return of WorkspaceEdit, I need to return two WorkspaceEdits?

[DanTup]

From what I understand the extension is manage the saving and the passing of the parameters for the handler, so the dart code doesn't need to save the "user preferences". Am I right?

That's a good question. It's correct that the server cannot modify the settings itself. In the extension (pre-LSP) this would be quite simple, but there's not a good way to do this with LSP. Implementing a custom message would work for VS Code, but not other editors so I'm not sure that's worth doing.

So for now, probably it would be better for the prompt to just ask if you want to rename the file with only options for Yes/No and the user would have to change the setting themselves (this could be mentioned in the prompt).

Also can you take a look at my code and check if what I did it fine?

Sure - it would be better to open a review to make it easier to comment (it'll need reviewing by a Googler there to merge anyway), but I've had a quick look through and made a few comments below.

• There shouldn't be any changes to protocol_generated, instead the setting should be added to the client_configuration.dart file (see enableSdkFormatter for a an example, although that one is just a boolean whereas this one should be nullable)
  This should also be documented in https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/tool/lsp_spec/README.md#client-workspace-configuration so other LSP clients (or their users) can discover it (we'd also want to add it to Dart-Code's package.json so it shows up in the Settings UI)
• Based on my comment above, I think we'd need to remove thisTimeOnly and just use Yes and No for the prompt instead
• I would break up the code added to handler_rename into some private methods as it's grown quite large now (for example a method to prompt the user and collect the reply, and another to wrap it that first checks the filename before calling it)
• I think the call to server.getResolvedUnit(oldPath) may be redundant, as this is already available in unit.result?
• We might not need to call MoveFileRefactoring directly - I would have expected if we included a rename ResourceOperation in the original results (we'd need to check the client supports this using the clientCapabilities) the client should then fire the standard rename events (that will end up calling the willRenameFiles handler). If that doesn't seem to be the case, let me know and we can find out whether that's a bug or not.
• It might be simpler to use regex to convert the class name to a filename, for ex. something like:

  final _upperCasePattern = RegExp('[A-Z]');
  String _toFileName(String className) => className.replaceAllMapped(_upperCasePattern, (match) => match.start == 0 ? match[0]! : '_${match[0]}').toLowerCase();

• Tests will be needed - they'd go in lsp/rename_test.dart. There are some existing tests that use _test_rename_prompt to handle a prompt from the server and then check the results afterwards. There should be tests that check both the behaviour of Yes and No responses when the setting is set to prompt, and also that there is no prompt (and the right result is returned) if the setting is set to true/false. To set the config you can use the provideConfig function (see test_lineLength in test/lsp/format_test.dart for an example of using this).

If anything isn't clear or you have any more questions, let me know!

[DanTup]

@jacobggman is this something you're still looking at (or planning to)? There's no rush to do so, but I'm happy to take a look instead if that suits you better. Thanks!

[jacobggman]

Hi! I have been a little bit busy lately. I will try to finish this issue until next week. Thanks!

[jacobggman]

@DanTup I not finish yet this issue, but I want to open a review. But when I am typing git cl upload -s git saying git: 'cl' is not a git command. It is fine if I just open PR or that I need to download something?

[DanTup]

@jacobggman The "git cl" command comes from depot_tools. The instructions for setting that up are in the Dependencies section here:

https://github.com/dart-lang/sdk/wiki/Building

It includes a binary git-cl - once that's in your PATH, the git cl command will use it.

[jacobggman]

I open a review of this issue, so now hopefully now it's will be easier for you to review my code.

I update the code, but I still not write tests because I did not understand this part completely:

• We might not need to call MoveFileRefactoring directly - I would have expected if we included a rename ResourceOperation in the original results (we'd need to check the client supports this using the clientCapabilities) the client should then fire the standard rename events (that will end up calling the willRenameFiles handler). If that doesn't seem to be the case, let me know and we can find out whether that's a bug or not.

I did not manage to understand what is ResourceOperation. I need to add new rename method? Do I need to call it? I am little bit confused.

[DanTup]

Sorry for the delay!

I did not manage to understand what is ResourceOperation. I need to add new rename method? Do I need to call it?

I think in your original code you were directly calling the MoveFileRefactoring refactor to generate the edits, but I was expecting the client could handle this. So in the WorkspaceEdit sent back, it would instead include a ResourceOperation.rename:

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspaceEdit

I'm not certain if this would trigger VS Code to fire the onWillRename event, so we might need to do both - it would require some testing. If you haven't already set it up, you should be able to test in VS Code using the local analyzer by setting the dart.analyzerPath setting in VS Code, for example:

"dart.analyzerPath": "/Users/danny/Dev/dart-sdk/sdk/pkg/analysis_server/bin/server.dart",

[jacobggman]

Hi. I am a little busy lately and I don't have enough time to finish this issue, if you want, you can take from where I stop. Thanks you for your time!

[DanTup]

This works is complete but requires two things to use it:

1. The next release of Dart-Code (likely early next month)
2. An updated Dart SDK that includes dart-lang/sdk@3aa464a (I believe this will be v2.15, but it may be some way off release)

It's off by default but can be turned on with the dart.renameFilesWithClasses setting (which can be "never", "prompt" or "always").

If you want to try it before them, you can manually add the setting to your VS Code settings (ignore that VS Code says it doesn't exist):

"dart.renameFilesWithClasses": "prompt"

And use a bleeding edge SDK build (not recommended for general dev, stick to stable!):

https://dart.dev/tools/sdk/archive#main-channel-url-scheme

[jacobggman]

Thanks!