unnecessary_lambdas constructor tear-off false postitives

[MohiuddinM]

class A {
  A(int i);
}

void main() {
  // these are ok (but dart lint encourages tear-off)
  [].map((e) => A(e));
  [].map((e) => A.new(e));
  
  // this is an error: The argument type 'A Function(int)' can't be assigned to the parameter type 'dynamic Function(dynamic)'.
  [].map(A.new);
}

[lrhn]

This is correct behavior, so the issue is with recommending using the tear-offs.

The error is correct. The torn off constructor cannot take any value as argument, so it cannot be used as a function with dynamic as parameter type.

Wrapping in (e) => …(e) seems like it should do nothing, but due to type inference, the type inferred for e is actually dynamic, what is then implicitly downcast where is used.
If you had written it as (dynamic e) => A(e as int), it would be clearer that it isn't just forwarding the argument to the constructor, but it's a different function, with different typing, than the tear-off.

So the analyzer may want to check that the parameter type and argument expression types are actually compatible with the tear-off before recommending it.

[pq]

Good catch!

It looks like unnecessary_lambdas is wrongly linting these cases:

  [].map((e) => A(e));
  [].map((e) => A.new(e));

and that we should fix these false positives in the linter.

Thanks for the report @MohiuddinM!

[eernstg]

we should fix these false positives in the linter.

Interestingly, we have several design forces pushing each other around here.

unnecessary_lambdas might detect that the eta expansion has an effect because (as @lrhn mentioned) the bare A.new and (x) => A.new(x) have different signatures. So it's not unnecessary, and shouldn't be reported.

However, some other lint or option (perhaps avoid_dynamic_calls) should then be able to report that the invocation of A.new(x) in the body of the function literal may incur a run-time type error.

That should help nudging developers in the direction of the more concise and statically typed variant A.new.

Still, [] would be inferred to mean <dynamic>[] because it's a receiver (and hence there's no context type). So A.new will still give rise to a compile-time error. This may not be very helpful, but it will at least give the developer a heads-up that something about the typing needs to be refined.

For the future, if and when we get dart-lang/language#3527, we will also be able to infer [] as <int>[].

[srawlins]

Fixed with dart-lang/sdk@293469e