diff --git a/ci/analyze.sh b/ci/analyze.sh index 9ee12a9138b1d..33d930238209a 100755 --- a/ci/analyze.sh +++ b/ci/analyze.sh @@ -32,6 +32,20 @@ if [ -n "$RESULTS" ]; then exit 1; fi +echo "Analyzing flutter_kernel_transformers..." +RESULTS=`dartanalyzer \ + --packages=flutter/flutter_kernel_transformers/.packages \ + --options flutter/analysis_options.yaml \ + flutter/flutter_kernel_transformers \ + 2>&1 \ + | grep -Ev "No issues found!" \ + | grep -Ev "Analyzing.+flutter_kernel_transformers"` +echo "$RESULTS" +if [ -n "$RESULTS" ]; then + echo "Failed." + exit 1; +fi + echo "Analyzing tools/licenses..." (cd flutter/tools/licenses && pub get) RESULTS=`dartanalyzer \ diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index ee13f32a19bca..b20158634eea5 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -88,6 +88,7 @@ FILE: ../../../flutter/flow/texture.cc FILE: ../../../flutter/flow/texture.h FILE: ../../../flutter/flow/view_holder.cc FILE: ../../../flutter/flow/view_holder.h +FILE: ../../../flutter/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart FILE: ../../../flutter/fml/base32.cc FILE: ../../../flutter/fml/base32.h FILE: ../../../flutter/fml/base32_unittest.cc diff --git a/flutter_kernel_transformers/BUILD.gn b/flutter_kernel_transformers/BUILD.gn new file mode 100644 index 0000000000000..bde059f34990f --- /dev/null +++ b/flutter_kernel_transformers/BUILD.gn @@ -0,0 +1,23 @@ +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +assert(is_fuchsia || is_fuchsia_host) + +import("//build/dart/dart_library.gni") + +dart_library("flutter_kernel_transformers") { + disable_analysis = true + package_name = "flutter_kernel_transformers" + + sources = [ + "track_widget_constructor_locations.dart", + ] + + deps = [ + "//third_party/dart-pkg/pub/meta", + "//third_party/dart/pkg/front_end", + "//third_party/dart/pkg/kernel", + "//third_party/dart/pkg/vm", + ] +} diff --git a/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart b/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart new file mode 100644 index 0000000000000..a4abc5b3eb534 --- /dev/null +++ b/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart @@ -0,0 +1,595 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +library track_widget_constructor_locations; + +// The kernel/src import below that requires lint `ignore_for_file` +// is a temporary state of things until kernel team builds better api that would +// replace api used below. This api was made private in an effort to discourage +// further use. +// ignore_for_file: implementation_imports +import 'package:kernel/ast.dart'; +import 'package:kernel/class_hierarchy.dart'; +import 'package:meta/meta.dart'; +import 'package:vm/frontend_server.dart' show ProgramTransformer; + +// Parameter name used to track were widget constructor calls were made from. +// +// The parameter name contains a randomly generate hex string to avoid collision +// with user generated parameters. +const String _creationLocationParameterName = + r'$creationLocationd_0dea112b090073317d4'; + +/// Name of private field added to the Widget class and any other classes that +/// implement Widget. +/// +/// Regardless of what library a class implementing Widget is defined in, the +/// private field will always be defined in the context of the widget_inspector +/// library ensuring no name conflicts with regular fields. +const String _locationFieldName = r'_location'; + +bool _hasNamedParameter(FunctionNode function, String name) { + return function.namedParameters + .any((VariableDeclaration parameter) => parameter.name == name); +} + +bool _hasNamedArgument(Arguments arguments, String argumentName) { + return arguments.named + .any((NamedExpression argument) => argument.name == argumentName); +} + +VariableDeclaration _getNamedParameter( + FunctionNode function, + String parameterName, +) { + for (VariableDeclaration parameter in function.namedParameters) { + if (parameter.name == parameterName) { + return parameter; + } + } + return null; +} + +// TODO(jacobr): find a solution that supports optional positional parameters. +/// Add the creation location to the arguments list if possible. +/// +/// Returns whether the creation location argument could be added. We cannot +/// currently add the named argument for functions with optional positional +/// parameters as the current scheme requires adding the creation location as a +/// named parameter. Fortunately that is not a significant issue in practice as +/// no Widget classes in package:flutter have optional positional parameters. +/// This code degrades gracefully for constructors with optional positional +/// parameters by skipping adding the creation location argument rather than +/// failing. +void _maybeAddCreationLocationArgument( + Arguments arguments, + FunctionNode function, + Expression creationLocation, + Class locationClass, +) { + if (_hasNamedArgument(arguments, _creationLocationParameterName)) { + return; + } + if (!_hasNamedParameter(function, _creationLocationParameterName)) { + return; + } + + final NamedExpression namedArgument = + NamedExpression(_creationLocationParameterName, creationLocation); + namedArgument.parent = arguments; + arguments.named.add(namedArgument); +} + +/// Adds a named parameter to a function if the function does not already have +/// a named parameter with the name or optional positional parameters. +bool _maybeAddNamedParameter( + FunctionNode function, + VariableDeclaration variable, +) { + if (_hasNamedParameter(function, _creationLocationParameterName)) { + // Gracefully handle if this method is called on a function that has already + // been transformed. + return false; + } + // Function has optional positional parameters so cannot have optional named + // parameters. + if (function.requiredParameterCount != function.positionalParameters.length) { + return false; + } + variable.parent = function; + function.namedParameters.add(variable); + return true; +} + +/// Transformer that modifies all calls to Widget constructors to include +/// a [DebugLocation] parameter specifying the location where the constructor +/// call was made. +/// +/// This transformer requires that all Widget constructors have already been +/// transformed to have a named parameter with the name specified by +/// `_locationParameterName`. +class _WidgetCallSiteTransformer extends Transformer { + final ClassHierarchy _hierarchy; + + /// The [Widget] class defined in the `package:flutter` library. + /// + /// Used to perform instanceof checks to determine whether Dart constructor + /// calls are creating [Widget] objects. + Class _widgetClass; + + /// The [DebugLocation] class defined in the `package:flutter` library. + Class _locationClass; + + /// Current factory constructor that node being transformed is inside. + /// + /// Used to flow the location passed in as an argument to the factory to the + /// actual constructor call within the factory. + Procedure _currentFactory; + + _WidgetCallSiteTransformer( + this._hierarchy, { + @required Class widgetClass, + @required Class locationClass, + }) + : _widgetClass = widgetClass, + _locationClass = locationClass; + + /// Builds a call to the const constructor of the [DebugLocation] + /// object specifying the location where a constructor call was made and + /// optionally the locations for all parameters passed in. + /// + /// Specifying the parameters passed in is an experimental feature. With + /// access to the source code of an application you could determine the + /// locations of the parameters passed in from the source location of the + /// constructor call but it is convenient to bundle the location and names + /// of the parameters passed in so that tools can show parameter locations + /// without re-parsing the source code. + ConstructorInvocation _constructLocation( + Location location, { + String name, + ListLiteral parameterLocations, + bool showFile = true, + }) { + final List arguments = [ + NamedExpression('line', IntLiteral(location.line)), + NamedExpression('column', IntLiteral(location.column)), + ]; + if (showFile) { + arguments.add(NamedExpression( + 'file', StringLiteral(location.file.toString()))); + } + if (name != null) { + arguments.add(NamedExpression('name', StringLiteral(name))); + } + if (parameterLocations != null) { + arguments + .add(NamedExpression('parameterLocations', parameterLocations)); + } + return ConstructorInvocation( + _locationClass.constructors.first, + Arguments([], named: arguments), + isConst: true, + ); + } + + @override + Procedure visitProcedure(Procedure node) { + if (node.isFactory) { + _currentFactory = node; + node.transformChildren(this); + _currentFactory = null; + return node; + } + return defaultTreeNode(node); + } + + bool _isSubclassOfWidget(Class clazz) { + // TODO(jacobr): use hierarchy.isSubclassOf once we are using the + // non-deprecated ClassHierarchy constructor. + return _hierarchy.isSubclassOf(clazz, _widgetClass); + } + + @override + StaticInvocation visitStaticInvocation(StaticInvocation node) { + node.transformChildren(this); + final Procedure target = node.target; + if (!target.isFactory) { + return node; + } + final Class constructedClass = target.enclosingClass; + if (!_isSubclassOfWidget(constructedClass)) { + return node; + } + + _addLocationArgument(node, target.function, constructedClass); + return node; + } + + void _addLocationArgument(InvocationExpression node, FunctionNode function, + Class constructedClass) { + _maybeAddCreationLocationArgument( + node.arguments, + function, + _computeLocation(node, function, constructedClass), + _locationClass, + ); + } + + @override + ConstructorInvocation visitConstructorInvocation(ConstructorInvocation node) { + node.transformChildren(this); + + final Constructor constructor = node.target; + final Class constructedClass = constructor.enclosingClass; + if (!_isSubclassOfWidget(constructedClass)) { + return node; + } + + _addLocationArgument(node, constructor.function, constructedClass); + return node; + } + + Expression _computeLocation(InvocationExpression node, FunctionNode function, + Class constructedClass) { + // For factory constructors we need to use the location specified as an + // argument to the factory constructor rather than the location + // TODO(jacobr): use hierarchy.isSubclassOf once we are using the + // non-deprecated ClassHierarchy constructor. + if (_currentFactory != null && + _hierarchy.isSubclassOf(constructedClass, _currentFactory.enclosingClass)) { + final VariableDeclaration creationLocationParameter = _getNamedParameter( + _currentFactory.function, + _creationLocationParameterName, + ); + if (creationLocationParameter != null) { + return VariableGet(creationLocationParameter); + } + } + + final Arguments arguments = node.arguments; + final Location location = node.location; + final List parameterLocations = + []; + final List parameters = function.positionalParameters; + for (int i = 0; i < arguments.positional.length; ++i) { + final Expression expression = arguments.positional[i]; + final VariableDeclaration parameter = parameters[i]; + parameterLocations.add(_constructLocation( + expression.location, + name: parameter.name, + showFile: false, + )); + } + for (NamedExpression expression in arguments.named) { + parameterLocations.add(_constructLocation( + expression.location, + name: expression.name, + showFile: false, + )); + } + return _constructLocation( + location, + parameterLocations: ListLiteral( + parameterLocations, + typeArgument: _locationClass.thisType, + isConst: true, + ), + ); + } +} + + +/// Rewrites all widget constructors and constructor invocations to add a +/// parameter specifying the location the constructor was called from. +/// +/// The creation location is stored as a private field named `_location` +/// on the base widget class and flowed through the constructors using a named +/// parameter. +class WidgetCreatorTracker implements ProgramTransformer { + Class _widgetClass; + Class _locationClass; + + /// Marker interface indicating that a private _location field is + /// available. + Class _hasCreationLocationClass; + + /// The [ClassHierarchy] that should be used after applying this transformer. + /// If any class was updated, in general we need to create a new + /// [ClassHierarchy] instance, with new dispatch targets; or at least let + /// the existing instance know that some of its dispatch tables are not + /// valid anymore. + ClassHierarchy hierarchy; + + void _resolveFlutterClasses(Iterable libraries) { + // If the Widget or Debug location classes have been updated we need to get + // the latest version + for (Library library in libraries) { + final Uri importUri = library.importUri; + if (!library.isExternal && + importUri != null && + importUri.scheme == 'package') { + if (importUri.path == 'flutter/src/widgets/framework.dart') { + for (Class class_ in library.classes) { + if (class_.name == 'Widget') { + _widgetClass = class_; + } + } + } else { + if (importUri.path == 'flutter/src/widgets/widget_inspector.dart') { + for (Class class_ in library.classes) { + if (class_.name == '_HasCreationLocation') { + _hasCreationLocationClass = class_; + } else if (class_.name == '_Location') { + _locationClass = class_; + } + } + } + } + } + } + } + + /// Modify [clazz] to add a field named [_locationFieldName] that is the + /// first parameter of all constructors of the class. + /// + /// This method should only be called for classes that implement but do not + /// extend [Widget]. + void _transformClassImplementingWidget(Class clazz) { + if (clazz.fields + .any((Field field) => field.name.name == _locationFieldName)) { + // This class has already been transformed. Skip + return; + } + clazz.implementedTypes + .add(Supertype(_hasCreationLocationClass, [])); + // We intentionally use the library context of the _HasCreationLocation + // class for the private field even if [clazz] is in a different library + // so that all classes implementing Widget behave consistently. + final Field locationField = Field( + Name( + _locationFieldName, + _hasCreationLocationClass.enclosingLibrary, + ), + isFinal: true, + ); + clazz.addMember(locationField); + + final Set _handledConstructors = + Set.identity(); + + void handleConstructor(Constructor constructor) { + if (!_handledConstructors.add(constructor)) { + return; + } + assert(!_hasNamedParameter( + constructor.function, + _creationLocationParameterName, + )); + final VariableDeclaration variable = VariableDeclaration( + _creationLocationParameterName, + type: _locationClass.thisType, + ); + if (!_maybeAddNamedParameter(constructor.function, variable)) { + return; + } + + bool hasRedirectingInitializer = false; + for (Initializer initializer in constructor.initializers) { + if (initializer is RedirectingInitializer) { + if (initializer.target.enclosingClass == clazz) { + // We need to handle this constructor first or the call to + // addDebugLocationArgument bellow will fail due to the named + // parameter not yet existing on the constructor. + handleConstructor(initializer.target); + } + _maybeAddCreationLocationArgument( + initializer.arguments, + initializer.target.function, + VariableGet(variable), + _locationClass, + ); + hasRedirectingInitializer = true; + break; + } + } + if (!hasRedirectingInitializer) { + constructor.initializers.add(FieldInitializer( + locationField, + VariableGet(variable), + )); + // TODO(jacobr): add an assert verifying the locationField is not + // null. Currently, we cannot safely add this assert because we do not + // handle Widget classes with optional positional arguments. There are + // no Widget classes in the flutter repo with optional positional + // arguments but it is possible users could add classes with optional + // positional arguments. + // + // constructor.initializers.add(AssertInitializer(AssertStatement( + // IsExpression( + // VariableGet(variable), _locationClass.thisType), + // conditionStartOffset: constructor.fileOffset, + // conditionEndOffset: constructor.fileOffset, + // ))); + } + } + + // Add named parameters to all constructors. + clazz.constructors.forEach(handleConstructor); + } + + Component _computeFullProgram(Component deltaProgram) { + final Set libraries = {}; + final List workList = []; + for (Library library in deltaProgram.libraries) { + if (libraries.add(library)) { + workList.add(library); + } + } + while (workList.isNotEmpty) { + final Library library = workList.removeLast(); + for (LibraryDependency dependency in library.dependencies) { + if (libraries.add(dependency.targetLibrary)) { + workList.add(dependency.targetLibrary); + } + } + } + return Component()..libraries.addAll(libraries); + } + + /// Transform the given [program]. + /// + /// It is safe to call this method on a delta program generated as part of + /// performing a hot reload. + @override + void transform(Component program) { + final List libraries = program.libraries; + + if (libraries.isEmpty) { + return; + } + + _resolveFlutterClasses(libraries); + + if (_widgetClass == null) { + // This application doesn't actually use the package:flutter library. + return; + } + + // TODO(jacobr): once there is a working incremental ClassHierarchy + // constructor switch to using it instead of building a ClassHierarchy off + // the full program. + hierarchy = ClassHierarchy( + _computeFullProgram(program), + onAmbiguousSupertypes: (Class cls, Supertype a, Supertype b) { }, + ); + + final Set transformedClasses = Set.identity(); + final Set librariesToTransform = Set.identity() + ..addAll(libraries); + + for (Library library in libraries) { + if (library.isExternal) { + continue; + } + for (Class class_ in library.classes) { + _transformWidgetConstructors( + librariesToTransform, + transformedClasses, + class_, + ); + } + } + + // Transform call sites to pass the location parameter. + final _WidgetCallSiteTransformer callsiteTransformer = + _WidgetCallSiteTransformer( + hierarchy, + widgetClass: _widgetClass, + locationClass: _locationClass, + ); + + for (Library library in libraries) { + if (library.isExternal) { + continue; + } + library.transformChildren(callsiteTransformer); + } + } + + bool _isSubclassOfWidget(Class clazz) { + if (clazz == null) { + return false; + } + // TODO(jacobr): use hierarchy.isSubclassOf once we are using the + // non-deprecated ClassHierarchy constructor. + return hierarchy.isSubclassOf(clazz, _widgetClass); + } + + void _transformWidgetConstructors(Set librariesToBeTransformed, + Set transformedClasses, Class clazz) { + if (!_isSubclassOfWidget(clazz) || + !librariesToBeTransformed.contains(clazz.enclosingLibrary) || + !transformedClasses.add(clazz)) { + return; + } + + // Ensure super classes have been transformed before this class. + if (clazz.superclass != null && + !transformedClasses.contains(clazz.superclass)) { + _transformWidgetConstructors( + librariesToBeTransformed, + transformedClasses, + clazz.superclass, + ); + } + + for (Procedure procedure in clazz.procedures) { + if (procedure.isFactory) { + _maybeAddNamedParameter( + procedure.function, + VariableDeclaration( + _creationLocationParameterName, + type: _locationClass.thisType, + ), + ); + } + } + + // Handle the widget class and classes that implement but do not extend the + // widget class. + if (!_isSubclassOfWidget(clazz.superclass)) { + _transformClassImplementingWidget(clazz); + return; + } + + final Set _handledConstructors = + Set.identity(); + + void handleConstructor(Constructor constructor) { + if (!_handledConstructors.add(constructor)) { + return; + } + + final VariableDeclaration variable = VariableDeclaration( + _creationLocationParameterName, + type: _locationClass.thisType, + ); + if (_hasNamedParameter( + constructor.function, _creationLocationParameterName)) { + // Constructor was already rewritten. TODO(jacobr): is this case actually hit? + return; + } + if (!_maybeAddNamedParameter(constructor.function, variable)) { + return; + } + for (Initializer initializer in constructor.initializers) { + if (initializer is RedirectingInitializer) { + if (initializer.target.enclosingClass == clazz) { + // We need to handle this constructor first or the call to + // addDebugLocationArgument could fail due to the named parameter + // not existing. + handleConstructor(initializer.target); + } + + _maybeAddCreationLocationArgument( + initializer.arguments, + initializer.target.function, + VariableGet(variable), + _locationClass, + ); + } else if (initializer is SuperInitializer && + _isSubclassOfWidget(initializer.target.enclosingClass)) { + _maybeAddCreationLocationArgument( + initializer.arguments, + initializer.target.function, + VariableGet(variable), + _locationClass, + ); + } + } + } + + clazz.constructors.forEach(handleConstructor); + } +} diff --git a/flutter_kernel_transformers/pubspec.yaml b/flutter_kernel_transformers/pubspec.yaml new file mode 100644 index 0000000000000..e23088b859200 --- /dev/null +++ b/flutter_kernel_transformers/pubspec.yaml @@ -0,0 +1,13 @@ +name: flutter_kernel_transformers +version: 0.0.1-dev +description: Kernel transformers that operate on Flutter source code. +homepage: http://flutter.io +author: Flutter Authors + +dependencies: + kernel: any + meta: any + +dev_dependencies: + test: any + when: any diff --git a/frontend_server/BUILD.gn b/frontend_server/BUILD.gn index 7d3dfd5f6892c..21660d2976c76 100644 --- a/frontend_server/BUILD.gn +++ b/frontend_server/BUILD.gn @@ -22,6 +22,7 @@ if (is_fuchsia_host || is_fuchsia) { "//third_party/dart/pkg/front_end", "//third_party/dart/pkg/kernel", "//third_party/dart/pkg/vm", + "//third_party/flutter/flutter_kernel_transformers", ] } @@ -60,6 +61,14 @@ if (is_fuchsia_host || is_fuchsia) { ], "list lines") + frontend_server_files += + exec_script("//third_party/dart/tools/list_dart_files.py", + [ + "absolute", + rebase_path("../flutter_kernel_transformers"), + ], + "list lines") + frontend_server_files += exec_script("//third_party/dart/tools/list_dart_files.py", [ diff --git a/frontend_server/lib/server.dart b/frontend_server/lib/server.dart index b3a61f181e308..9620a5de0bd57 100644 --- a/frontend_server/lib/server.dart +++ b/frontend_server/lib/server.dart @@ -10,6 +10,7 @@ import 'dart:io' hide FileSystemEntity; import 'package:args/args.dart'; import 'package:path/path.dart' as path; +import 'package:flutter_kernel_transformers/track_widget_constructor_locations.dart'; import 'package:vm/incremental_compiler.dart'; import 'package:vm/frontend_server.dart' as frontend show FrontendCompiler, CompilerInterface, listenAndCompile, argParser, usage; @@ -20,8 +21,9 @@ class _FlutterFrontendCompiler implements frontend.CompilerInterface{ final frontend.CompilerInterface _compiler; _FlutterFrontendCompiler(StringSink output, - {bool unsafePackageSerialization}) : + {bool trackWidgetCreation = false, bool unsafePackageSerialization}) : _compiler = frontend.FrontendCompiler(output, + transformer: trackWidgetCreation ? WidgetCreatorTracker() : null, unsafePackageSerialization: unsafePackageSerialization); @override @@ -100,10 +102,8 @@ Future starter( '--incremental', '--sdk-root=$sdkRoot', '--output-dill=$outputTrainingDill', - '--target=flutter', - '--track-widget-creation', - ]); - compiler ??= _FlutterFrontendCompiler(output); + '--target=flutter']); + compiler ??= _FlutterFrontendCompiler(output, trackWidgetCreation: true); await compiler.compile(Platform.script.toFilePath(), options); compiler.acceptLastDelta(); @@ -121,6 +121,7 @@ Future starter( } compiler ??= _FlutterFrontendCompiler(output, + trackWidgetCreation: options['track-widget-creation'], unsafePackageSerialization: options['unsafe-package-serialization']); if (options.rest.isNotEmpty) { diff --git a/frontend_server/pubspec.yaml b/frontend_server/pubspec.yaml index badc50e49d184..de6263ccd8480 100644 --- a/frontend_server/pubspec.yaml +++ b/frontend_server/pubspec.yaml @@ -12,6 +12,7 @@ dependencies: convert: any crypto: any front_end: any + flutter_kernel_transformers: any kernel: any logging: any meta: any diff --git a/tools/generate_package_files.py b/tools/generate_package_files.py index 13399b1263de0..39dc60d20905d 100644 --- a/tools/generate_package_files.py +++ b/tools/generate_package_files.py @@ -3,14 +3,16 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -# This script generates .packages file for frontend_server from Dart SDKs -# .packages file located in third_party/dart/.packages +# This script generates .packages file for frontend_server and +# flutter_kernel_transformers from Dart SDKs .packages file located in +# third_party/dart/.packages import os import shutil ALL_PACKAGES = { - 'frontend_server': [], + 'frontend_server': ['flutter_kernel_transformers'], + 'flutter_kernel_transformers': [], } SRC_DIR = os.getcwd()