dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
633 stars 170 forks source link

Potential false positive for omit_local_variable_types. #2097

Open lrhn opened 4 years ago

lrhn commented 4 years ago
E id<E>(E value) => value;
void main() {
  int Function(int) intId = id; 
  var list = [intId];
}

This code gets a "omit_local_variable_types" lint on the intId variable.

Changing the type to var will change semantics and make the type of intId be the generic function type T1 Function<T1>(T1) instead of int Function(int).

Writing var intId = id as int Function(int); instead does not work. It always throw at run-time.

I can write int Function(int) intId; intId = id; instead, but that's a completely unnecessary workaround.

When a context type changes the meaning of the RHS, and there is no available syntax to make the same change explicitly, then the type should be allowed.

Could be related to #1318, which is also about a context type induced change on the RHS. That one is more easily fixed by adding .call. There is no syntax for implicit generic function instantiated tear-off.

natebosch commented 4 years ago

@pq any idea on how difficult this may be? False positives for omit_local_variable_types have high impact since this is enforced in google code.

pq commented 4 years ago

I don't think it should be too hard and have a hacked fix locally but I'm not sure it's the best approach. I'll think on it some more and then put something up for review.

pq commented 4 years ago

Brian and I chatted a bit and it looks like a robust solution would benefit from some new support from analyzer. The good news is that may well address #1318 in the progress.

@bwilkerson: did you want to add some of your specific thoughts?

bwilkerson commented 4 years ago

I believe that we need additional analyzer support to do this correctly. The real question here is whether the explicit type of the variable was used during inference.

@scheglov Is that information already available? If not is this something we could get from analyzer?

If nothing else we could remove the type annotation and re-analyze the file, then compare the results both with and without the annotation, but that sounds prohibitively expensive.