Skip to content

Commit 80bc855

Browse files
dnfielddnfield
authored
Const finder fixes (flutter#15880)
1 parent 7e53e73 commit 80bc855

File tree

8 files changed

+117
-57
lines changed

8 files changed

+117
-57
lines changed

‎tools/const_finder/bin/main.dart‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ void main(List<String> args) {
4343
valueHelp: 'path/to/main.dill',
4444
help: 'The path to a kernel file to parse, which was created from the '
4545
'main-package-uri library.')
46-
..addOption('main-library-uri',
47-
help: 'The package: URI to treat as the main entrypoint library '
48-
'(the same package used to create the kernel-file).',
49-
valueHelp: 'package:hello_world/main.dart')
5046
..addOption('class-library-uri',
5147
help: 'The package: URI of the class to find.',
5248
valueHelp: 'package:flutter/src/widgets/icon_data.dart')
@@ -73,7 +69,6 @@ void main(List<String> args) {
7369

7470
final ConstFinder finder = ConstFinder(
7571
kernelFilePath: getArg<String>('kernel-file'),
76-
targetLibraryUri: getArg<String>('main-library-uri'),
7772
classLibraryUri: getArg<String>('class-library-uri'),
7873
className: getArg<String>('class-name'),
7974
);

‎tools/const_finder/lib/const_finder.dart‎

Lines changed: 15 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,25 @@ import 'package:meta/meta.dart';
88
class _ConstVisitor extends RecursiveVisitor<void> {
99
_ConstVisitor(
1010
this.kernelFilePath,
11-
this.targetLibraryUri,
1211
this.classLibraryUri,
1312
this.className,
1413
) : assert(kernelFilePath != null),
15-
assert(targetLibraryUri != null),
1614
assert(classLibraryUri != null),
1715
assert(className != null),
16+
_visitedInstnaces = <String>{},
1817
constantInstances = <Map<String, dynamic>>[],
1918
nonConstantLocations = <Map<String, dynamic>>[];
2019

2120
/// The path to the file to open.
2221
final String kernelFilePath;
2322

24-
/// The library URI for the main entrypoint of the target library.
25-
final String targetLibraryUri;
26-
2723
/// The library URI for the class to find.
2824
final String classLibraryUri;
2925

3026
/// The name of the class to find.
3127
final String className;
3228

29+
final Set<String> _visitedInstnaces;
3330
final List<Map<String, dynamic>> constantInstances;
3431
final List<Map<String, dynamic>> nonConstantLocations;
3532

@@ -43,6 +40,7 @@ class _ConstVisitor extends RecursiveVisitor<void> {
4340
final Class parentClass = node.target.parent as Class;
4441
if (!_matches(parentClass)) {
4542
super.visitConstructorInvocation(node);
43+
return;
4644
}
4745
nonConstantLocations.add(<String, dynamic>{
4846
'file': node.location.file.toString(),
@@ -53,15 +51,22 @@ class _ConstVisitor extends RecursiveVisitor<void> {
5351

5452
@override
5553
void visitInstanceConstantReference(InstanceConstant node) {
54+
node.fieldValues.values.whereType<InstanceConstant>().forEach(visitInstanceConstantReference);
5655
if (!_matches(node.classNode)) {
56+
super.visitInstanceConstantReference(node);
5757
return;
5858
}
5959
final Map<String, dynamic> instance = <String, dynamic>{};
6060
for (MapEntry<Reference, Constant> kvp in node.fieldValues.entries) {
61+
if (kvp.value is! PrimitiveConstant<dynamic>) {
62+
continue;
63+
}
6164
final PrimitiveConstant<dynamic> value = kvp.value as PrimitiveConstant<dynamic>;
6265
instance[kvp.key.canonicalName.name] = value.value;
6366
}
64-
constantInstances.add(instance);
67+
if (_visitedInstnaces.add(instance.toString())) {
68+
constantInstances.add(instance);
69+
}
6570
}
6671
}
6772

@@ -71,53 +76,27 @@ class ConstFinder {
7176
/// be null.
7277
///
7378
/// The `kernelFilePath` is the path to a dill (kernel) file to process.
74-
///
75-
/// The `targetLibraryUri` is the `package:` URI of the main entrypoint to
76-
/// search from.
77-
///
78-
///
79-
///
8079
ConstFinder({
8180
@required String kernelFilePath,
82-
@required String targetLibraryUri,
8381
@required String classLibraryUri,
8482
@required String className,
8583
}) : _visitor = _ConstVisitor(
8684
kernelFilePath,
87-
targetLibraryUri,
8885
classLibraryUri,
8986
className,
9087
);
9188

9289
final _ConstVisitor _visitor;
9390

94-
Library _getRoot() {
95-
final Component binary = loadComponentFromBinary(_visitor.kernelFilePath);
96-
return binary.libraries.firstWhere(
97-
(Library library) => library.canonicalName.name == _visitor.targetLibraryUri,
98-
orElse: () => throw LibraryNotFoundException._(_visitor.targetLibraryUri),
99-
);
100-
}
101-
10291
/// Finds all instances
10392
Map<String, dynamic> findInstances() {
104-
final Library root = _getRoot();
105-
root.visitChildren(_visitor);
93+
_visitor._visitedInstnaces.clear();
94+
for (Library library in loadComponentFromBinary(_visitor.kernelFilePath).libraries) {
95+
library.visitChildren(_visitor);
96+
}
10697
return <String, dynamic>{
10798
'constantInstances': _visitor.constantInstances,
10899
'nonConstantLocations': _visitor.nonConstantLocations,
109100
};
110101
}
111102
}
112-
113-
/// Exception thrown by [ConstFinder.findInstances] when the target library
114-
/// is not found.
115-
class LibraryNotFoundException implements Exception {
116-
const LibraryNotFoundException._(this.targetLibraryUri);
117-
118-
/// The library target URI that could not be found.
119-
final String targetLibraryUri;
120-
121-
@override
122-
String toString() => 'Could not find target library for "$targetLibraryUri".';
123-
}

‎tools/const_finder/test/const_finder_test.dart‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ void _checkConsts() {
3434
print('Checking for expected constants.');
3535
final ConstFinder finder = ConstFinder(
3636
kernelFilePath: constsDill,
37-
targetLibraryUri: 'package:const_finder_fixtures/consts.dart',
3837
classLibraryUri: 'package:const_finder_fixtures/target.dart',
3938
className: 'Target',
4039
);
@@ -43,8 +42,15 @@ void _checkConsts() {
4342
jsonEncode(finder.findInstances()),
4443
jsonEncode(<String, dynamic>{
4544
'constantInstances': <Map<String, dynamic>>[
46-
<String, dynamic>{'stringValue': '1', 'intValue': 1},
47-
<String, dynamic>{'stringValue': '2', 'intValue': 2}
45+
<String, dynamic>{'stringValue': '1', 'intValue': 1, 'targetValue': null},
46+
<String, dynamic>{'stringValue': '4', 'intValue': 4, 'targetValue': null},
47+
<String, dynamic>{'stringValue': '2', 'intValue': 2},
48+
<String, dynamic>{'stringValue': '6', 'intValue': 6, 'targetValue': null},
49+
<String, dynamic>{'stringValue': '8', 'intValue': 8, 'targetValue': null},
50+
<String, dynamic>{'stringValue': '10', 'intValue': 10, 'targetValue': null},
51+
<String, dynamic>{'stringValue': '9', 'intValue': 9},
52+
<String, dynamic>{'stringValue': '7', 'intValue': 7, 'targetValue': null},
53+
<String, dynamic>{'stringValue': 'package', 'intValue':-1, 'targetValue': null},
4854
],
4955
'nonConstantLocations': <dynamic>[],
5056
}),
@@ -55,7 +61,6 @@ void _checkNonConsts() {
5561
print('Checking for non-constant instances.');
5662
final ConstFinder finder = ConstFinder(
5763
kernelFilePath: constsAndNonDill,
58-
targetLibraryUri: 'package:const_finder_fixtures/consts_and_non.dart',
5964
classLibraryUri: 'package:const_finder_fixtures/target.dart',
6065
className: 'Target',
6166
);
@@ -64,19 +69,34 @@ void _checkNonConsts() {
6469
jsonEncode(finder.findInstances()),
6570
jsonEncode(<String, dynamic>{
6671
'constantInstances': <dynamic>[
67-
<String, dynamic>{'stringValue': '1', 'intValue': 1}
72+
<String, dynamic>{'stringValue': '1', 'intValue': 1, 'targetValue': null},
73+
<String, dynamic>{'stringValue': '6', 'intValue': 6, 'targetValue': null},
74+
<String, dynamic>{'stringValue': '8', 'intValue': 8, 'targetValue': null},
75+
<String, dynamic>{'stringValue': '10', 'intValue': 10, 'targetValue': null},
76+
<String, dynamic>{'stringValue': '9', 'intValue': 9},
77+
<String, dynamic>{'stringValue': '7', 'intValue': 7, 'targetValue': null},
6878
],
6979
'nonConstantLocations': <dynamic>[
7080
<String, dynamic>{
7181
'file': 'file://$fixtures/lib/consts_and_non.dart',
72-
'line': 12,
73-
'column': 26
82+
'line': 14,
83+
'column': 26,
7484
},
7585
<String, dynamic>{
7686
'file': 'file://$fixtures/lib/consts_and_non.dart',
77-
'line': 14,
78-
'column': 26
87+
'line': 17,
88+
'column': 26,
89+
},
90+
<String, dynamic>{
91+
'file': 'file://$fixtures/lib/consts_and_non.dart',
92+
'line': 19,
93+
'column': 26,
7994
},
95+
<String, dynamic>{
96+
'file': 'file://$fixtures/pkg/package.dart',
97+
'line': 10,
98+
'column': 25,
99+
}
80100
]
81101
}),
82102
);

‎tools/const_finder/test/fixtures/.packages‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
# Generated by pub on 2020-01-15 10:08:29.776333.
22
const_finder_fixtures:lib/
3+
const_finder_fixtures_package:pkg/

‎tools/const_finder/test/fixtures/lib/consts.dart‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,38 @@
44

55
import 'dart:core';
66

7+
import 'package:const_finder_fixtures_package/package.dart';
8+
79
import 'target.dart';
810

911
void main() {
10-
const Target target1 = Target('1', 1);
11-
const Target target2 = Target('2', 2);
12+
const Target target1 = Target('1', 1, null);
13+
const Target target2 = Target('2', 2, Target('4', 4, null));
1214
// ignore: unused_local_variable
13-
const Target target3 = Target('3', 3); // should be tree shaken out.
15+
const Target target3 = Target('3', 3, Target('5', 5, null)); // should be tree shaken out.
1416
target1.hit();
1517
target2.hit();
18+
19+
blah(const Target('6', 6, null));
20+
21+
const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not.
22+
// ignore: prefer_const_constructors
23+
final IgnoreMe ignoreMe2 = IgnoreMe(const Target('8', 8, null));
24+
// ignore: prefer_const_constructors
25+
final IgnoreMe ignoreMe3 = IgnoreMe(const Target('9', 9, Target('10', 10, null)));
26+
print(ignoreMe);
27+
print(ignoreMe2);
28+
print(ignoreMe3);
29+
30+
createTargetInPackage();
1631
}
1732

33+
class IgnoreMe {
34+
const IgnoreMe([this.target]);
35+
36+
final Target target;
37+
}
38+
39+
void blah(Target target) {
40+
print(target);
41+
}

‎tools/const_finder/test/fixtures/lib/consts_and_non.dart‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,41 @@
55
// ignore_for_file: prefer_const_constructors
66
import 'dart:core';
77

8+
import 'package:const_finder_fixtures_package/package.dart';
9+
810
import 'target.dart';
911

1012
void main() {
11-
const Target target1 = Target('1', 1);
12-
final Target target2 = Target('2', 2);
13+
const Target target1 = Target('1', 1, null);
14+
final Target target2 = Target('2', 2, const Target('4', 4, null));
15+
16+
// ignore: unused_local_variable
17+
final Target target3 = Target('3', 3, Target('5', 5, null)); // should be tree shaken out.
1318
// ignore: unused_local_variable
14-
final Target target3 = Target('3', 3); // should be tree shaken out.
19+
final Target target6 = Target('6', 6, null); // should be tree shaken out.
1520
target1.hit();
1621
target2.hit();
22+
23+
blah(const Target('6', 6, null));
24+
25+
const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not.
26+
// ignore: prefer_const_constructors
27+
final IgnoreMe ignoreMe2 = IgnoreMe(const Target('8', 8, null));
28+
// ignore: prefer_const_constructors
29+
final IgnoreMe ignoreMe3 = IgnoreMe(const Target('9', 9, Target('10', 10, null)));
30+
print(ignoreMe);
31+
print(ignoreMe2);
32+
print(ignoreMe3);
33+
34+
createNonConstTargetInPackage();
35+
}
36+
37+
class IgnoreMe {
38+
const IgnoreMe([this.target]);
39+
40+
final Target target;
41+
}
42+
43+
void blah(Target target) {
44+
print(target);
1745
}

‎tools/const_finder/test/fixtures/lib/target.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
// found in the LICENSE file.
44

55
class Target {
6-
const Target(this.stringValue, this.intValue);
6+
const Target(this.stringValue, this.intValue, this.targetValue);
77

88
final String stringValue;
99
final int intValue;
10+
final Target targetValue;
1011

1112
void hit() {
1213
print('$stringValue $intValue');

‎tools/const_finder/test/fixtures/pkg/package.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import 'package:const_finder_fixtures/target.dart';
2+
3+
void createTargetInPackage() {
4+
const Target target = Target('package', -1, null);
5+
target.hit();
6+
}
7+
8+
void createNonConstTargetInPackage() {
9+
// ignore: prefer_const_constructors
10+
final Target target = Target('package_non', -2, null);
11+
target.hit();
12+
}

0 commit comments

Comments
 (0)