Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 5894e4b

Browse files
code review
1 parent 0977118 commit 5894e4b

File tree

3 files changed

+58
-21
lines changed

3 files changed

+58
-21
lines changed

tools/const_finder/bin/main.dart

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,30 @@ void main(List<String> args) {
5959
..addFlag('help',
6060
abbr: 'h',
6161
negatable: false,
62-
help: 'Print usage and exit');
62+
help: 'Print usage and exit')
63+
..addOption('annotation-class-name',
64+
help: 'The class name of the annotation for classes that should be '
65+
'ignored.',
66+
valueHelp: 'StaticIconProvider')
67+
..addOption('annotation-class-library-uri',
68+
help: 'The package: URI of the class of the annotation for classes '
69+
'that should be ignored.',
70+
valueHelp: 'package:flutter/src/material/icons.dart');
6371

6472
final ArgResults argResults = parser.parse(args);
6573
T getArg<T>(String name) => argResults[name] as T;
6674

75+
final String? annotationClassName = getArg<String?>('annotation-class-name');
76+
final String? annotationClassLibraryUri = getArg<String?>('annotation-class-library-uri');
77+
78+
final bool annotationClassNameProvided = annotationClassName != null;
79+
final bool annotationClassLibraryUriProvided = annotationClassLibraryUri != null;
80+
if (annotationClassNameProvided != annotationClassLibraryUriProvided) {
81+
throw StateError(
82+
'If either "--annotation-class-name" or "--annotation-class-library-uri" are provided they both must be',
83+
);
84+
}
85+
6786
if (getArg<bool>('help')) {
6887
stdout.writeln(parser.usage);
6988
exit(0);
@@ -73,6 +92,8 @@ void main(List<String> args) {
7392
kernelFilePath: getArg<String>('kernel-file'),
7493
classLibraryUri: getArg<String>('class-library-uri'),
7594
className: getArg<String>('class-name'),
95+
annotationClassName: annotationClassName,
96+
annotationClassLibraryUri: annotationClassLibraryUri,
7697
);
7798

7899
final JsonEncoder encoder = getArg<bool>('pretty')

tools/const_finder/lib/const_finder.dart

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ class _ConstVisitor extends RecursiveVisitor<void> {
1111
this.kernelFilePath,
1212
this.classLibraryUri,
1313
this.className,
14+
this.annotationClassLibraryUri,
15+
this.annotationClassName,
1416
) : _visitedInstances = <String>{},
1517
constantInstances = <Map<String, dynamic>>[],
1618
nonConstantLocations = <Map<String, dynamic>>[];
@@ -30,6 +32,14 @@ class _ConstVisitor extends RecursiveVisitor<void> {
3032

3133
bool inIgnoredClass = false;
3234

35+
/// The name of the name of the class of the annotation marking classes
36+
/// whose constant references should be ignored.
37+
final String? annotationClassName;
38+
39+
/// The library URI of the class of the annotation marking classes whose
40+
/// constant references should be ignored.
41+
final String? annotationClassLibraryUri;
42+
3343
// A cache of previously evaluated classes.
3444
static final Map<Class, bool> _classHeirarchyCache = <Class, bool>{};
3545
bool _matches(Class node) {
@@ -76,27 +86,27 @@ class _ConstVisitor extends RecursiveVisitor<void> {
7686

7787
@override
7888
void visitClass(Class node) {
79-
// Check if this is a class that we should ignore.
80-
if (!_isStaticIconProvider(node)) {
81-
// Not an ignored class.
82-
super.visitClass(node);
83-
return;
84-
}
85-
inIgnoredClass = true;
89+
// check if this is a class that we should ignore
90+
inIgnoredClass = _classShouldBeIgnored(node);
8691
super.visitClass(node);
8792
inIgnoredClass = false;
8893
}
8994

90-
// Constants from classes annotated with an instance of StaticIconProvider
91-
// should be ignored.
92-
bool _isStaticIconProvider(Class node) {
95+
// If any annotations on the class match annotationClassName AND
96+
// annotationClassLibraryUri.
97+
bool _classShouldBeIgnored(Class node) {
98+
if (annotationClassName == null || annotationClassLibraryUri == null) {
99+
return false;
100+
}
93101
return node.annotations.any((Expression expression) {
94102
if (expression is! ConstantExpression) {
95103
return false;
96104
}
97105

98-
final DartType type = expression.type;
99-
return type is InterfaceType && type.classNode.name == 'StaticIconProvider';
106+
final Constant constant = expression.constant;
107+
return constant is InstanceConstant
108+
&& constant.classNode.name == annotationClassName
109+
&& constant.classNode.enclosingLibrary.importUri.toString() == annotationClassLibraryUri;
100110
});
101111
}
102112

@@ -131,10 +141,14 @@ class ConstFinder {
131141
required String kernelFilePath,
132142
required String classLibraryUri,
133143
required String className,
144+
String? annotationClassLibraryUri,
145+
String? annotationClassName,
134146
}) : _visitor = _ConstVisitor(
135147
kernelFilePath,
136148
classLibraryUri,
137149
className,
150+
annotationClassLibraryUri,
151+
annotationClassName,
138152
);
139153

140154
final _ConstVisitor _visitor;

tools/const_finder/test/const_finder_test.dart

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ void _checkAnnotation(String dillPath, Compiler compiler) {
122122
kernelFilePath: dillPath,
123123
classLibraryUri: 'package:const_finder_fixtures/target.dart',
124124
className: 'Target',
125+
annotationClassName: 'StaticIconProvider',
126+
annotationClassLibraryUri: 'package:const_finder_fixtures/static_icon_provider.dart',
125127
);
126128
expectInstances(
127129
finder.findInstances(),
@@ -262,7 +264,7 @@ Future<void> main(List<String> args) async {
262264
sdkRoot: sdkRoot,
263265
librariesSpec: librariesSpec,
264266
verify: _checkRecursion,
265-
compiler: Compiler.frontendServer,
267+
compiler: Compiler.aot,
266268
),
267269
_Test(
268270
name: 'box_web',
@@ -280,7 +282,7 @@ Future<void> main(List<String> args) async {
280282
sdkRoot: sdkRoot,
281283
librariesSpec: librariesSpec,
282284
verify: _checkConsts,
283-
compiler: Compiler.frontendServer,
285+
compiler: Compiler.aot,
284286
),
285287
_Test(
286288
name: 'consts_web',
@@ -298,7 +300,7 @@ Future<void> main(List<String> args) async {
298300
sdkRoot: sdkRoot,
299301
librariesSpec: librariesSpec,
300302
verify: _checkNonConstsFrontend,
301-
compiler: Compiler.frontendServer,
303+
compiler: Compiler.aot,
302304
),
303305
_Test(
304306
name: 'consts_and_non_web',
@@ -316,7 +318,7 @@ Future<void> main(List<String> args) async {
316318
sdkRoot: sdkRoot,
317319
librariesSpec: librariesSpec,
318320
verify: _checkAnnotation,
319-
compiler: Compiler.frontendServer,
321+
compiler: Compiler.aot,
320322
),
321323
_Test(
322324
name: 'static_icon_provider_web',
@@ -347,7 +349,7 @@ Future<void> main(List<String> args) async {
347349

348350
enum Compiler {
349351
// Uses TFA tree-shaking.
350-
frontendServer,
352+
aot,
351353
// Does not have TFA tree-shaking.
352354
dart2js,
353355
}
@@ -377,8 +379,8 @@ class _Test {
377379
void run() {
378380
stdout.writeln('Compiling $dartSource to $dillPath with $compiler');
379381

380-
if (compiler == Compiler.frontendServer) {
381-
_compileTFADill();
382+
if (compiler == Compiler.aot) {
383+
_compileAOTDill();
382384
} else {
383385
_compileWebDill();
384386
}
@@ -395,7 +397,7 @@ class _Test {
395397
}
396398
}
397399

398-
void _compileTFADill() {
400+
void _compileAOTDill() {
399401
checkProcessResult(Process.runSync(dart, <String>[
400402
frontendServer,
401403
'--sdk-root=$sdkRoot',

0 commit comments

Comments
 (0)