fix

Author: christopherfujino

tools/const_finder/lib/const_finder.dart

@@ -32,6 +32,31 @@ class _ConstVisitor extends RecursiveVisitor<void> {
 
   bool inIgnoredClass = false;
 
+  /// Whether or not we are currently within the declaration of the target class.
+  ///
+  /// We use this to determine when to skip tracking non-constant
+  /// [ConstructorInvocation]s. This is because, in web builds, a static
+  /// method is always created called _#new#tearOff() which returns the result
+  /// of a non-constant invocation of the unnamed constructor.
+  ///
+  /// For the following Dart class "FooBar":
+  ///
+  /// class FooBar {
+  ///   const FooBar();
+  /// }
+  ///
+  /// The following kernel structure is generated:
+  ///
+  /// class FooBar extends core::Object /*hasConstConstructor*/  {
+  ///   const constructor •() → min::FooBar
+  ///     : super core::Object::•()
+  ///     ;
+  ///   static method _#new#tearOff() → min::FooBar
+  ///     return new min::FooBar::•(); /* this is a non-const constructor invocation */
+  ///   method noOp() → void {}
+  /// }
+  bool inTargetClass = false;
+
   /// The name of the name of the class of the annotation marking classes
   /// whose constant references should be ignored.
   final String? annotationClassName;
@@ -73,7 +98,7 @@ class _ConstVisitor extends RecursiveVisitor<void> {
   @override
   void visitConstructorInvocation(ConstructorInvocation node) {
     final Class parentClass = node.target.parent! as Class;
-    if (_matches(parentClass)) {
+    if (_matches(parentClass) && !inTargetClass) {
       final Location location = node.location!;
       nonConstantLocations.add(<String, dynamic>{
         'file': location.file.toString(),
@@ -86,9 +111,11 @@ class _ConstVisitor extends RecursiveVisitor<void> {
 
   @override
   void visitClass(Class node) {
+    inTargetClass = _matches(node);
     // check if this is a class that we should ignore
     inIgnoredClass = _classShouldBeIgnored(node);
     super.visitClass(node);
+    inTargetClass = false;
     inIgnoredClass = false;
   }
 

tools/const_finder/test/const_finder_test.dart

@@ -31,10 +31,6 @@ void expectInstances(dynamic value, dynamic expected, Compiler compiler) {
 
   final Equality<Object?> equality;
   if (compiler == Compiler.dart2js) {
-    // Ignore comparing nonConstantLocations in web dills because it will have
-    // extra fields.
-    (value as Map<String, Object?>).remove('nonConstantLocations');
-    (expected as Map<String, Object?>).remove('nonConstantLocations');
     equality = const Dart2JSDeepCollectionEquality();
   } else {
     equality = const DeepCollectionEquality();
@@ -69,9 +65,7 @@ void _checkConsts(String dillPath, Compiler compiler) {
     classLibraryUri: 'package:const_finder_fixtures/target.dart',
     className: 'Target',
   );
-  expectInstances(
-    finder.findInstances(),
-    <String, dynamic>{
+  final Map<String, Object?> expectation = <String, dynamic>{
       'constantInstances': <Map<String, dynamic>>[
         <String, dynamic>{'stringValue': '100', 'intValue': 100, 'targetValue': null},
         <String, dynamic>{'stringValue': '102', 'intValue': 102, 'targetValue': null},
@@ -95,7 +89,27 @@ void _checkConsts(String dillPath, Compiler compiler) {
         <String, dynamic>{'stringValue': 'package', 'intValue':-1, 'targetValue': null},
       ],
       'nonConstantLocations': <dynamic>[],
-    },
+    };
+  if (compiler == Compiler.aot) {
+    expectation['nonConstantLocations'] = <Object?>[];
+  } else {
+    // Without true tree-shaking, there is a non-const reference in a
+    // never-invoked function that will be present in the dill.
+    final String fixturesUrl = Platform.isWindows
+      ? '/$fixtures'.replaceAll(Platform.pathSeparator, '/')
+      : fixtures;
+
+    expectation['nonConstantLocations'] = <Object?>[
+      <String, dynamic>{
+          'file': 'file://$fixturesUrl/pkg/package.dart',
+          'line': 14,
+          'column': 25,
+        },
+    ];
+  }
+  expectInstances(
+    finder.findInstances(),
+    expectation,
     compiler,
   );
 
@@ -212,6 +226,9 @@ void _checkNonConstsWeb(String dillPath, Compiler compiler) {
     className: 'Target',
   );
 
+  final String fixturesUrl = Platform.isWindows
+    ? '/$fixtures'.replaceAll(Platform.pathSeparator, '/')
+    : fixtures;
   expectInstances(
     finder.findInstances(),
     <String, dynamic>{
@@ -225,7 +242,33 @@ void _checkNonConstsWeb(String dillPath, Compiler compiler) {
         <String, dynamic>{'stringValue': '7', 'intValue': 7, 'targetValue': null},
         <String, dynamic>{'stringValue': 'package', 'intValue': -1, 'targetValue': null},
       ],
-      'nonConstantLocations': <dynamic>[]
+      'nonConstantLocations': <dynamic>[
+        <String, dynamic>{
+          'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
+          'line': 14,
+          'column': 26,
+        },
+        <String, dynamic>{
+          'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
+          'line': 16,
+          'column': 26,
+        },
+        <String, dynamic>{
+          'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
+          'line': 16,
+          'column': 41,
+        },
+        <String, dynamic>{
+          'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
+          'line': 17,
+          'column': 26,
+        },
+        <String, dynamic>{
+          'file': 'file://$fixturesUrl/pkg/package.dart',
+          'line': 14,
+          'column': 25,
+        }
+      ],
     },
     compiler,
   );