search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

[analyzer] Assist (and fix) to convert `Object.hash` to `Object.hashAll` #58260

Open parlough opened 3 days ago

parlough commented 3 days ago

The assist could always be offered, while the fix version could be offered when there are 21 or more positional arguments specified causing an error.

bwilkerson commented 2 days ago

How likely is it that a user would want to convert from hash to hashAll when there are few enough arguments that it isn't required?

I ask because adding code to create a list that doesn't need to be created seems like an anti-pattern and hence something that we might not want to suggest.

parlough commented 1 day ago

How likely is it that a user would want to convert from hash to hashAll when there are few enough arguments that it isn't required?

I think not that often, I mostly included that in the suggestion as it comes mostly free with the fix and thought a developer might convert to a list if expecting to hit the limit.

I ask because adding code to create a list that doesn't need to be created seems like an anti-pattern and hence something that we might not want to suggest.

I think that makes sense and I'm fine with not having the assist. But as a question, if there's an assist both ways, does that really imply we're suggesting it?

bwilkerson commented 20 hours ago

But as a question, if there's an assist both ways, does that really imply we're suggesting it?

I would like to be able to say "no", but I have an existence proof indicating that the answer is, at least for some users, "yes" (an issue asking why we recommend a particular change only to then recommend changing it back). Some IDEs present assists and fixes in the same way, making them indistinguishable. If a user thinks of fixes as being suggestions for ways to fix or improve the code, then it's not unreasonable for them to apply the same status to assists, especially given the lack of distinction in the IDE.

This lack of distinction in the UI is one of the reasons that I have been leaning toward, and arguing for, referring to all such features as refactors, and not trying to make a distinction between fixes, assists, and refactorings in our user-facing docs.