Give warning if code tries to do `Type as OtherType`.

[lrhn]

It's a common failure for people to attempt to check something about the type of a type variable by doing if (T is SomeType) (or even occasionally T as SomeType as a way to promote the type variable itself).

This doesn't work, it just converts T to a Type object and then the is/as operation fails (because the type after it is never Type or a top type), and it does so at runtime because the is/as operators accept any object as first operand.

We should warn people who try to do this. It's almost invariably due to a mistaken assumption about what the operation does, and because there is no static warning, the mistake isn't corrected until the author asks on StackOverflow or similar.

I suggest the analyzer should give a warning whenever a Type literal is the first operand of is or as (nothing good can come from such a test) ~~, and in null safe code when any expression of type Type is used as the first operand. (In non-null safe code, a typeExpression is Type could be intended as a null-check.)~~ (Nope, that's not safe because something of type Type can also implement other types, type literals won't).

(We could probably warn on any is check where we can statically determine the result, just as we warn about unnecessary casts. An always-true or always-false is check is also unnecessary).

[bwilkerson]

I suggest the analyzer should give a warning whenever a Type literal is the first operand of is or as ...

I agree. I'm not sure it's worth special casing for the non-null safe case, but that's because I doubt there will be many uses of that pattern. I might be wrong about that.

We could probably warn on any is check where we can statically determine the result ...

We already have some checks for that (HintCode UNNECESSARY_TYPE_CHECK_FALSE and HintCode UNNECESSARY_TYPE_CHECK_TRUE), but the places where it's checked appears to be incomplete. For example, I just tried the following code and didn't get any diagnostic:

void f(int i) {
  if (i is String) {
    print(i);
  }
}

[asashour]

I hope it is fine to say something.

Regarding the is scenario: are the below tests missing something? As I have a local CL to fix them.

UNNECESSARY_TYPE_CHECK_FALSE

void f(int a) {
  a is String;
}

void f<T>(T a) {
  T is String;
}

void f<T>(T a) {
  T is! Object;
}

UNNECESSARY_TYPE_CHECK_TRUE

void f(int a) {
  a is! String;
}

void f<T>(T a) {
  T is! String;
}

void f<T>(T a) {
  T is Object;
}

No Hint

void f(Object a) {
  a is String;
}

void f(Object a) {
  a is! String;
}

[lrhn]

Looks fine to me.

Maybe also: int is T. Definitely if T has a bound which is not a supertype of Type, maybe even then because it's unlikely to be doing what the author thinks (which is to check whether T is a supertype of Type).

[asashour]

This may not seem possible, please see the discussion in https://dart-review.googlesource.com/c/sdk/+/213345/comments/e47f43cc_3dca944e