[prefer_collection_literals] false positives (local variable assignment/`??` operator/...)

[yanok]

Consider this code:

final m = LinkedHashMap<A,B>();  // LINT

This is currently reported. But I might have completely valid reasons to want a linked hash map here:

• I might need to return a LinkedHashMap later,
• or call another function that takes it,
• or the algorithm of the current function might rely on the insert order being preserved.

But currently I have to either do an explicit cast:

final m = <A,B>{} as LinkedHashMap<A,B>;  // OK

which is ugly (and also theoretically has potential to fail at runtime, since there is no promise that map literals are always linked hash maps, even though in reality they always are), or add an explicit type to the declaration:

final LinkedHashMap<A,B> m = LinkedHashMap<A,B>();   // OK

not only this is ugly and repetitive, it also hits another lint ;) (something about omitting unnecessary type annotations).

So I'd suggest to not lint cases where local variable type is omitted and explicit collection constructor is used.

[yanok]

Found another weird case:

class D {
  LinkedHashMap<String, int>? m;
}
void g(D d, LinkedHashMap<String, int>? hm) {
  d.m = LinkedHashMap<String, int>() ;  // OK
}

but

void g(D d, LinkedHashMap<String, int>? hm) {
  d.m = hm ?? LinkedHashMap<String, int>() ;  // LINT
}

[yanok]

In general, I think this lint should be conservative and only suggest to change to a literal if we 100% sure it will fit (e.g. there is a context type and Map/Set is a subtype of it).

[srawlins]

See the comments here:

https://github.com/dart-lang/sdk/blob/9bcb4a024ba74055e468860881cb44a8c556e2c7/pkg/linter/lib/src/rules/prefer_collection_literals.dart#L149

and here:

https://github.com/dart-lang/sdk/blob/main/pkg/linter/lib/src/rules/prefer_collection_literals.dart#L167

This rule is inherently inaccurate in its design, as there is no mechanism provided by the analyzer to answer "what is the context type of expression 1 when I change the code text at expression 2" or something similar. I agree about being conservative.

[yanok]

Thanks for the pointers!

This rule is inherently inaccurate in its design, as there is no mechanism provided by the analyzer to answer "what is the context type of expression 1 when I change the code text at expression 2" or something similar.

But we control the Analyzer, right? It seems a bit more straight-forward to add support for reporting the context type used during inference, then to try to guess the context type based on the AST nodes, which is never going to be precise. Also, while I see that in some cases we really want to have this "what if?" functionality, that seems not trivial to implement, here probably just exposing the context type via elements would suffice.

[srawlins]

Yeah there are a few lint rules etc that would benefit from "what is the context type of e1 without considering the type of e2?" unnecessary_cast, this one, unnecessary_lambdas, maybe more.

Currently I believe strict-inference takes advantage of "what do things look like during inference, when context types are known," in the same way these rules need, and yes, we could move some lint rules to do their calculations during that stage.

[bwilkerson]

If we have multiple lints that would benefit from knowing what the context type is during inference, then it seems likely that other lint authors might want that information as well. I have to wonder then whether the right solution here might be to capture that information in the AST structure so that it can be used after resolution has completed.

[yanok]

to capture that information in the AST structure so that it can be used after resolution has completed.

Yes, that's exactly what I proposed above! Technically I think that information belongs more to the element structure and not to the AST, but that's just nit picking.

[bwilkerson]

Unless I'm misunderstanding what you're asking for, I don't believe that we could associate the context type with the element model. The context type is associated with a particular location in the code, not with a declared name. For example, consider

List<int> x = [y];
String y = '';
Set<double> z = {y};

There is a context type for the list literal [y] in the initializer for x. There is no element representing the list literal, so there would be no element on which to capture that information.

On the flip side, there is a context type for y in both the list literal and the set literal, but there is a single element for y, so we couldn't reasonably capture both context types if we associated them with elements.

[yanok]

Sorry, I completely forgot about this discussion.

You are right, we'll have to put it into the AST. It's just my understanding of the element model that was wrong, I thought it essentially mirrors the AST, adding the semantic information (while AST is mostly about syntax). So I was thinking that since the context type belongs to the semantics analysis, it would be better placed in the element model. But now I realize we don't have enough elements to put all the context types. Thanks for giving me an example!