Task - triple check every case of enumerating DartType subtypes for RecordType

[srawlins]

A casual search for is InterfaceType and is! InterfaceType yields a lot of results:

$ git grep 'is\(!\)* InterfaceType' | wc -l
      11

We should review these to add, if appropriate, handling of RecordType.

[yanok]

Nice bug ;) I ran the same command on the fresh sources and now it's down to 10.

$ git grep 'is\(!\)* analyzer.InterfaceType' | wc -l
10

Here is the list:

lib/src/builder.dart:312:    if (type is analyzer.InterfaceType) {
lib/src/builder.dart:328:            if (parameterType is analyzer.InterfaceType) {
lib/src/builder.dart:738:    if (typeToMock is analyzer.InterfaceType) {
lib/src/builder.dart:915:    if (returnType is analyzer.InterfaceType) {
lib/src/builder.dart:937:      if (parameterType is analyzer.InterfaceType) {
lib/src/builder.dart:982:      if (typeParameter is analyzer.InterfaceType) {
lib/src/builder.dart:1008:      if (typeArgument is analyzer.InterfaceType) {
lib/src/builder.dart:1480:    if (type is! analyzer.InterfaceType) {
lib/src/builder.dart:2107:    if (type is analyzer.InterfaceType) {
lib/src/builder.dart:2308:    } else if (self is analyzer.InterfaceType) {

I will comment on the each one.

1.  lib/src/builder.dart:312:    if (type is analyzer.InterfaceType) {
   

   TypeVisitor._addType, records are not covered, likely could result in bugs not importing the types referenced only
   inside records.

2.  lib/src/builder.dart:328:            if (parameterType is analyzer.InterfaceType) {
   

   Also TypeVisitor._addType in very specific clause handling arguments of overriden toString method. Records are not
   covered, if overriden toString has record-typed arguments that reference some types not referenced elsewhere, this will trigger a bug.

3.  lib/src/builder.dart:738:    if (typeToMock is analyzer.InterfaceType) {
   

   _MockTargetGatherer._determineDartType, records are not covered, but here it's fine, we can't mock records, so the code will correctly complain.

4.  lib/src/builder.dart:915:    if (returnType is analyzer.InterfaceType) {
    lib/src/builder.dart:937:      if (parameterType is analyzer.InterfaceType) {
    lib/src/builder.dart:982:      if (typeParameter is analyzer.InterfaceType) {
    lib/src/builder.dart:1008:      if (typeArgument is analyzer.InterfaceType) {
   

   _MockTargetGatherer._checkFunction, _MockTargetGatherer._checkTypeParameters and _MockTargetGatherer._checkTypeArguments. Records are not covered but they should. This will result in the code generation producing code that won't compile sometimes, instead of complaining. I really want to let users provide dummy values for private types too, so we could probably just get rid of most of these checks, instead of fixing them. So I would de-prioritize fixing these.

5.  lib/src/builder.dart:1480:    if (type is! analyzer.InterfaceType) {
   

   _MockClassInfo._dummyValue, records are covered.

6.  lib/src/builder.dart:2107:    if (type is analyzer.InterfaceType) {
   

   _MockClassInfo._typeReference, records are covered.

7.  lib/src/builder.dart:2308:    } else if (self is analyzer.InterfaceType) {
   

   DartType.containsPrivateName, records are covered.

[yanok]

To summarize, I think there are two issues left:

1. TypeVisitor not covering all records usages. This one is kinda important.
2. Various checker methods not checking records. I hope this could mostly go away.