firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Use track-widget-creation transformer included in the sdk. (#9085)" (flutter/engine#9134)

Browse Source 
This reverts commit 03d5bb8f53.

With this change applied, test/widgets/widget_inspector_test.dart is failing
when run by the dev/bots/test.dart script in the framework tree.
This commit is contained in:
Jason Simmons
2019-05-29 17:40:28 -07:00
committed by GitHub
parent 67711efcaf
commit 9397cd4316
9 changed files with 667 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
engine/src/flutter/ci/analyze.sh
View File

@@ -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 \

1
engine/src/flutter/ci/licenses_golden/licenses_flutter
View File

@@ -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

23
engine/src/flutter/flutter_kernel_transformers/BUILD.gn Normal file
View File

@@ -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",
]
}

595
engine/src/flutter/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart Normal file
View File

@@ -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<NamedExpression> arguments = <NamedExpression>[
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(<Expression>[], 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<ConstructorInvocation> parameterLocations =
<ConstructorInvocation>[];
final List<VariableDeclaration> 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<Library> 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, <DartType>[]));
// 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<Constructor> _handledConstructors =
Set<Constructor>.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<Library> libraries = <Library>{};
final List<Library> workList = <Library>[];
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<Library> 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<Class> transformedClasses = Set<Class>.identity();
final Set<Library> librariesToTransform = Set<Library>.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<Library> librariesToBeTransformed,
Set<Class> 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<Constructor> _handledConstructors =
Set<Constructor>.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);
}
}

13
engine/src/flutter/flutter_kernel_transformers/pubspec.yaml Normal file
View File

@@ -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 <flutter-dev@googlegroups.com>
dependencies:
kernel: any
meta: any
dev_dependencies:
test: any
when: any

9
engine/src/flutter/frontend_server/BUILD.gn
View File

@@ -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",
[

11
engine/src/flutter/frontend_server/lib/server.dart
View File

@@ -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<int> 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<int> starter(
}
compiler ??= _FlutterFrontendCompiler(output,
trackWidgetCreation: options['track-widget-creation'],
unsafePackageSerialization: options['unsafe-package-serialization']);
if (options.rest.isNotEmpty) {

1
engine/src/flutter/frontend_server/pubspec.yaml
View File

@@ -12,6 +12,7 @@ dependencies:
convert: any
crypto: any
front_end: any
flutter_kernel_transformers: any
kernel: any
logging: any
meta: any

8
engine/src/flutter/tools/generate_package_files.py
View File

@@ -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()