Migration: properly handle non-nullable built_value getters

This change makes the migration tool understand that the method
`BuiltValueNullFieldError.checkNotNull` expresses non-null intent.
Since the built_value code generator calls this method from generated
built_value constructors, this is sufficient to ensure that built
value getters that lack the `@nullable` annotation will be treated by
the migration tool as non-nullable.

Bug: https://github.com/dart-lang/sdk/issues/43878
Change-Id: I37a9ce584a10d2a1bb27da92f3f664632547f6ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206881
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -1388,7 +1388,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
       }
       _variables!.recordDecoratedExpressionType(node, expressionType);
     }
-    _handleArgumentErrorCheckNotNull(node);
+    _handleCustomCheckNotNull(node);
     _handleQuiverCheckNotNull(node);
     return expressionType;
   }
@@ -2356,29 +2356,6 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         .decoratedTypeParameterBound((type.type as TypeParameterType).element);
   }
 
-  void _handleArgumentErrorCheckNotNull(MethodInvocation node) {
-    var callee = node.methodName.staticElement;
-    var calleeIsStatic = callee is ExecutableElement && callee.isStatic;
-    var target = node.realTarget;
-    bool targetIsArgumentError =
-        (target is SimpleIdentifier && target.name == 'ArgumentError') ||
-            (target is PrefixedIdentifier &&
-                target.identifier.name == 'ArgumentError');
-
-    if (calleeIsStatic &&
-        targetIsArgumentError &&
-        callee!.name == 'checkNotNull' &&
-        node.argumentList.arguments.isNotEmpty) {
-      var argument = node.argumentList.arguments.first;
-      if (argument is SimpleIdentifier && _isReferenceInScope(argument)) {
-        var argumentType =
-            _variables!.decoratedElementType(argument.staticElement!);
-        _graph.makeNonNullable(argumentType.node,
-            ArgumentErrorCheckNotNullOrigin(source, argument));
-      }
-    }
-  }
-
   /// Creates the necessary constraint(s) for an assignment of the given
   /// [expression] to a destination whose type is [destinationType].
   ///
@@ -2551,6 +2528,32 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         redirectedClass.typeParameters);
   }
 
+  void _handleCustomCheckNotNull(MethodInvocation node) {
+    var callee = node.methodName.staticElement;
+    if (node.argumentList.arguments.isNotEmpty &&
+        callee is ExecutableElement &&
+        callee.isStatic) {
+      var enclosingElement = callee.enclosingElement;
+      if (enclosingElement is ClassElement) {
+        if (callee.name == 'checkNotNull' &&
+                enclosingElement.name == 'ArgumentError' &&
+                callee.library.isDartCore ||
+            callee.name == 'checkNotNull' &&
+                enclosingElement.name == 'BuiltValueNullFieldError' &&
+                callee.library.source.uri.toString() ==
+                    'package:built_value/built_value.dart') {
+          var argument = node.argumentList.arguments.first;
+          if (argument is SimpleIdentifier && _isReferenceInScope(argument)) {
+            var argumentType = _variables!.decoratedElementType(
+                _favorFieldFormalElements(getWriteOrReadElement(argument))!);
+            _graph.makeNonNullable(argumentType.node,
+                ArgumentErrorCheckNotNullOrigin(source, argument));
+          }
+        }
+      }
+    }
+  }
+
   void _handleExecutableDeclaration(
       Declaration node,
       ExecutableElement declaredElement,

pkg/nnbd_migration/test/abstract_context.dart

@@ -45,6 +45,17 @@ class AbstractContextTest with ResourceProviderMixin {
 
   String get testsPath => '$homePath/tests';
 
+  void addBuiltValuePackage() {
+    addPackageFile('built_value', 'built_value.dart', '''
+abstract class Built<V extends Built<V, B>, B extends Builder<V, B>> {}
+abstract class Builder<V extends Built<V, B>, B extends Builder<V, B>> {}
+const String nullable = 'nullable';
+class BuiltValueNullFieldError extends Error {
+  static T checkNotNull<T>(T? value, String type, String field) => value!;
+}
+''');
+  }
+
   void addMetaPackage() {
     addPackageFile('meta', 'meta.dart', r'''
 library meta;

pkg/nnbd_migration/test/api_test.dart

@@ -48,7 +48,7 @@ abstract class _ProvisionalApiTestBase extends AbstractContextTest {
   /// Optional parameter [removeViaComments] indicates whether dead code should
   /// be removed in its entirety (the default) or removed by commenting it out.
   Future<void> _checkMultipleFileChanges(
-      Map<String, String> input, Map<String, String> expectedOutput,
+      Map<String, String> input, Map<String, dynamic> expectedOutput,
       {Map<String, String> migratedInput = const {},
       bool removeViaComments = false,
       bool warnOnWeakCode = false,
@@ -495,6 +495,59 @@ main() {
     await _checkSingleFileChanges(content, expected);
   }
 
+  Future<void> test_built_value_non_nullable_getter() async {
+    addBuiltValuePackage();
+    var root = '$projectPath/lib';
+    var path1 = convertPath('$root/lib.dart');
+    var file1 = r'''
+import 'package:built_value/built_value.dart';
+
+part 'lib.g.dart';
+
+abstract class Foo implements Built<Foo, FooBuilder> {
+  int get value;
+  Foo._();
+  factory Foo([void Function(FooBuilder) updates]) = _$Foo;
+}
+''';
+    var expected1 = r'''
+import 'package:built_value/built_value.dart';
+
+part 'lib.g.dart';
+
+abstract class Foo implements Built<Foo, FooBuilder> {
+  int get value;
+  Foo._();
+  factory Foo([void Function(FooBuilder) updates]) = _$Foo;
+}
+''';
+    // Note: in a real-world scenario the generated file would be in a different
+    // directory but we don't need to simulate that detail for this test.  Also,
+    // the generated file would have a lot more code in it, but we don't need to
+    // simulate all the details of what is generated.
+    var path2 = convertPath('$root/lib.g.dart');
+    var file2 = r'''
+part of 'lib.dart';
+
+class _$Foo extends Foo {
+  @override
+  final int value;
+
+  factory _$Foo([void Function(FooBuilder) updates]) => throw '';
+
+  _$Foo._({this.value}) : super._() {
+    BuiltValueNullFieldError.checkNotNull(value, 'Foo', 'value');
+  }
+}
+
+class FooBuilder implements Builder<Foo, FooBuilder> {
+  int get value => throw '';
+}
+''';
+    await _checkMultipleFileChanges(
+        {path1: file1, path2: file2}, {path1: expected1, path2: anything});
+  }
+
   Future<void> test_call_already_migrated_extension() async {
     var content = '''
 import 'already_migrated.dart';