Linter: False positive on only_throw_errors lint

[jonahwilliams]

The following code leads to an only_throw_errors diagnostic, but foo is known to be an Exception at this point. Casting to exception leads to an unnecessary cast lint. This does not repro if the static type of foo is changed to Object or dynamic.

abstract class Foo {}

void bar(Foo foo) {
  if (foo is Exception) {
    throw foo;
  }
}

@pq

[jonahwilliams]

Interestingly I do not see a diagnostic from the following code:

abstract class Foo {}

void bar(Foo foo) {
  if (foo is FooException) {
    throw foo;
  }
}

class FooException extends Foo implements Exception {}

[bwilkerson]

I believe that the reason for the observed behavior is that the foo is Exception doesn't promote foo because there is no relationship between Foo and Exception. The reason is works in the second case is because there _is_ a relationship between Foo and FooException.

I'm not sure that we can "fix" the lint, given that the type system can't tell us that foo is an Exception.

[jonahwilliams]

Then maybe the problem is the cast diagnostic instead? This doesn't repro on my example but in the issue I linked I also got a diagnostic for an unnecessary cast error in flutter_reactive_ble - so that example might be different.

[jonahwilliams]

i.e. might be a bad reduced example

[jonahwilliams]

I got confused with the ble example, its not a separate type its a type parameter:

void bar<T>(T foo) {
  if (foo is Exception) {
    throw foo;
  }
}

void bar<T extends Exception>(T foo) {
  if (foo is Exception) {
    throw foo;
  }
}

Either of these seems like they should work but both have the diagnostic?

[bwilkerson]

Ah, then yes, this is a bug in the lint. When we have a TypeParameterType we need to check the promotedBound as well as the original type.

[srawlins]

Given that dynamic is allowed by the lint, I think an unbound type parameter should also be allowed:

f(dynamic e) => throw e; // OK
g<T>(T e) => throw e; // OK