From 8df1757d3566ffe699987673aec769b8f690882f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 16 Jan 2020 17:01:20 -0800 Subject: [PATCH] const finder (#15668) --- BUILD.gn | 1 + testing/run_tests.py | 11 +- .../.dart_tool/package_config.json | 38 +++++ tools/const_finder/.gitignore | 6 + tools/const_finder/.packages | 6 + tools/const_finder/BUILD.gn | 23 +++ tools/const_finder/README.md | 11 ++ tools/const_finder/bin/main.dart | 86 +++++++++++ tools/const_finder/lib/const_finder.dart | 123 ++++++++++++++++ tools/const_finder/pubspec.yaml | 35 +++++ .../const_finder/test/const_finder_test.dart | 136 ++++++++++++++++++ tools/const_finder/test/fixtures/.packages | 2 + .../test/fixtures/lib/consts.dart | 17 +++ .../test/fixtures/lib/consts_and_non.dart | 17 +++ .../test/fixtures/lib/target.dart | 14 ++ 15 files changed, 525 insertions(+), 1 deletion(-) create mode 100644 tools/const_finder/.dart_tool/package_config.json create mode 100644 tools/const_finder/.gitignore create mode 100644 tools/const_finder/.packages create mode 100644 tools/const_finder/BUILD.gn create mode 100644 tools/const_finder/README.md create mode 100644 tools/const_finder/bin/main.dart create mode 100644 tools/const_finder/lib/const_finder.dart create mode 100644 tools/const_finder/pubspec.yaml create mode 100644 tools/const_finder/test/const_finder_test.dart create mode 100644 tools/const_finder/test/fixtures/.packages create mode 100644 tools/const_finder/test/fixtures/lib/consts.dart create mode 100644 tools/const_finder/test/fixtures/lib/consts_and_non.dart create mode 100644 tools/const_finder/test/fixtures/lib/target.dart diff --git a/BUILD.gn b/BUILD.gn index ce3a55c500b22..dd5add06f68b4 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -37,6 +37,7 @@ group("flutter") { if (current_toolchain == host_toolchain) { public_deps += [ "$flutter_root/shell/testing" ] + public_deps += [ "//flutter/tools/const_finder" ] } if (is_fuchsia && using_fuchsia_sdk) { diff --git a/testing/run_tests.py b/testing/run_tests.py index efa49b0942399..6dcaa7accb898 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -307,8 +307,16 @@ def RunDartTests(build_dir, filter, verbose_dart_snapshot): RunDartTest(build_dir, dart_test_file, verbose_dart_snapshot, True) RunDartTest(build_dir, dart_test_file, verbose_dart_snapshot, False) +def RunConstFinderTests(build_dir): + test_dir = os.path.join(buildroot_dir, 'flutter', 'tools', 'const_finder', 'test') + opts = [ + os.path.join(test_dir, 'const_finder_test.dart'), + os.path.join(build_dir, 'gen', 'frontend_server.dart.snapshot'), + os.path.join(build_dir, 'flutter_patched_sdk')] + RunEngineExecutable(build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=test_dir) + def main(): - parser = argparse.ArgumentParser(); + parser = argparse.ArgumentParser() parser.add_argument('--variant', dest='variant', action='store', default='host_debug_unopt', help='The engine build variant to run the tests for.'); @@ -344,6 +352,7 @@ def main(): assert not IsWindows(), "Dart tests can't be run on windows. https://github.com/flutter/flutter/issues/36301." dart_filter = args.dart_filter.split(',') if args.dart_filter else None RunDartTests(build_dir, dart_filter, args.verbose_dart_snapshot) + RunConstFinderTests(build_dir) if 'java' in types: assert not IsWindows(), "Android engine files can't be compiled on Windows." diff --git a/tools/const_finder/.dart_tool/package_config.json b/tools/const_finder/.dart_tool/package_config.json new file mode 100644 index 0000000000000..5ca024b38e7ab --- /dev/null +++ b/tools/const_finder/.dart_tool/package_config.json @@ -0,0 +1,38 @@ +{ + "configVersion": 2, + "packages": [ + { + "name": "args", + "rootUri": "../../../../third_party/dart/third_party/pkg/args", + "packageUri": "lib/", + "languageVersion": "2.0" + }, + { + "name": "kernel", + "rootUri": "../../../../third_party/dart/pkg/kernel", + "packageUri": "lib/", + "languageVersion": "2.2" + }, + { + "name": "meta", + "rootUri": "../../../../third_party/dart/pkg/meta", + "packageUri": "lib/", + "languageVersion": "1.12" + }, + { + "name": "path", + "rootUri": "../../../../third_party/dart/third_party/pkg/path", + "packageUri": "lib/", + "languageVersion": "2.0" + }, + { + "name": "const_finder", + "rootUri": "../", + "packageUri": "lib/", + "languageVersion": "2.4" + } + ], + "generated": "2020-01-16T19:11:54.963296Z", + "generator": "pub", + "generatorVersion": "2.7.0" +} diff --git a/tools/const_finder/.gitignore b/tools/const_finder/.gitignore new file mode 100644 index 0000000000000..da3df735e84e7 --- /dev/null +++ b/tools/const_finder/.gitignore @@ -0,0 +1,6 @@ +*.dill + +!.dart_tool +!.dart_tool/package_config.json +!.packages +!test/fixtures/.packages diff --git a/tools/const_finder/.packages b/tools/const_finder/.packages new file mode 100644 index 0000000000000..2a698fd32e89e --- /dev/null +++ b/tools/const_finder/.packages @@ -0,0 +1,6 @@ +# Generated by pub on 2020-01-16 11:11:54.947929. +args:../../../third_party/dart/third_party/pkg/args/lib/ +kernel:../../../third_party/dart/pkg/kernel/lib/ +meta:../../../third_party/dart/pkg/meta/lib/ +path:../../../third_party/dart/third_party/pkg/path/lib/ +const_finder:lib/ diff --git a/tools/const_finder/BUILD.gn b/tools/const_finder/BUILD.gn new file mode 100644 index 0000000000000..6aa045aeb591a --- /dev/null +++ b/tools/const_finder/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. + +import("//third_party/dart/utils/application_snapshot.gni") + +application_snapshot("const_finder") { + main_dart = "bin/main.dart" + dot_packages = ".packages" + + training_args = [ "--help" ] + + inputs = [ + "bin/main.dart", + "lib/const_finder.dart", + ".packages", + ".dart_tool/package_config.json", + ] + + deps = [ + "//flutter/flutter_frontend_server:frontend_server", + ] +} diff --git a/tools/const_finder/README.md b/tools/const_finder/README.md new file mode 100644 index 0000000000000..dce32b80cc0a3 --- /dev/null +++ b/tools/const_finder/README.md @@ -0,0 +1,11 @@ +# Const Finder + +This program uses package:kernel from the Dart SDK in //third_party. + +A snapshot is created via the build rules in BUILD.gn. This is then vended +to the Flutter tool, which uses it to find `const` creations of `IconData` +classes. The information from this can then be passed to the `font-subset` tool +to create a smaller icon font file specific to the application. + +Once [flutter/flutter#47162](https://github.com/flutter/flutter/issues/47162) is +resolved, this package should be moved to the flutter tool. diff --git a/tools/const_finder/bin/main.dart b/tools/const_finder/bin/main.dart new file mode 100644 index 0000000000000..d29a957118983 --- /dev/null +++ b/tools/const_finder/bin/main.dart @@ -0,0 +1,86 @@ +// 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:io'; + +import 'package:args/args.dart'; +import 'package:const_finder/const_finder.dart'; + +void main(List args) { + final ArgParser parser = ArgParser(); + parser + ..addSeparator('Finds constant instances of a specified class from the\n' + 'specified package, and outputs JSON like the following:') + ..addSeparator(''' + { + "constantInstances": [ + { + "codePoint": 59470, + "fontFamily": "MaterialIcons", + "fontPackage": null, + "matchTextDirection": false + } + ], + "nonConstantInstances": [ + { + "file": "file:///Path/to/hello_world/lib/file.dart", + "line": 19, + "column": 11 + } + ] + }''') + ..addSeparator('Where the "constantInstances" is a list of objects containing\n' + 'the properties passed to the const constructor of the class, and\n' + '"nonConstantInstances" is a list of source locations of non-constant\n' + 'creation of the specified class. Non-constant creation cannot be\n' + 'statically evaluated by this tool, and callers may wish to treat them\n' + 'as errors. The non-constant creation may include entries that are not\n' + 'reachable at runtime.') + ..addSeparator('Required arguments:') + ..addOption('kernel-file', + valueHelp: 'path/to/main.dill', + help: 'The path to a kernel file to parse, which was created from the ' + 'main-package-uri library.') + ..addOption('main-library-uri', + help: 'The package: URI to treat as the main entrypoint library ' + '(the same package used to create the kernel-file).', + valueHelp: 'package:hello_world/main.dart') + ..addOption('class-library-uri', + help: 'The package: URI of the class to find.', + valueHelp: 'package:flutter/src/widgets/icon_data.dart') + ..addOption('class-name', + help: 'The class name for the class to find.', valueHelp: 'IconData') + ..addSeparator('Optional arguments:') + ..addFlag('pretty', + defaultsTo: false, + negatable: false, + help: 'Pretty print JSON output (defaults to false).') + ..addFlag('help', + abbr: 'h', + defaultsTo: false, + negatable: false, + help: 'Print usage and exit'); + + final ArgResults argResults = parser.parse(args); + T getArg(String name) => argResults[name] as T; + + if (getArg('help')) { + stdout.writeln(parser.usage); + exit(0); + } + + final ConstFinder finder = ConstFinder( + kernelFilePath: getArg('kernel-file'), + targetLibraryUri: getArg('main-library-uri'), + classLibraryUri: getArg('class-library-uri'), + className: getArg('class-name'), + ); + + final JsonEncoder encoder = getArg('pretty') + ? const JsonEncoder.withIndent(' ') + : const JsonEncoder(); + + stdout.writeln(encoder.convert(finder.findInstances())); +} diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart new file mode 100644 index 0000000000000..b40ac249d9901 --- /dev/null +++ b/tools/const_finder/lib/const_finder.dart @@ -0,0 +1,123 @@ +// 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 'package:kernel/kernel.dart' hide MapEntry; +import 'package:meta/meta.dart'; + +class _ConstVisitor extends RecursiveVisitor { + _ConstVisitor( + this.kernelFilePath, + this.targetLibraryUri, + this.classLibraryUri, + this.className, + ) : assert(kernelFilePath != null), + assert(targetLibraryUri != null), + assert(classLibraryUri != null), + assert(className != null), + constantInstances = >[], + nonConstantLocations = >[]; + + /// The path to the file to open. + final String kernelFilePath; + + /// The library URI for the main entrypoint of the target library. + final String targetLibraryUri; + + /// The library URI for the class to find. + final String classLibraryUri; + + /// The name of the class to find. + final String className; + + final List> constantInstances; + final List> nonConstantLocations; + + bool _matches(Class node) { + return node.enclosingLibrary.canonicalName.name == classLibraryUri && + node.name == className; + } + + @override + void visitConstructorInvocation(ConstructorInvocation node) { + final Class parentClass = node.target.parent as Class; + if (!_matches(parentClass)) { + super.visitConstructorInvocation(node); + } + nonConstantLocations.add({ + 'file': node.location.file.toString(), + 'line': node.location.line, + 'column': node.location.column, + }); + } + + @override + void visitInstanceConstantReference(InstanceConstant node) { + if (!_matches(node.classNode)) { + return; + } + final Map instance = {}; + for (MapEntry kvp in node.fieldValues.entries) { + final PrimitiveConstant value = kvp.value as PrimitiveConstant; + instance[kvp.key.canonicalName.name] = value.value; + } + constantInstances.add(instance); + } +} + +/// A kernel AST visitor that finds const references. +class ConstFinder { + /// Creates a new ConstFinder class. All arguments are required and must not + /// be null. + /// + /// The `kernelFilePath` is the path to a dill (kernel) file to process. + /// + /// The `targetLibraryUri` is the `package:` URI of the main entrypoint to + /// search from. + /// + /// + /// + ConstFinder({ + @required String kernelFilePath, + @required String targetLibraryUri, + @required String classLibraryUri, + @required String className, + }) : _visitor = _ConstVisitor( + kernelFilePath, + targetLibraryUri, + classLibraryUri, + className, + ); + + final _ConstVisitor _visitor; + + Library _getRoot() { + final Component binary = loadComponentFromBinary(_visitor.kernelFilePath); + return binary.libraries.firstWhere( + (Library library) => library.canonicalName.name == _visitor.targetLibraryUri, + orElse: () => throw LibraryNotFoundException._(_visitor.targetLibraryUri), + ); + } + + /// Finds all instances + Map findInstances() { + final Library root = _getRoot(); + root.visitChildren(_visitor); + return { + 'constantInstances': _visitor.constantInstances, + 'nonConstantLocations': _visitor.nonConstantLocations, + }; + } +} + +/// Exception thrown by [ConstFinder.findInstances] when the target library +/// is not found. +class LibraryNotFoundException implements Exception { + const LibraryNotFoundException._(this.targetLibraryUri); + + /// The library target URI that could not be found. + final String targetLibraryUri; + + @override + String toString() => 'Could not find target library for "$targetLibraryUri".'; +} diff --git a/tools/const_finder/pubspec.yaml b/tools/const_finder/pubspec.yaml new file mode 100644 index 0000000000000..6411e1ef98992 --- /dev/null +++ b/tools/const_finder/pubspec.yaml @@ -0,0 +1,35 @@ +# 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. + +name: const_finder +publish_to: none + +environment: + sdk: ">=2.4.0 <3.0.0" + +# Do not add any dependencies that require more than what is provided in +# //third_party/dart/pkg or //third_party/dart/third_party/pkg. +# In particular, package:test is not usable here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart + +dependencies: + args: any + meta: any + kernel: any + +dev_dependencies: + path: any + +dependency_overrides: + args: + path: ../../../third_party/dart/third_party/pkg/args + kernel: + path: ../../../third_party/dart/pkg/kernel + meta: + path: ../../../third_party/dart/pkg/meta + path: + path: ../../../third_party/dart/third_party/pkg/path diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart new file mode 100644 index 0000000000000..cc9a2761755f8 --- /dev/null +++ b/tools/const_finder/test/const_finder_test.dart @@ -0,0 +1,136 @@ +// 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' show jsonEncode; +import 'dart:io'; + +import 'package:const_finder/const_finder.dart'; +import 'package:path/path.dart' as path; + +void expect(T value, T expected) { + if (value != expected) { + stderr.writeln('Expected: $expected'); + stderr.writeln('Actual: $value'); + exitCode = -1; + } +} + +final String basePath = + path.canonicalize(path.join(path.dirname(Platform.script.path), '..')); +final String fixtures = path.join(basePath, 'test', 'fixtures'); +final String consts = path.join(fixtures, 'lib', 'consts.dart'); +final String dotPackages = path.join(fixtures, '.packages'); +final String constsAndNon = path.join(fixtures, 'lib', 'consts_and_non.dart'); +final String constsDill = path.join(fixtures, 'consts.dill'); +final String constsAndNonDill = path.join(fixtures, 'consts_and_non.dill'); + +// This test is assuming the `dart` used to invoke the tests is compatible +// with the version of package:kernel in //third-party/dart/pkg/kernel +final String dart = Platform.resolvedExecutable; +final String bat = Platform.isWindows ? '.bat' : ''; + +void _checkConsts() { + print('Checking for expected constants.'); + final ConstFinder finder = ConstFinder( + kernelFilePath: constsDill, + targetLibraryUri: 'package:const_finder_fixtures/consts.dart', + classLibraryUri: 'package:const_finder_fixtures/target.dart', + className: 'Target', + ); + + expect( + jsonEncode(finder.findInstances()), + jsonEncode({ + 'constantInstances': >[ + {'stringValue': '1', 'intValue': 1}, + {'stringValue': '2', 'intValue': 2} + ], + 'nonConstantLocations': [], + }), + ); +} + +void _checkNonConsts() { + print('Checking for non-constant instances.'); + final ConstFinder finder = ConstFinder( + kernelFilePath: constsAndNonDill, + targetLibraryUri: 'package:const_finder_fixtures/consts_and_non.dart', + classLibraryUri: 'package:const_finder_fixtures/target.dart', + className: 'Target', + ); + + expect( + jsonEncode(finder.findInstances()), + jsonEncode({ + 'constantInstances': [ + {'stringValue': '1', 'intValue': 1} + ], + 'nonConstantLocations': [ + { + 'file': 'file://$fixtures/lib/consts_and_non.dart', + 'line': 12, + 'column': 26 + }, + { + 'file': 'file://$fixtures/lib/consts_and_non.dart', + 'line': 14, + 'column': 26 + }, + ] + }), + ); +} + +Future main(List args) async { + if (args.length != 2) { + stderr.writeln('The first argument must be the path to the forntend server dill.'); + stderr.writeln('The second argument must be the path to the flutter_patched_sdk'); + exit(-1); + } + final String frontendServer = args[0]; + final String sdkRoot = args[1]; + try { + void _checkProcessResult(ProcessResult result) { + if (result.exitCode != 0) { + stdout.writeln(result.stdout); + stderr.writeln(result.stderr); + } + expect(result.exitCode, 0); + } + + stdout.writeln('Generating kernel fixtures...'); + stdout.writeln(consts); + _checkProcessResult(Process.runSync(dart, [ + frontendServer, + '--sdk-root=$sdkRoot', + '--target=flutter', + '--aot', + '--tfa', + '--packages=$dotPackages', + '--output-dill=$constsDill', + consts, + ])); + + _checkProcessResult(Process.runSync(dart, [ + frontendServer, + '--sdk-root=$sdkRoot', + '--target=flutter', + '--aot', + '--tfa', + '--packages=$dotPackages', + '--output-dill=$constsAndNonDill', + constsAndNon, + ])); + + _checkConsts(); + _checkNonConsts(); + } finally { + try { + File(constsDill).deleteSync(); + File(constsAndNonDill).deleteSync(); + } finally { + stdout.writeln('Tests ${exitCode == 0 ? 'succeeded' : 'failed'} - exit code: $exitCode'); + } + } +} diff --git a/tools/const_finder/test/fixtures/.packages b/tools/const_finder/test/fixtures/.packages new file mode 100644 index 0000000000000..899ade66cd7b0 --- /dev/null +++ b/tools/const_finder/test/fixtures/.packages @@ -0,0 +1,2 @@ +# Generated by pub on 2020-01-15 10:08:29.776333. +const_finder_fixtures:lib/ diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart new file mode 100644 index 0000000000000..ef1969c491deb --- /dev/null +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -0,0 +1,17 @@ +// 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:core'; + +import 'target.dart'; + +void main() { + const Target target1 = Target('1', 1); + const Target target2 = Target('2', 2); + // ignore: unused_local_variable + const Target target3 = Target('3', 3); // should be tree shaken out. + target1.hit(); + target2.hit(); +} + diff --git a/tools/const_finder/test/fixtures/lib/consts_and_non.dart b/tools/const_finder/test/fixtures/lib/consts_and_non.dart new file mode 100644 index 0000000000000..d717faaeae3ae --- /dev/null +++ b/tools/const_finder/test/fixtures/lib/consts_and_non.dart @@ -0,0 +1,17 @@ +// 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. + +// ignore_for_file: prefer_const_constructors +import 'dart:core'; + +import 'target.dart'; + +void main() { + const Target target1 = Target('1', 1); + final Target target2 = Target('2', 2); + // ignore: unused_local_variable + final Target target3 = Target('3', 3); // should be tree shaken out. + target1.hit(); + target2.hit(); +} diff --git a/tools/const_finder/test/fixtures/lib/target.dart b/tools/const_finder/test/fixtures/lib/target.dart new file mode 100644 index 0000000000000..779fa8691c334 --- /dev/null +++ b/tools/const_finder/test/fixtures/lib/target.dart @@ -0,0 +1,14 @@ +// 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. + +class Target { + const Target(this.stringValue, this.intValue); + + final String stringValue; + final int intValue; + + void hit() { + print('$stringValue $intValue'); + } +}