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

I don't want to include dynamic objects in find usages and rename #6347

Open
marcglasberg opened this issue Oct 7, 2022 · 5 comments
Open
Milestone

Comments

@marcglasberg
Copy link

marcglasberg commented Oct 7, 2022

In Windows, when I find usages (CTRL-ALT-F7) or rename (SHIFT-F6) it tries to find all instances, but it usually makes mistakes because it's trying to find untyped (dynamic) declarations. I understand some people may have use for dynamic typing, but in my experience almost no one uses it. My team certainly doesn't. When we have a large codebase, not only it takes a long time to find/rename, but it also risks making mistakes.

I could be wrong, but I'd guess having to deal with dynamic variables makes the find/rename process extremely slow, in comparison with Java, for example, which is almost instantaneous. I'd guess it also wastes a lot of memory. In Java I can also just trust rename refactors, but in Dart I only do when the name I am trying to change is very specific.

My request is a new setting:

  • Refactors should include dynamic variables

Turning this off would disable finding and renaming dynamic variables.

@stevemessick
Copy link
Member

@bwilkerson Does dynamic typing cause performance issues for find usages? If so, would the suggested option be a reasonable request?

@stevemessick
Copy link
Member

@bwilkerson friendly ping

@marcglasberg
Copy link
Author

I'd also like to point out that even in case it doesn't create performance problems, it still creates usability problems for us. We never want to rename dynamic variables, but it keeps suggesting some, and it breaks the code if we let it modify. Worse, since it's changing dynamic variables, it's only going to break at runtime.

@stevemessick
Copy link
Member

Sorry @marcglasberg but we don't seem to be getting a response from the analyzer team. I suggest creating an issue in https://github.com/dart-lang/sdk/issues for this and #6346.

@bwilkerson
Copy link

Sorry for the delay. I didn't notice this in my inbox earlier. The team is more likely to see issues that are on the SDK issue tracker, but we ought to see issues here too.

Does dynamic typing cause performance issues for find usages?

I don't know. I don't think we've ever tested for that.

We never want to rename dynamic variables, but it keeps suggesting some, and it breaks the code if we let it modify. Worse, since it's changing dynamic variables, it's only going to break at runtime.

I'm guessing you mean "rename invocations against dynamic variables". If that's not the case ignore the below and let me know.

As I noted in #6346, the problem is that the type dynamic means 'any type', so an instance of the class whose member is being renamed might be in that location.

I don't know which IDE you're using, but IntelliJ has the best support for rename. In IntelliJ we're able to specify dynamic invocation sites separately and if there are any dynamic sites IntelliJ will open a view in which you can choose which, if any, of the dynamic sites are renamed. Even if you never rename dynamic invocation sites you're at least aware of the fact that there might be invocation sites that will be broken by the rename.

I'll point out that the opposite behavior of never renaming dynamic references would break (at runtime) in places where instances of the class have flowed into a dynamic location. I don't know which failure mode is more common, though I would hope that dynamic is rarely ever used anymore, in which case there should be no breakage in either direction.

But your point is well taken; this is a decision that we should probably revisit periodically as the usage of the language evolves.

@jacob314 jacob314 added this to the Backlog milestone Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants