[cfe] Detect Record and Function through builders

johnniwinther · Commit Queue · commit 5316613bde8d · 2025-02-28T04:41:50.000-08:00
The current check using Class doesn't work when compiling the platform and the Class node not yet been built. Change-Id: I3b9e692f761ebdfbfc34bf8334d7dc8ef31230a3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412643 Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/front_end/lib/src/builder/library_builder.dart b/pkg/front_end/lib/src/builder/library_builder.dart
@@ -3,7 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_fe_analyzer_shared/src/messages/severity.dart' show Severity;
-import 'package:kernel/ast.dart' show Annotatable, Class, Library, Version;
+import 'package:kernel/ast.dart' show Annotatable, Library, Version;
 import 'package:kernel/reference_from_index.dart';
 
 import '../api_prototype/experimental_flags.dart';
@@ -413,29 +413,22 @@ abstract class LibraryBuilder implements Builder, ProblemReporting {
   void recordAccess(
       CompilationUnit accessor, int charOffset, int length, Uri fileUri);
 
-  /// Returns `true` if [cls] is the 'Function' class defined in [coreLibrary].
-  static bool isFunction(Class cls, LibraryBuilder coreLibrary) {
-    return cls.name == 'Function' && _isCoreClass(cls, coreLibrary);
+  /// Returns `true` if [typeDeclarationBuilder] is the 'Function' class defined
+  /// in [coreLibrary].
+  static bool isFunction(TypeDeclarationBuilder? typeDeclarationBuilder,
+      LibraryBuilder coreLibrary) {
+    return typeDeclarationBuilder is ClassBuilder &&
+        typeDeclarationBuilder.name == 'Function' &&
+        typeDeclarationBuilder.libraryBuilder == coreLibrary;
   }
 
-  /// Returns `true` if [cls] is the 'Record' class defined in [coreLibrary].
-  static bool isRecord(Class cls, LibraryBuilder coreLibrary) {
-    return cls.name == 'Record' && _isCoreClass(cls, coreLibrary);
-  }
-
-  static bool _isCoreClass(Class cls, LibraryBuilder coreLibrary) {
-    // We use `superclass.parent` here instead of
-    // `superclass.enclosingLibrary` to handle platform compilation. If
-    // we are currently compiling the platform, the enclosing library of
-    // the core class has not yet been set, so the accessing
-    // `enclosingLibrary` would result in a cast error. We assume that the
-    // SDK does not contain this error, which we otherwise not find. If we
-    // are _not_ compiling the platform, the `superclass.parent` has been
-    // set, if it is a class from `dart:core`.
-    if (cls.parent == coreLibrary.library) {
-      return true;
-    }
-    return false;
+  /// Returns `true` if [typeDeclarationBuilder] is the 'Record' class defined
+  /// in [coreLibrary].
+  static bool isRecord(TypeDeclarationBuilder? typeDeclarationBuilder,
+      LibraryBuilder coreLibrary) {
+    return typeDeclarationBuilder is ClassBuilder &&
+        typeDeclarationBuilder.name == 'Record' &&
+        typeDeclarationBuilder.libraryBuilder == coreLibrary;
   }
 }
 
diff --git a/pkg/front_end/lib/src/source/source_class_builder.dart b/pkg/front_end/lib/src/source/source_class_builder.dart
@@ -409,13 +409,13 @@ class SourceClassBuilder extends ClassBuilderImpl
     if (_supertypeBuilder != null) {
       _supertypeBuilder = _checkSupertype(_supertypeBuilder!);
     }
-    Supertype? supertype = supertypeBuilder?.buildSupertype(libraryBuilder,
-        isMixinDeclaration ? TypeUse.mixinOnType : TypeUse.classExtendsType);
-    if (supertype != null &&
-        LibraryBuilder.isFunction(supertype.classNode, coreLibrary)) {
-      supertype = null;
+    TypeDeclarationBuilder? supertypeDeclaration =
+        supertypeBuilder?.computeUnaliasedDeclaration(isUsedAsClass: false);
+    if (LibraryBuilder.isFunction(supertypeDeclaration, coreLibrary)) {
       _supertypeBuilder = null;
     }
+    Supertype? supertype = supertypeBuilder?.buildSupertype(libraryBuilder,
+        isMixinDeclaration ? TypeUse.mixinOnType : TypeUse.classExtendsType);
     if (!isMixinDeclaration &&
         actualCls.supertype != null &&
         // Coverage-ignore(suite): Not run.
@@ -440,14 +440,15 @@ class SourceClassBuilder extends ClassBuilderImpl
     if (_mixedInTypeBuilder != null) {
       _mixedInTypeBuilder = _checkSupertype(_mixedInTypeBuilder!);
     }
-    Supertype? mixedInType =
-        _mixedInTypeBuilder?.buildMixedInType(libraryBuilder);
-    if (mixedInType != null &&
-        LibraryBuilder.isFunction(mixedInType.classNode, coreLibrary)) {
-      mixedInType = null;
+    TypeDeclarationBuilder? mixedInDeclaration =
+        _mixedInTypeBuilder?.computeUnaliasedDeclaration(isUsedAsClass: false);
+    if (LibraryBuilder.isFunction(mixedInDeclaration, coreLibrary)) {
       _mixedInTypeBuilder = null;
       actualCls.isAnonymousMixin = false;
     }
+    Supertype? mixedInType =
+        _mixedInTypeBuilder?.buildMixedInType(libraryBuilder);
+
     actualCls.isMixinDeclaration = isMixinDeclaration;
     actualCls.mixedInType = mixedInType;
 
@@ -464,12 +465,16 @@ class SourceClassBuilder extends ClassBuilderImpl
     if (interfaceBuilders != null) {
       for (int i = 0; i < interfaceBuilders.length; ++i) {
         interfaceBuilders[i] = _checkSupertype(interfaceBuilders[i]);
+        TypeDeclarationBuilder? implementedDeclaration = interfaceBuilders[i]
+            .computeUnaliasedDeclaration(isUsedAsClass: false);
+        if (LibraryBuilder.isFunction(implementedDeclaration, coreLibrary) &&
+            // Allow wasm to implement `Function`.
+            !libraryBuilder.mayImplementRestrictedTypes) {
+          continue;
+        }
         Supertype? supertype = interfaceBuilders[i]
             .buildSupertype(libraryBuilder, TypeUse.classImplementsType);
         if (supertype != null) {
-          if (LibraryBuilder.isFunction(supertype.classNode, coreLibrary)) {
-            continue;
-          }
           // TODO(ahe): Report an error if supertype is null.
           actualCls.implementedTypes.add(supertype);
         }
diff --git a/pkg/front_end/lib/src/source/source_extension_type_declaration_builder.dart b/pkg/front_end/lib/src/source/source_extension_type_declaration_builder.dart
@@ -216,10 +216,33 @@ class SourceExtensionTypeDeclarationBuilder
             typeBuilder.declaration is TypeAliasBuilder
                 ? typeBuilder.declaration as TypeAliasBuilder
                 : null;
+
         DartType interface = typeBuilder.build(
             libraryBuilder, TypeUse.extensionTypeImplementsType);
-        Message? errorMessage;
-        List<LocatedMessage>? errorContext;
+
+        TypeDeclarationBuilder? implementedDeclaration =
+            typeBuilder.computeUnaliasedDeclaration(isUsedAsClass: false);
+        if (LibraryBuilder.isFunction(implementedDeclaration, coreLibrary) ||
+            LibraryBuilder.isRecord(implementedDeclaration, coreLibrary)) {
+          Message? errorMessage;
+          List<LocatedMessage>? errorContext;
+          if (aliasBuilder != null) {
+            // Coverage-ignore-block(suite): Not run.
+            errorMessage = templateSuperExtensionTypeIsIllegalAliased
+                .withArguments(typeBuilder.fullNameForErrors, interface);
+            errorContext = [
+              messageTypedefCause.withLocation(
+                  aliasBuilder.fileUri, aliasBuilder.fileOffset, noLength),
+            ];
+          } else {
+            errorMessage = templateSuperExtensionTypeIsIllegal
+                .withArguments(typeBuilder.fullNameForErrors);
+          }
+          libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
+              noLength, typeBuilder.fileUri,
+              context: errorContext);
+          continue;
+        }
 
         if (typeParameters?.isNotEmpty ?? false) {
           for (NominalParameterBuilder variable in typeParameters!) {
@@ -228,6 +251,7 @@ class SourceExtensionTypeDeclarationBuilder
                     sourceLoader: libraryBuilder.loader)
                 .variance!;
             if (!variance.greaterThanOrEqual(variable.variance)) {
+              Message? errorMessage;
               if (variable.parameter.isLegacyCovariant) {
                 errorMessage =
                     templateWrongTypeParameterVarianceInSuperinterface
@@ -239,31 +263,33 @@ class SourceExtensionTypeDeclarationBuilder
                         .withArguments(variable.variance.keyword, variable.name,
                             variance.keyword, typeBuilder.typeName!.name);
               }
+              libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
+                  noLength, typeBuilder.fileUri);
             }
           }
-          if (errorMessage != null) {
-            libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
-                noLength, typeBuilder.fileUri,
-                context: errorContext);
-            errorMessage = null;
-          }
         }
 
         if (interface is ExtensionType) {
           if (interface.nullability == Nullability.nullable) {
-            errorMessage = templateSuperExtensionTypeIsNullableAliased
+            Message? errorMessage = templateSuperExtensionTypeIsNullableAliased
                 .withArguments(typeBuilder.fullNameForErrors, interface);
+            List<LocatedMessage>? errorContext;
             if (aliasBuilder != null) {
               errorContext = [
                 messageTypedefCause.withLocation(
                     aliasBuilder.fileUri, aliasBuilder.fileOffset, noLength),
               ];
             }
+            libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
+                noLength, typeBuilder.fileUri,
+                context: errorContext);
           } else {
             extensionTypeDeclaration.implements.add(interface);
           }
         } else if (interface is InterfaceType) {
           if (interface.isPotentiallyNullable) {
+            Message? errorMessage;
+            List<LocatedMessage>? errorContext;
             if (typeBuilder.nullabilityBuilder.isNullable) {
               errorMessage = templateNullableInterfaceError
                   .withArguments(typeBuilder.fullNameForErrors);
@@ -277,37 +303,29 @@ class SourceExtensionTypeDeclarationBuilder
                 ];
               }
             }
+            libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
+                noLength, typeBuilder.fileUri,
+                context: errorContext);
           } else {
-            Class cls = interface.classNode;
-            if (LibraryBuilder.isFunction(cls, coreLibrary) ||
-                LibraryBuilder.isRecord(cls, coreLibrary)) {
-              if (aliasBuilder != null) {
-                // Coverage-ignore-block(suite): Not run.
-                errorMessage = templateSuperExtensionTypeIsIllegalAliased
-                    .withArguments(typeBuilder.fullNameForErrors, interface);
-                errorContext = [
-                  messageTypedefCause.withLocation(
-                      aliasBuilder.fileUri, aliasBuilder.fileOffset, noLength),
-                ];
-              } else {
-                errorMessage = templateSuperExtensionTypeIsIllegal
-                    .withArguments(typeBuilder.fullNameForErrors);
-              }
-            } else {
-              extensionTypeDeclaration.implements.add(interface);
-            }
+            extensionTypeDeclaration.implements.add(interface);
           }
         } else if (interface is TypeParameterType) {
-          errorMessage = templateSuperExtensionTypeIsTypeParameter
+          Message? errorMessage = templateSuperExtensionTypeIsTypeParameter
               .withArguments(typeBuilder.fullNameForErrors);
+          List<LocatedMessage>? errorContext;
           if (aliasBuilder != null) {
             // Coverage-ignore-block(suite): Not run.
             errorContext = [
               messageTypedefCause.withLocation(
                   aliasBuilder.fileUri, aliasBuilder.fileOffset, noLength),
             ];
           }
+          libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
+              noLength, typeBuilder.fileUri,
+              context: errorContext);
         } else {
+          Message? errorMessage;
+          List<LocatedMessage>? errorContext;
           if (aliasBuilder != null) {
             errorMessage = templateSuperExtensionTypeIsIllegalAliased
                 .withArguments(typeBuilder.fullNameForErrors, interface);
@@ -319,8 +337,6 @@ class SourceExtensionTypeDeclarationBuilder
             errorMessage = templateSuperExtensionTypeIsIllegal
                 .withArguments(typeBuilder.fullNameForErrors);
           }
-        }
-        if (errorMessage != null) {
           libraryBuilder.addProblem(errorMessage, typeBuilder.charOffset!,
               noLength, typeBuilder.fileUri,
               context: errorContext);
diff --git a/pkg/front_end/test/coverage_suite_expected.dart b/pkg/front_end/test/coverage_suite_expected.dart
@@ -860,7 +860,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
   ),
   // 100.0%.
   "package:front_end/src/source/source_class_builder.dart": (
-    hitCount: 1397,
+    hitCount: 1402,
     missCount: 0,
   ),
   // 100.0%.
@@ -886,7 +886,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
   // 100.0%.
   "package:front_end/src/source/source_extension_type_declaration_builder.dart":
       (
-    hitCount: 495,
+    hitCount: 511,
     missCount: 0,
   ),
   // 100.0%.
