Add a flag to disable implicit downcasts

[nex3]

I want to ensure that there are no implicit downcasts in my code. I never intend to write them, so I expect that all instances of them are likely to be bugs. I understand that it's complex and wide-reaching to make them errors everywhere (as in #26437), but adding a flag that disables them would only affect users who enable this flag. It would probably also make it easier to determine how disruptive it would be to disable them entirely in strong mode.

[jmesserly]

Great idea! Yeah we should do this. For what it's worth, shouldn't be all that hard to create this flag. We already find them, more or less, but we suppress them in most cases (to make upgrading to strong mode a little easier on folks).

The hardest part is probably just adding the option to analyzer_cli and analysis_options.

---

Note, this wouldn't cover covariant generics, e.g. this would still be allowed at compile time:

main() {
  var ints = <int>[1, 2, 3];
  List<Object> objects = ints;
  objects.add('hello'); // runtime failure
}

We could add a flag for invariant generics as well, but that's a good deal bigger task. Also it would be a shame because we have some interfaces like Iterable<T> that are totally safe as covariant interfaces. (If we had variance annotations, we could declare it as abstract class Iterable<out T> which would restrict T to "output" positions only, therefore making it completely safe to cast Iterable<String> to Iterable<Object> for example)

[nex3]

For my part, I'm much less worried about variance bugs than I am about other implicit downcasts.

[jmesserly]

Interesting ... it looks like this option kind of exists internally (not sure if it's exposed) as "strongModeHints", however, that will issue a hint about dynamic invokes as well, which is probably not what we want.

[jmesserly]

A few other interesting findings:

There's something called AssignmentCast for T x = y pattern (note: it's a bit misnamed -- that is a VariableDeclaration not an assignment). Maybe we weren't sure if these downcasts should be considered explicit or implicit.

We have a special case for null. Given that null literal's static type is defined as bottom, I'm not sure how it is possible to downcast it. Maybe that dates back to our experiments with non-null primitive types.

There is also cast from dynamic, e.g. dynamic x; int y = x; ... not sure if we want to allow that. EDIT: for what it's worth C# allows dynamic to be implicitly converted to anything, according to the docs:
https://msdn.microsoft.com/en-us/library/dd264736(v=vs.100).aspx

[nex3]

There is also cast from dynamic, e.g. dynamic x; int y = x; ... not sure if we want to allow that. EDIT: for what it's worth C# allows dynamic to be implicitly converted to anything, according to the docs:
https://msdn.microsoft.com/en-us/library/dd264736(v=vs.100).aspx

Personally I would prefer having a warning there and having to write var y = x as int.

[leafpetersen]

What about:

  --[no-]implicit-cast-on-assignment
  --[no-]implicit-cast-from-dynamic
  --[no-]implicit-cast-call-arguments

all defaulting to true (allowed). Casting from dynamic is always allowed unless that flag is set.

 --[no-]implicit-cast

defaults to true (allowed), and sets all three of the above flags.

I don't love that two of the three cover cast locations, while the third covers a specific source type. Maybe we should just implement the first and third flags, and if we get a feature request for the middle one we could add it. My sense is that if you want to avoid implicit downcasts, then implicit downcasts from dynamic are probably the most important ones to catch (since inference failures will show up as things being dynamic).

[jmesserly]

A nice, thanks for those thoughts! Based on that, I'm implementing one flag that disables all of those implicit casts.

[jmesserly]

https://codereview.chromium.org/2054443002/ starts making some progress. Assuming that approach looks good, there is follow up in analyzer_cli & analysis_options.