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

LinkedHashMap<String, T>(); is changed to <String, T>{}; after dart fix #59934

Open
tolotrasamuel opened this issue Jan 17, 2025 · 5 comments
Open
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@tolotrasamuel
Copy link

LinkedHashMap<String, T>(); is changed to <String, T>{}; after dart fix

class CollectionState<K, T> {
  final LinkedHashMap<String, T> data;
  final Map<String, List<String>> subsetState;

  CollectionState({
    LinkedHashMap<String, T>? data,
    this.subsetState = const {},
  }) : data = data ?? LinkedHashMap<String, T>();

}

Map is not the same as LinkedHashMap.

@dart-github-bot
Copy link
Collaborator

Summary: dart fix incorrectly converts LinkedHashMap<String, T>() to <String, T>{}, losing the LinkedHashMap's insertion order. This breaks code relying on LinkedHashMap's specific behavior.

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 17, 2025
@lrhn
Copy link
Member

lrhn commented Jan 17, 2025

It's not typed the same, but {} really is a LinkedHashMap.

The fix shouldn't break your typed code.
On the other hand, your typed code is should use Map<String, T> for the variable type. There is never any good reason to use LinkedHashMap as a type - it has the same API as Map.

@tolotrasamuel
Copy link
Author

tolotrasamuel commented Jan 18, 2025

Okay. I guess I was confused by the documentation. https://api.dart.dev/dart-core/Map-class.html

Maps, and their keys and values, can be iterated. The order of iteration is defined by the individual type of map. Examples:

The plain HashMap is unordered (no order is guaranteed),
the LinkedHashMap iterates in key insertion order,
and a sorted map like SplayTreeMap iterates the keys in sorted order.

The above does not directly suggested that the default implementation of Map is LinkedHashMap.

After reading your responses, that only when I saw down the page:

Map()
Creates an empty LinkedHashMap.
factory

Maybe it will help to put that statement in the documentation text section.

@rakudrama
Copy link
Member

It is definitely a bug if dart fix causes your program to no longer compile.

While the documentation for Map() and LinkedHashMap do mention the map literal syntax {}, it does not explain that the map literal syntax has static type Map but, for non-const map literals, is implemented by LinkedHashMap.

There is never any good reason to use LinkedHashMap as a type - it has the same API as Map.

Nitpick: I disagree with 'never' here. I would say 'usually no reason'.

LinkedHashMap guarantees to maintain the insertion order, something which is cannot be expressed other than by using the type explicitly. This might be important to the algorithms that use data.

I always use the type Map and a map literal {} unless I care about the insertion order, which is almost never.
Over the years we have put a lot of effort into making the default Map be the best Map for most uses.

@rakudrama rakudrama changed the title LinkedHashMap<String, T>(); is changed to <String, T>{}; after dart fix` LinkedHashMap<String, T>(); is changed to <String, T>{}; after dart fix Jan 18, 2025
@tolotrasamuel
Copy link
Author

I agree. dart fix should "fix" the code, not cause a compile time error.

@lrhn lrhn removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Jan 18, 2025
@bwilkerson bwilkerson added P3 A lower priority bug or feature request analyzer-quick-fix labels Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants