Skip to content

"Extract Final" / "Extract Constant" #31701

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

Open
Tracked by #55825
brianegan opened this issue Dec 20, 2017 · 6 comments
Open
Tracked by #55825

"Extract Final" / "Extract Constant" #31701

brianegan opened this issue Dec 20, 2017 · 6 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-refactoring Issues with analysis server refactorings devexp-server Issues related to some aspect of the analysis server P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@brianegan
Copy link

brianegan commented Dec 20, 2017

Hey hey :) The analysis server has "extract variable" support. It would be super if we had "extract final" as well. In IntelliJ, this is known as "extract constant," but the feature is going to be removed since it no longer works and isn't backed up by the Dart Analysis Server, but is a feature I find invaluable (I use it far more often than "extract variable").

This would also enable the usage in other editors, such as VSCode.

What I envision: when you "extract final", it would work exactly like "extract variable", except instead of using var it would use thefinal keyword.

In code, instead of var extracted = new Thing(); it would be final extracted = new Thing();.

For some context as to where this came from (please see comments section as well): https://youtrack.jetbrains.com/issue/WEB-30141#comment=27-2639924

Thanks!

@bwilkerson bwilkerson added devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Dec 20, 2017
@bwilkerson bwilkerson added the devexp-refactoring Issues with analysis server refactorings label Jan 28, 2018
@Stargator
Copy link
Contributor

Would be helpful if it included the option of extracting with or without the type annotation.

@escamoteur
Copy link

on the same line it would be great to have another variation named "extract field" which would create a class field and assign the value there.

@srawlins
Copy link
Member

@escamoteur I'd file a new issue for the field idea. I don't know if that will be solved in the same way as local final and const variables.

@srawlins
Copy link
Member

@bwilkerson I know you've expressed concern at different times over offering too many refactorings that are detrimental to the UX. What do you think of these two requests?

@bwilkerson
Copy link
Member

First, some terminology so that everyone's on the same page. In terms of the analysis server's protocol, a 'quick assist' is a code change that doesn't require any analysis outside the current library and doesn't require any user feedback. A 'refactoring' is a code change that requires one or both of these. One reason this distinction is interesting is because quick assists are relatively easy to add, only requiring work on the server side; refactorings require work on both the server and in the client, so they tend to be much harder to add.

So adding an "extract variable" refactoring that would have a way of choosing 'field' or 'local', a way of choosing 'var', 'final' or 'const', an option for 'late', etc. would be a lot of work and take a long time to get into user's hands. As a result, we haven't added many (or any that I can think of) refactorings in several years.

But we do need to be careful when implementing quick assists. I suspect that a menu with a dozen variations of "extract variable" would not be a good UX either.

That's not to say that we shouldn't implement any new assists in this area, it's just to say that we need to be intentional about the design so that we don't do more harm than good.

One approach that we've taken in the past is to implement finer-grained assists that can be composed. For example, selecting an expression might provide an "Extract local variable" and "Extract field" pair of assists. We could then offer assists to convert a variable to 'final' and a field to 'final' or 'const', and to convert either to being 'late', and inverses for most of those. The assists only show up when they're valid based on the context, and that allows us to keep the number of assists offered at any given point reasonably small.

It's more work for users to get the code to the state they want, so it isn't always a clear win, but it does sometimes provide useful assists that can be used independently.

@Stargator
Copy link
Contributor

Stargator commented Jun 1, 2021

@bwilkerson Thanks. Regardless which way the architectural goes, I think something is needed in this area as you stated new assists haven't been implemented in years. This was posted in 2017.

And Dart in other areas has been very friendly in terms of UX. The lint rules, local tools (now bundled under the dart command`), the API documentation, and Language Tour.

So I think in terms of UX, this fits well in what the Dart term has provided since day 1. It just hasn't gotten the attention and that can be scary when throughout all the re-designs of the architectural likely didn't factor these in because the Dart team hasn't kept the muscle that does the assists active and in use.

Please don't let another 3.5 years go by. The Dart team has done great in a lot of areas, but I think it's time for the team to invest some time in this area and figuring out how you want to move forward.

These kinds of features are very helpful and clearly valued by the community.

@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-refactoring Issues with analysis server refactorings devexp-server Issues related to some aspect of the analysis server P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants