[cfe,flutter] Make widget transformer location parameters optional

johnniwinther · commit-bot@chromium.org · commit ab958bee577b · 2020-07-16T06:12:13.000Z
The location parameters added to Widgets was created as required and non-nullable when opting flutter in to null-safety. This leads to incremental compilation failing because already transformed constructors require parameters that cannot be passed by user code. Fixes flutter/flutter#60269 Change-Id: I91cbeaa455e67e82983ebb17f93da13b46376da2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153085 Reviewed-by: Jens Johansen <jensj@google.com> Reviewed-by: Jacob Richman <jacobr@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml b/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml
@@ -0,0 +1,83 @@
+# Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE.md file.
+
+type: newworld
+target: DDC
+trackWidgetCreation: true
+worlds:
+  - entry: main.dart
+    experiments: non-nullable
+    sources:
+      main.dart: |
+        import 'foo.dart';
+      foo.dart:
+        import 'package:flutter/src/widgets/framework.dart';
+        import 'package:flutter/src/widgets/widget_inspector.dart';
+
+        class Foo extends Widget {
+          const Foo();
+        }
+      flutter/lib/src/widgets/framework.dart: |
+        abstract class Bar {
+          const Bar();
+        }
+
+        abstract class Widget extends Bar {
+          const Widget();
+        }
+      flutter/lib/src/widgets/widget_inspector.dart: |
+        abstract class _HasCreationLocation {
+          _Location get _location;
+        }
+
+        /// A tuple with file, line, and column number, for displaying human-readable
+        /// file locations.
+        class _Location {
+          const _Location({
+            required this.file,
+            required this.line,
+            required this.column,
+            required this.name,
+            required this.parameterLocations,
+          });
+
+          /// File path of the location.
+          final String file;
+
+          /// 1-based line number.
+          final int line;
+
+          /// 1-based column number.
+          final int column;
+
+          /// Optional name of the parameter or function at this location.
+          final String name;
+
+          /// Optional locations of the parameters of the member at this location.
+          final List<_Location> parameterLocations;
+        }
+      .dart_tool/package_config.json: |
+        {
+          "configVersion": 2,
+          "packages": [
+            {
+              "name": "flutter",
+              "rootUri": "../flutter",
+              "packageUri": "lib/",
+              "languageVersion": "2.9"
+            }
+          ]
+        }
+    expectedLibraryCount: 4
+  - entry: main.dart
+    worldType: updated
+    invalidate:
+      - main.dart
+    expectInitializeFromDill: false
+    sources:
+      main.dart: |
+        import 'foo.dart';
+
+        Foo foo = const Foo();
+    expectedLibraryCount: 4
diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml.world.1.expect b/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml.world.1.expect
@@ -0,0 +1,53 @@
+main = <No Member>;
+library from "package:flutter/src/widgets/framework.dart" as fra {
+
+  abstract class Bar extends dart.core::Object /*hasConstConstructor*/  {
+    const constructor •() → fra::Bar
+      : super dart.core::Object::•()
+      ;
+  }
+  abstract class Widget extends fra::Bar implements wid::_HasCreationLocation /*hasConstConstructor*/  {
+    final field wid::_Location? _location /*isNullableByDefault, from null */;
+    const constructor •({wid::_Location? $creationLocationd_0dea112b090073317d4}) → fra::Widget
+      : super fra::Bar::•(), fra::Widget::_location = $creationLocationd_0dea112b090073317d4!
+      ;
+  }
+}
+library from "package:flutter/src/widgets/widget_inspector.dart" as wid {
+
+  abstract class _HasCreationLocation extends dart.core::Object {
+    synthetic constructor •() → wid::_HasCreationLocation
+      : super dart.core::Object::•()
+      ;
+    abstract get _location() → wid::_Location;
+  }
+  class _Location extends dart.core::Object /*hasConstConstructor*/  {
+    final field dart.core::String file;
+    final field dart.core::int line;
+    final field dart.core::int column;
+    final field dart.core::String name;
+    final field dart.core::List<wid::_Location> parameterLocations;
+    const constructor •({required dart.core::String file = #C1, required dart.core::int line = #C1, required dart.core::int column = #C1, required dart.core::String name = #C1, required dart.core::List<wid::_Location> parameterLocations = #C1}) → wid::_Location
+      : wid::_Location::file = file, wid::_Location::line = line, wid::_Location::column = column, wid::_Location::name = name, wid::_Location::parameterLocations = parameterLocations, super dart.core::Object::•()
+      ;
+  }
+}
+library from "org-dartlang-test:///foo.dart" as foo {
+
+  import "package:flutter/src/widgets/framework.dart";
+  import "package:flutter/src/widgets/widget_inspector.dart";
+
+  class Foo extends fra::Widget /*hasConstConstructor*/  {
+    const constructor •({wid::_Location? $creationLocationd_0dea112b090073317d4}) → foo::Foo
+      : super fra::Widget::•($creationLocationd_0dea112b090073317d4: $creationLocationd_0dea112b090073317d4)
+      ;
+  }
+}
+library from "org-dartlang-test:///main.dart" as main {
+
+  import "org-dartlang-test:///foo.dart";
+
+}
+constants  {
+  #C1 = null
+}
diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml.world.2.expect b/pkg/front_end/testcases/incremental_initialize_from_dill/flutter_widget_transform_nnbd.yaml.world.2.expect
@@ -0,0 +1,54 @@
+main = <No Member>;
+library from "package:flutter/src/widgets/framework.dart" as fra {
+
+  abstract class Bar extends dart.core::Object /*hasConstConstructor*/  {
+    const constructor •() → fra::Bar
+      : super dart.core::Object::•()
+      ;
+  }
+  abstract class Widget extends fra::Bar implements wid::_HasCreationLocation /*hasConstConstructor*/  {
+    final field wid::_Location? _location /*isNullableByDefault, from null */;
+    const constructor •({wid::_Location? $creationLocationd_0dea112b090073317d4}) → fra::Widget
+      : super fra::Bar::•(), fra::Widget::_location = $creationLocationd_0dea112b090073317d4!
+      ;
+  }
+}
+library from "package:flutter/src/widgets/widget_inspector.dart" as wid {
+
+  abstract class _HasCreationLocation extends dart.core::Object {
+    synthetic constructor •() → wid::_HasCreationLocation
+      : super dart.core::Object::•()
+      ;
+    abstract get _location() → wid::_Location;
+  }
+  class _Location extends dart.core::Object /*hasConstConstructor*/  {
+    final field dart.core::String file;
+    final field dart.core::int line;
+    final field dart.core::int column;
+    final field dart.core::String name;
+    final field dart.core::List<wid::_Location> parameterLocations;
+    const constructor •({required dart.core::String file = #C1, required dart.core::int line = #C1, required dart.core::int column = #C1, required dart.core::String name = #C1, required dart.core::List<wid::_Location> parameterLocations = #C1}) → wid::_Location
+      : wid::_Location::file = file, wid::_Location::line = line, wid::_Location::column = column, wid::_Location::name = name, wid::_Location::parameterLocations = parameterLocations, super dart.core::Object::•()
+      ;
+  }
+}
+library from "org-dartlang-test:///foo.dart" as foo {
+
+  import "package:flutter/src/widgets/framework.dart";
+  import "package:flutter/src/widgets/widget_inspector.dart";
+
+  class Foo extends fra::Widget /*hasConstConstructor*/  {
+    const constructor •({wid::_Location? $creationLocationd_0dea112b090073317d4}) → foo::Foo
+      : super fra::Widget::•($creationLocationd_0dea112b090073317d4: $creationLocationd_0dea112b090073317d4)
+      ;
+  }
+}
+library from "org-dartlang-test:///main.dart" as main {
+
+  import "org-dartlang-test:///foo.dart";
+
+  static field foo::Foo foo = invalid-expression "Constant evaluation has no support for NullCheck!";
+}
+constants  {
+  #C1 = null
+}
diff --git a/pkg/kernel/lib/transformations/track_widget_constructor_locations.dart b/pkg/kernel/lib/transformations/track_widget_constructor_locations.dart
@@ -368,8 +368,8 @@ class WidgetCreatorTracker {
       _hasCreationLocationClass.enclosingLibrary,
     );
     final Field locationField = new Field(fieldName,
-        type: new InterfaceType(
-            _locationClass, clazz.enclosingLibrary.nonNullable),
+        type:
+            new InterfaceType(_locationClass, clazz.enclosingLibrary.nullable),
         isFinal: true,
         reference: clazz.reference.canonicalName
             ?.getChildFromFieldWithName(fieldName)
@@ -389,8 +389,8 @@ class WidgetCreatorTracker {
       ));
       final VariableDeclaration variable = new VariableDeclaration(
         _creationLocationParameterName,
-        type: new InterfaceType(
-            _locationClass, clazz.enclosingLibrary.nonNullable),
+        type:
+            new InterfaceType(_locationClass, clazz.enclosingLibrary.nullable),
       );
       if (!_maybeAddNamedParameter(constructor.function, variable)) {
         return;
@@ -418,7 +418,11 @@ class WidgetCreatorTracker {
       if (!hasRedirectingInitializer) {
         constructor.initializers.add(new FieldInitializer(
           locationField,
-          new VariableGet(variable),
+          clazz.enclosingLibrary.isNonNullableByDefault
+              // The parameter is nullable so that it can be optional but the
+              // field is non-nullable so we check it here.
+              ? new NullCheck(new VariableGet(variable))
+              : new VariableGet(variable),
         ));
         // TODO(jacobr): add an assert verifying the locationField is not
         // null. Currently, we cannot safely add this assert because we do not
@@ -535,8 +539,7 @@ class WidgetCreatorTracker {
           procedure.function,
           new VariableDeclaration(_creationLocationParameterName,
               type: new InterfaceType(
-                  _locationClass, clazz.enclosingLibrary.nonNullable),
-              isRequired: clazz.enclosingLibrary.isNonNullableByDefault),
+                  _locationClass, clazz.enclosingLibrary.nullable)),
         );
       }
     }
@@ -559,8 +562,7 @@ class WidgetCreatorTracker {
       final VariableDeclaration variable = new VariableDeclaration(
           _creationLocationParameterName,
           type: new InterfaceType(
-              _locationClass, clazz.enclosingLibrary.nonNullable),
-          isRequired: clazz.enclosingLibrary.isNonNullableByDefault);
+              _locationClass, clazz.enclosingLibrary.nullable));
       if (_hasNamedParameter(
           constructor.function, _creationLocationParameterName)) {
         // Constructor was already rewritten. TODO(jacobr): is this case actually hit?
