add lint voidness_propagation

[a14n]

Fixes dart-lang/sdk#57770

[pq]

Interesting that we're seeing exceptions in benchmarking:

https://travis-ci.org/dart-lang/linter/jobs/439879861

Thoughts on that?

[bwilkerson]

Both here and below, node.returnType can return null when the method declaration has no declared return type.

[bwilkerson]

@MichaelRFairhurst

[pq]

ping: @MichaelRFairhurst 👍

[MichaelRFairhurst]

nit: "lose"

[MichaelRFairhurst]

same here

[MichaelRFairhurst]

I would like to maybe see about giving this lint better support in the analyzer API.

There are many more places that downcasts can occur -- for instance:

class DefinesPlus {
  operator +(DefinesPlus other) => null;
}

void main() {
  var x = DefinesPlus();
  void y;
  print(x + y); // downcast from void to DefinesPlus
}

The CFE project that lowers Dart to kernel code will include an explicit cast here, and the analyzer can do the same sort of resolution. If we could expose that in the tree, that would make it easier for you to, for instance, find any expressions that are downcasted and then check what the from + to types were. This would get a performance boost (by not having to check every node for downcasts again), make this code simpler (hopefully!), and make it easy to have this lint and others like it be consistently conclusive in coverage.

Are you interested in doing that for this lint or do you want me to keep that in my mind as a theoretical? Note: I have not discussed this with the other analyzer teammembers and they may not agree with me on it.

[bwilkerson]

I would like to maybe see about giving this lint better support in the analyzer API.

I'm not sure what kind of support you're thinking about, so it's hard for me to say what I think about it. :-)

I would expect that we already have the "from" and "to" types in the AST. It should just be a matter of testing the relationships between those types. You might be proposing that we encapsulate that test in a requiresImplicitDowncast member so that clients don't have to repeat the test in multiple locations and risk getting it wrong, which seems reasonable.

[MichaelRFairhurst]

Do you want to check parameters too to catch this case?

void foo(int x) => print(x);

void main {
  void Function(void) f = foo; // fails at runtime
}

?

[MichaelRFairhurst]

I thought we already caught this?

class B {                                                                        
  int f() => 123;                                                                
}                                                                                
                                                                                 
class C extends B{                                                               
  void f() => null;                                                              
} 

gives me "invalid override"

[MichaelRFairhurst]

Hmm, this won't catch Iterable<void> being assigned to List<int>, or List<int> being assigned to Iterable<void>.

There are other pathological cases too:

abstract class IV implements Iterable<void> {}
abstract class LI implements List<int> {}

void bad(IV iv, LI li) {
  li = iv;
}

I guess this is probably the most complex part and the API change I described earlier wouldn't help you solve this...once again, though, this is logic we have implemented in the analyzer. I wonder if we can expose that API to you at all in a way where you can provide custom hooks into the subtype comparison. FWIW the subtype compare code is here: https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/src/generated/type_system.dart#L1480

[bwilkerson]

We actually do have a public API for subtype comparison: https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/dart/element/type.dart#L154.

[MichaelRFairhurst]

I would call _check on typeArguments[i], and have _check handle where type.isVoid && !expectedType.isVoid

I think the current code misses this:

void Function() x;
int Function() y = x;

[mit-mit]

Hi @a14n this appears stale; can we close it?