Skip to content

Commit

Permalink
Teach frontend compiler to replace toString with super.toString f…
Browse files Browse the repository at this point in the history
…or selected packages (flutter#17068)

Adds annotation `keepToString` to opt out.
  • Loading branch information
dnfield authored Mar 17, 2020
1 parent f00a135 commit 034f913
Show file tree
Hide file tree
Showing 13 changed files with 539 additions and 8 deletions.
10 changes: 7 additions & 3 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@
# private fields, especially on the Window object):

analyzer:
# this test pretends to be part of dart:ui and results in lots of false
# positives.
exclude: [ testing/dart/window_hooks_integration_test.dart ]
exclude: [
# this test pretends to be part of dart:ui and results in lots of false
# positives.
testing/dart/window_hooks_integration_test.dart,
# Fixture depends on dart:ui and raises false positives.
flutter_frontend_server/test/fixtures/lib/main.dart
]
strong-mode:
implicit-casts: false
implicit-dynamic: false
Expand Down
2 changes: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ FILE: ../../../flutter/lib/io/dart_io.cc
FILE: ../../../flutter/lib/io/dart_io.h
FILE: ../../../flutter/lib/snapshot/libraries.json
FILE: ../../../flutter/lib/snapshot/snapshot.h
FILE: ../../../flutter/lib/ui/annotations.dart
FILE: ../../../flutter/lib/ui/channel_buffers.dart
FILE: ../../../flutter/lib/ui/compositing.dart
FILE: ../../../flutter/lib/ui/compositing/scene.cc
Expand Down Expand Up @@ -494,6 +495,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/util.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/validators.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/window.dart
FILE: ../../../flutter/lib/web_ui/lib/src/ui/annotations.dart
FILE: ../../../flutter/lib/web_ui/lib/src/ui/canvas.dart
FILE: ../../../flutter/lib/web_ui/lib/src/ui/channel_buffers.dart
FILE: ../../../flutter/lib/web_ui/lib/src/ui/compositing.dart
Expand Down
104 changes: 99 additions & 5 deletions flutter_frontend_server/lib/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ import 'dart:async';
import 'dart:io' hide FileSystemEntity;

import 'package:args/args.dart';
import 'package:path/path.dart' as path;

import 'package:vm/incremental_compiler.dart';
import 'package:frontend_server/frontend_server.dart' as frontend
show
FrontendCompiler,
Expand All @@ -19,6 +16,9 @@ import 'package:frontend_server/frontend_server.dart' as frontend
argParser,
usage,
ProgramTransformer;
import 'package:kernel/ast.dart';
import 'package:path/path.dart' as path;
import 'package:vm/incremental_compiler.dart';

/// Wrapper around [FrontendCompiler] that adds [widgetCreatorTracker] kernel
/// transformation to the compilation.
Expand Down Expand Up @@ -107,6 +107,13 @@ Future<int> starter(
frontend.ProgramTransformer transformer,
}) async {
ArgResults options;
frontend.argParser.addMultiOption(
'delete-tostring-package-uri',
help: 'Replaces implementations of `toString` with `super.toString()` for '
'specified package',
valueHelp: 'dart:ui',
defaultsTo: const <String>[],
);
try {
options = frontend.argParser.parse(args);
} catch (error) {
Expand All @@ -115,6 +122,8 @@ Future<int> starter(
return 1;
}

final Set<String> deleteToStringPackageUris = (options['delete-tostring-package-uri'] as List<String>).toSet();

if (options['train'] as bool) {
if (!options.rest.isNotEmpty) {
throw Exception('Must specify input.dart');
Expand All @@ -137,7 +146,10 @@ Future<int> starter(
'--gen-bytecode',
'--bytecode-options=source-positions,local-var-info,debugger-stops,instance-field-initializers,keep-unreachable-code,avoid-closure-call-instructions',
]);
compiler ??= _FlutterFrontendCompiler(output);
compiler ??= _FlutterFrontendCompiler(
output,
transformer: ToStringTransformer(null, deleteToStringPackageUris),
);

await compiler.compile(input, options);
compiler.acceptLastDelta();
Expand All @@ -156,7 +168,7 @@ Future<int> starter(
}

compiler ??= _FlutterFrontendCompiler(output,
transformer: transformer,
transformer: ToStringTransformer(transformer, deleteToStringPackageUris),
useDebuggerModuleNames: options['debugger-module-names'] as bool,
unsafePackageSerialization:
options['unsafe-package-serialization'] as bool);
Expand All @@ -169,3 +181,85 @@ Future<int> starter(
frontend.listenAndCompile(compiler, input ?? stdin, options, completer);
return completer.future;
}

// Transformer/visitor for toString
// If we add any more of these, they really should go into a separate library.

/// A [RecursiveVisitor] that replaces [Object.toString] overrides with
/// `super.toString()`.
class ToStringVisitor extends RecursiveVisitor<void> {
/// The [packageUris] must not be null.
ToStringVisitor(this._packageUris) : assert(_packageUris != null);

/// A set of package URIs to apply this transformer to, e.g. 'dart:ui' and
/// 'package:flutter/foundation.dart'.
final Set<String> _packageUris;

/// Turn 'dart:ui' into 'dart:ui', or
/// 'package:flutter/src/semantics_event.dart' into 'package:flutter'.
String _importUriToPackage(Uri importUri) => '${importUri.scheme}:${importUri.pathSegments.first}';

bool _isInTargetPackage(Procedure node) {
return _packageUris.contains(_importUriToPackage(node.enclosingLibrary.importUri));
}

bool _hasKeepAnnotation(Procedure node) {
for (ConstantExpression expression in node.annotations.whereType<ConstantExpression>()) {
if (expression.constant is! InstanceConstant) {
continue;
}
final InstanceConstant constant = expression.constant as InstanceConstant;
if (constant.classNode.name == '_KeepToString' && constant.classNode.enclosingLibrary.importUri.toString() == 'dart:ui') {
return true;
}
}
return false;
}

@override
void visitProcedure(Procedure node) {
if (
node.name.name == 'toString' &&
node.enclosingClass != null &&
node.enclosingLibrary != null &&
!node.isStatic &&
!node.isAbstract &&
_isInTargetPackage(node) &&
!_hasKeepAnnotation(node)
) {
node.function.body.replaceWith(
ReturnStatement(
SuperMethodInvocation(
node.name,
Arguments(<Expression>[]),
),
),
);
}
}

@override
void defaultMember(Member node) {}
}

/// Replaces [Object.toString] overrides with calls to super for the specified
/// [packageUris].
class ToStringTransformer extends frontend.ProgramTransformer {
/// The [packageUris] parameter must not be null, but may be empty.
ToStringTransformer(this._child, this._packageUris) : assert(_packageUris != null);

final frontend.ProgramTransformer _child;

/// A set of package URIs to apply this transformer to, e.g. 'dart:ui' and
/// 'package:flutter/foundation.dart'.
final Set<String> _packageUris;

@override
void transform(Component component) {
assert(_child is! ToStringTransformer);
if (_packageUris.isNotEmpty) {
component.visitChildren(ToStringVisitor(_packageUris));
}
_child?.transform(component);
}
}
1 change: 1 addition & 0 deletions flutter_frontend_server/test/fixtures/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!.packages
2 changes: 2 additions & 0 deletions flutter_frontend_server/test/fixtures/.packages
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Generated by pub on 2020-01-15 10:08:29.776333.
flutter_frontend_fixtures:lib/
26 changes: 26 additions & 0 deletions flutter_frontend_server/test/fixtures/lib/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.

import 'dart:convert';
import 'dart:ui';

void main() {
final Paint paint = Paint()..color = Color(0xFFFFFFFF);
print(jsonEncode(<String, String>{
'Paint.toString': paint.toString(),
'Foo.toString': Foo().toString(),
'Keep.toString': Keep().toString(),
}));
}

class Foo {
@override
String toString() => 'I am a Foo';
}

class Keep {
@keepToString
@override
String toString() => 'I am a Keep';
}
Loading

0 comments on commit 034f913

Please sign in to comment.