Confusing error from built_value generator

[filiph]

Hi there! First of all, thanks for maintaining this package. It's been a big part of my workflow for years now!

I'm afraid I hit a snag. I have a class like this:

abstract class Inventory implements Built<Inventory, InventoryBuilder> {
  static Serializer<Inventory> get serializer => _$inventorySerializer;

  factory Inventory([void updates(InventoryBuilder b)]) = _$Inventory;

  Inventory._();

  BuiltList<Item> get items;

  // ...
}

and a custom builder class like this:

abstract class InventoryBuilder
    implements Builder<Inventory, InventoryBuilder> {
  ListBuilder<Item> items;

  factory InventoryBuilder() = _$InventoryBuilder;

  InventoryBuilder._();

  // ...
}

This worked just fine before null safety. After upgrading to null safety using the migration tool (naive approach?), the classes look like this:

abstract class Inventory implements Built<Inventory, InventoryBuilder> {
  static Serializer<Inventory> get serializer => _$inventorySerializer;

  factory Inventory([void updates(InventoryBuilder b)]) = _$Inventory;

  Inventory._();

  BuiltList<Item> get items;  // <-- no change

  // ...
}

and the builder class:

abstract class InventoryBuilder
    implements Builder<Inventory, InventoryBuilder> {
  ListBuilder<Item>? items;  // <-- added "or null" here

  factory InventoryBuilder() = _$InventoryBuilder;

  InventoryBuilder._();

  // ...
}

Now, the generator does not work, and spits out this error message:

Error in BuiltValueGenerator for abstract class Inventory implements Built<Inventory, InventoryBuilder>.
Please make the following changes to use BuiltValue:

1. Make builder field items have type: BuiltList<Item>? (or, if applicable, builder)

It took me a while to understand that the "if applicable, builder" part means ListBuilder<Item>?. Then I was confused, because that's what I already have there.

So I changed the code in built_value_generator's value_source_field.dart to tell me more:

result.add(GeneratorError((b) => b
          ..message = 'Make builder field $name have type: '
              '$type$orNull (or, if applicable, builder: ${_toBuilderType(element.type, type)}) -- current type: ${builderElementTypeOrNull}'));

And this was the output:

Please make the following changes to use BuiltValue:

1. Make builder field items have type: BuiltList<Item>? (or, if applicable, builder: ListBuilder<Item>) -- current type: ListBuilder<Item>?

So, either the check is incorrect (and nullable BuiltList<Item>? should be permitted), or the error message should be expanded.

What worked for me is this:

abstract class InventoryBuilder
    implements Builder<Inventory, InventoryBuilder> {
  ListBuilder<Item> items = ListBuilder<Item>();

  factory InventoryBuilder() = _$InventoryBuilder;

  InventoryBuilder._();

  // ...
}

Now the source generation works and tests pass.

[davidmorgan]

Hmmmm I think the right thing is that for a nested builder for a non-nullable field, the builder field should be not nullable and initialized, exactly as you ended up with at the end.

So I think fixing the error message is correct.

Will take a look when I get to the next batch of built_value improvements :)

Thanks!