NNBD support

[davidmorgan]

I've been looking at the built_value generator.

There are a whole pile of fun issues here, including hacks needed to get build runner to run at all when some things are opted in to NNBD:

pub run --enable-experiment=non-nullable build_runner build
sed -i -e 's#// ignore_for_file: directives_ordering#// ignore_for_file: directives_ordering\n//@dart=2.8#'  .dart_tool/build/entrypoint/build.dart
dart --enable-experiment=non-nullable --no-null-safety .dart_tool/build/entrypoint/build.dart build

One thing that does not seem to work currently, is that the LibraryElement.isNonNullableByDefault property is always false. I'm kind of puzzled by this--even if the target is clearly NNBD enabled, i.e. is using NNBD syntax, it seems to return false.

result.writeln('// NNBD? ${library.element.isNonNullableByDefault}'); // outputs `false`

For now I can work around this by checking the source for an explicit version comment, but this misses other types of configuration.

I think this works in google3, so I am guessing it is a matter of the wiring around the generators, and so probably package:build :)

[jakemac53]

You need to do pub run build_runner build --enable-experiment=non-nullable (note the flag moves to a build_runner argument).

Otherwise you are enabling NNBD for the build script, not the build itself :). It is confusing for sure though :D.

[jakemac53]

Note that we do not support toggling null safety mode on/off yet, and we also don't support dart2js directly yet (you can hack it with the dart2js args options, but I wouldn't bother).

[davidmorgan]

Thanks.

This seems to rely on allowed_experiments.json, though; any ideas on how to make it work for built_value, please?

> ~/bin/dart-sdk/bin/pub run build_runner build --enable-experiment=non-nullable
Unable to spawn isolate: ../built_value/lib/serializer.dart:136:13: Error: This requires the 'non-nullable' language feature to be enabled.
Try updating your pubspec.yaml to set the minimum SDK constraint to 2.9 or higher, and running 'pub get'.
  Serializer? serializerForType(Type type);

Well, I know how to make it work locally, I can build an SDK with built_value in allowed_experiments.json :) which should unblock me for now; but it won't work for travis. Any ideas appreciated :)

Thanks.

[jakemac53]

Oh, I didn't realize you were also using null safety in your code generator code. To do that you would need to pass --enable-experiment like you were before, as well as where I told you to put it, so pub run --enable-experiment=non-nullable build_runner build --enable-experiment=non-nullable.

However this won't work with the latest build_runner because it has an sdk constraint which opts it in to null safety even though it doesn't want to be....

cc @jonasfj @leafpetersen @natebosch an example of why we need some alternate mechanism for setting the language version.

I will not be adding opt out comments to the entire build_runner package, nor will I be approving such a cl, because that is unreasonable IMO.

[davidmorgan]

The code generator built_value_generator is not migrated to null safety--but it depends on the runtime library built_value, which does. So I guess that's the problem.

Anyway, I'm not blocked--I was able to work with a custom built SDK allowlist :)--I mention since I think you said this would matter for external support at some point.

Thanks.

[davidmorgan]

(I think, makes sense to reopen? Feel free to reclose if not...)

[jonasfj]

However this won't work with the latest build_runner because it has an sdk constraint which opts it in to null safety even though it doesn't want to be....

I'm curious, why does it have such an SDK constraint?

[natebosch]

I'm curious, why does it have such an SDK constraint?

It's related to the problems we had with the analyzer package not pinning to a specific SDK which caused problems for codegen users when their SDK and version of analyzer didn't agree won language versioning details.

dart-lang/sdk#42887 (comment)

[jakemac53]

I'm curious, why does it have such an SDK constraint?

Note that this sdk constraint only causes a problem with the null safety experiment flag enabled. On its own today the constraint is totally valid.

Any package with a min sdk constraint of >=2.9 today for instance would have this same problem on the 2.9 stable sdk.

[jakemac53]

I am going to go ahead and close this as I don't believe there is anything actionable here that isn't covered by other issues