[cfe] Disallow implementing a legacy library subclass of a final/base class in the core libraries.

This behaviour should only happen when a post-feature library implements
a pre-feature library declaration that has a final/base core library class as a super declaration.

Bug: #52115
Change-Id: If42129ba3ba7e337cc6ffc21604c6d0f2976344c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301503
Reviewed-by: Nate Bosch <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: kallentu

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -367,34 +367,6 @@ Message _withArgumentsBaseClassImplementedOutsideOfLibrary(String name) {
       arguments: {'name': name});
 }
 
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Template<Message Function(String name, String name2)>
-    templateBaseClassImplementedOutsideOfLibraryCause =
-    const Template<Message Function(String name, String name2)>(
-        problemMessageTemplate:
-            r"""The type '#name' is a subtype of '#name2', and '#name2' is defined here.""",
-        withArguments: _withArgumentsBaseClassImplementedOutsideOfLibraryCause);
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Code<Message Function(String name, String name2)>
-    codeBaseClassImplementedOutsideOfLibraryCause =
-    const Code<Message Function(String name, String name2)>(
-        "BaseClassImplementedOutsideOfLibraryCause",
-        severity: Severity.context);
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-Message _withArgumentsBaseClassImplementedOutsideOfLibraryCause(
-    String name, String name2) {
-  if (name.isEmpty) throw 'No name provided';
-  name = demangleMixinApplicationName(name);
-  if (name2.isEmpty) throw 'No name provided';
-  name2 = demangleMixinApplicationName(name2);
-  return new Message(codeBaseClassImplementedOutsideOfLibraryCause,
-      problemMessage:
-          """The type '${name}' is a subtype of '${name2}', and '${name2}' is defined here.""",
-      arguments: {'name': name, 'name2': name2});
-}
-
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeBaseEnum = messageBaseEnum;
 
@@ -431,6 +403,35 @@ Message _withArgumentsBaseMixinImplementedOutsideOfLibrary(String name) {
       arguments: {'name': name});
 }
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<Message Function(String name, String name2)>
+    templateBaseOrFinalClassImplementedOutsideOfLibraryCause =
+    const Template<Message Function(String name, String name2)>(
+        problemMessageTemplate:
+            r"""The type '#name' is a subtype of '#name2', and '#name2' is defined here.""",
+        withArguments:
+            _withArgumentsBaseOrFinalClassImplementedOutsideOfLibraryCause);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name, String name2)>
+    codeBaseOrFinalClassImplementedOutsideOfLibraryCause =
+    const Code<Message Function(String name, String name2)>(
+        "BaseOrFinalClassImplementedOutsideOfLibraryCause",
+        severity: Severity.context);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsBaseOrFinalClassImplementedOutsideOfLibraryCause(
+    String name, String name2) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  if (name2.isEmpty) throw 'No name provided';
+  name2 = demangleMixinApplicationName(name2);
+  return new Message(codeBaseOrFinalClassImplementedOutsideOfLibraryCause,
+      problemMessage:
+          """The type '${name}' is a subtype of '${name2}', and '${name2}' is defined here.""",
+      arguments: {'name': name, 'name2': name2});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<
     Message Function(

pkg/_fe_analyzer_shared/test/inheritance/data/function.dart

@@ -16,7 +16,8 @@ class B
 /*cfe|cfe:builder.class: C:C,Object,_C&Object&Function*/
 /*analyzer.class: C:C,Object*/
 /*cfe|cfe:builder.class: _C&Object&Function:Object,_C&Object&Function*/
-class C extends Object
+class /*cfe|cfe:builder.error: SubtypeOfFinalIsNotBaseFinalOrSealed*/ C
+    extends Object
     with /*analyzer.error: CompileTimeErrorCode.CLASS_USED_AS_MIXIN*/
         /*cfe|cfe:builder.error: CantUseClassAsMixin*/ Function {}
 

pkg/front_end/lib/src/fasta/source/source_loader.dart

@@ -2301,13 +2301,38 @@ severity: $severity
         if (!superclass.isBase &&
             !superclass.isFinal &&
             !superclass.isSealed &&
-            !superclass.cls.isAnonymousMixin) {
+            !superclass.cls.isAnonymousMixin &&
+            superclass.libraryBuilder.library.languageVersion >=
+                ExperimentalFlag.classModifiers.experimentEnabledVersion) {
           // Only report an error on the nearest subtype that does not fulfill
           // the base or final subtype restriction.
           return;
         }
 
         if (baseOrFinalSuperClass.isFinal) {
+          // Don't check base and final subtyping restriction if the supertype
+          // is a final type used outside of its library.
+          if (cls.libraryBuilder.origin !=
+              baseOrFinalSuperClass.libraryBuilder.origin) {
+            // In the special case where the 'baseOrFinalSuperClass' is a core
+            // library class and we are indirectly subtyping from a superclass
+            // that's from a pre-feature library, we want to produce a final
+            // transitivity error.
+            //
+            // For implements clauses with the above scenario, we avoid
+            // over-reporting since there will already be a
+            // [FinalClassImplementedOutsideOfLibrary] error.
+            //
+            // TODO(kallentu): Avoid over-reporting for with clauses.
+            if (baseOrFinalSuperClass.libraryBuilder.origin ==
+                    superclass.libraryBuilder.origin ||
+                !baseOrFinalSuperClass.libraryBuilder.importUri
+                    .isScheme("dart") ||
+                implementsBuilder != null) {
+              return;
+            }
+          }
+
           cls.addProblem(
               templateSubtypeOfFinalIsNotBaseFinalOrSealed.withArguments(
                   cls.fullNameForErrors,
@@ -2330,15 +2355,9 @@ severity: $severity
       final TypeDeclarationBuilder? supertypeDeclaration =
           unaliasDeclaration(supertypeBuilder);
       if (supertypeDeclaration is ClassBuilder) {
-        if (isClassModifiersEnabled(supertypeDeclaration)) {
-          if (cls.libraryBuilder.origin ==
-                  supertypeDeclaration.libraryBuilder.origin ||
-              !supertypeDeclaration.isFinal) {
-            // Don't check base and final subtyping restriction if the supertype
-            // is a final type used outside of its library.
-            checkForBaseFinalRestriction(supertypeDeclaration);
-          }
+        checkForBaseFinalRestriction(supertypeDeclaration);
 
+        if (isClassModifiersEnabled(supertypeDeclaration)) {
           if (cls.libraryBuilder.origin !=
                   supertypeDeclaration.libraryBuilder.origin &&
               !mayIgnoreClassModifiers(supertypeDeclaration)) {
@@ -2385,15 +2404,9 @@ severity: $severity
       final TypeDeclarationBuilder? mixedInTypeDeclaration =
           unaliasDeclaration(mixedInTypeBuilder);
       if (mixedInTypeDeclaration is ClassBuilder) {
-        if (isClassModifiersEnabled(mixedInTypeDeclaration)) {
-          if (cls.libraryBuilder.origin ==
-                  mixedInTypeDeclaration.libraryBuilder.origin ||
-              !mixedInTypeDeclaration.isFinal) {
-            // Don't check base and final subtyping restriction if the supertype
-            // is a final type used outside of its library.
-            checkForBaseFinalRestriction(mixedInTypeDeclaration);
-          }
+        checkForBaseFinalRestriction(mixedInTypeDeclaration);
 
+        if (isClassModifiersEnabled(mixedInTypeDeclaration)) {
           // Check for classes being used as mixins. Only classes declared with
           // a 'mixin' modifier are allowed to be mixed in.
           if (cls.isMixinApplication &&
@@ -2428,79 +2441,55 @@ severity: $severity
         final TypeDeclarationBuilder? interfaceDeclaration =
             unaliasDeclaration(interfaceBuilder);
         if (interfaceDeclaration is ClassBuilder) {
-          if (isClassModifiersEnabled(interfaceDeclaration)) {
-            if (cls.libraryBuilder.origin ==
-                    interfaceDeclaration.libraryBuilder.origin ||
-                !interfaceDeclaration.isFinal) {
-              // Don't check base and final subtyping restriction if the
-              // supertype is a final type used outside of its library.
-              checkForBaseFinalRestriction(interfaceDeclaration,
-                  implementsBuilder: interfaceBuilder);
-            }
+          checkForBaseFinalRestriction(interfaceDeclaration,
+              implementsBuilder: interfaceBuilder);
+
+          ClassBuilder? checkedClass = interfaceDeclaration;
+          while (checkedClass != null) {
+            if (cls.libraryBuilder.origin !=
+                    checkedClass.libraryBuilder.origin &&
+                !mayIgnoreClassModifiers(checkedClass)) {
+              final List<LocatedMessage> context = [
+                if (checkedClass != interfaceDeclaration)
+                  templateBaseOrFinalClassImplementedOutsideOfLibraryCause
+                      .withArguments(interfaceDeclaration.fullNameForErrors,
+                          checkedClass.fullNameForErrors)
+                      .withLocation(checkedClass.fileUri,
+                          checkedClass.charOffset, noLength)
+              ];
 
-            ClassBuilder? checkedClass = interfaceDeclaration;
-            while (checkedClass != null) {
-              if (cls.libraryBuilder.origin !=
-                      checkedClass.libraryBuilder.origin &&
-                  !mayIgnoreClassModifiers(checkedClass)) {
+              if (checkedClass.isBase && !cls.cls.isAnonymousMixin) {
                 // Report an error for a class implementing a base class outside
                 // of its library.
-                if (checkedClass.isBase && !cls.cls.isAnonymousMixin) {
-                  if (checkedClass.isMixinDeclaration) {
-                    cls.addProblem(
-                        templateBaseMixinImplementedOutsideOfLibrary
-                            .withArguments(checkedClass.fullNameForErrors),
-                        interfaceBuilder.charOffset ?? TreeNode.noOffset,
-                        noLength,
-                        context: [
-                          if (checkedClass != interfaceDeclaration)
-                            templateBaseClassImplementedOutsideOfLibraryCause
-                                .withArguments(
-                                    interfaceDeclaration.fullNameForErrors,
-                                    checkedClass.fullNameForErrors)
-                                .withLocation(checkedClass.fileUri,
-                                    checkedClass.charOffset, noLength)
-                        ]);
-                  } else {
-                    cls.addProblem(
-                        templateBaseClassImplementedOutsideOfLibrary
-                            .withArguments(checkedClass.fullNameForErrors),
-                        interfaceBuilder.charOffset ?? TreeNode.noOffset,
-                        noLength,
-                        context: [
-                          if (checkedClass != interfaceDeclaration)
-                            templateBaseClassImplementedOutsideOfLibraryCause
-                                .withArguments(
-                                    interfaceDeclaration.fullNameForErrors,
-                                    checkedClass.fullNameForErrors)
-                                .withLocation(checkedClass.fileUri,
-                                    checkedClass.charOffset, noLength)
-                        ]);
-                  }
-                  // Break to only report one error.
-                  break;
-                } else if (checkedClass.isFinal &&
-                    checkedClass == interfaceDeclaration) {
-                  if (cls.cls.isAnonymousMixin) {
-                    cls.addProblem(
-                        templateFinalClassUsedAsMixinConstraintOutsideOfLibrary
-                            .withArguments(
-                                interfaceDeclaration.fullNameForErrors),
-                        interfaceBuilder.charOffset ?? TreeNode.noOffset,
-                        noLength);
-                  } else {
-                    cls.addProblem(
-                        templateFinalClassImplementedOutsideOfLibrary
-                            .withArguments(
-                                interfaceDeclaration.fullNameForErrors),
-                        interfaceBuilder.charOffset ?? TreeNode.noOffset,
-                        noLength);
-                  }
-                  break;
-                }
+                final Template<Message Function(String)> template =
+                    checkedClass.isMixinDeclaration
+                        ? templateBaseMixinImplementedOutsideOfLibrary
+                        : templateBaseClassImplementedOutsideOfLibrary;
+                cls.addProblem(
+                    template.withArguments(checkedClass.fullNameForErrors),
+                    interfaceBuilder.charOffset ?? TreeNode.noOffset,
+                    noLength,
+                    context: context);
+                // Break to only report one error.
+                break;
+              } else if (checkedClass.isFinal) {
+                // Report an error for a class implementing a final class
+                // outside of its library.
+                final Template<Message Function(String)> template = cls
+                            .cls.isAnonymousMixin &&
+                        checkedClass == interfaceDeclaration
+                    ? templateFinalClassUsedAsMixinConstraintOutsideOfLibrary
+                    : templateFinalClassImplementedOutsideOfLibrary;
+                cls.addProblem(
+                    template.withArguments(checkedClass.fullNameForErrors),
+                    interfaceBuilder.charOffset ?? TreeNode.noOffset,
+                    noLength,
+                    context: context);
+                // Break to only report one error.
+                break;
               }
-              checkedClass = classToBaseOrFinalSuperClass[checkedClass];
             }
+            checkedClass = classToBaseOrFinalSuperClass[checkedClass];
           }
 
           // Report error for implementing a sealed class or a sealed mixin

pkg/front_end/messages.yaml

@@ -6355,7 +6355,7 @@ BaseClassImplementedOutsideOfLibrary:
     lib.dart:
       base class A {}
 
-BaseClassImplementedOutsideOfLibraryCause:
+BaseOrFinalClassImplementedOutsideOfLibraryCause:
   problemMessage: "The type '#name' is a subtype of '#name2', and '#name2' is defined here."
   severity: CONTEXT
 

pkg/front_end/testcases/class_modifiers/issue52115/main.dart

@@ -0,0 +1,17 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. 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:collection';
+import "main_lib.dart";
+
+// Implementing a legacy class that implements a core library base class.
+abstract base class LegacyImplementBase<E extends LinkedListEntry<E>>
+    implements LegacyImplementBaseCore<E> {}
+
+// Implementing a legacy class that implements a core library final class.
+final class LegacyImplementFinal implements LegacyImplementFinalCore {
+  int get key => 0;
+  int get value => 1;
+  String toString() => "Bad";
+}

pkg/front_end/testcases/class_modifiers/issue52115/main.dart.strong.expect

@@ -0,0 +1,66 @@
+library /*isNonNullableByDefault*/;
+//
+// Problems in library:
+//
+// pkg/front_end/testcases/class_modifiers/issue52115/main.dart:10:16: Error: The class 'LinkedList' can't be implemented outside of its library because it's a base class.
+//     implements LegacyImplementBaseCore<E> {}
+//                ^
+// sdk/lib/collection/linked_list.dart:81:12: Context: The type 'LegacyImplementBaseCore' is a subtype of 'LinkedList', and 'LinkedList' is defined here.
+// base class LinkedList<E extends LinkedListEntry<E>> extends Iterable<E> {
+//            ^
+//
+// pkg/front_end/testcases/class_modifiers/issue52115/main.dart:13:45: Error: The class 'MapEntry' can't be implemented outside of its library because it's a final class.
+// final class LegacyImplementFinal implements LegacyImplementFinalCore {
+//                                             ^
+// sdk/lib/core/map.dart:472:13: Context: The type 'LegacyImplementFinalCore' is a subtype of 'MapEntry', and 'MapEntry' is defined here.
+// final class MapEntry<K, V> {
+//             ^
+//
+import self as self;
+import "dart:collection" as col;
+import "dart:core" as core;
+import "main_lib.dart" as mai;
+
+import "dart:collection";
+import "org-dartlang-testcase:///main_lib.dart";
+
+abstract base class LegacyImplementBase<E extends col::LinkedListEntry<self::LegacyImplementBase::E> = col::LinkedListEntry<dynamic>> extends core::Object implements mai::LegacyImplementBaseCore<self::LegacyImplementBase::E> {
+  synthetic constructor •() → self::LegacyImplementBase<self::LegacyImplementBase::E>
+    : super core::Object::•()
+    ;
+}
+final class LegacyImplementFinal extends core::Object implements mai::LegacyImplementFinalCore {
+  synthetic constructor •() → self::LegacyImplementFinal
+    : super core::Object::•()
+    ;
+  get key() → core::int
+    return 0;
+  get value() → core::int
+    return 1;
+  method toString() → core::String
+    return "Bad";
+}
+
+library /*isNonNullableByDefault*/;
+import self as mai;
+import "dart:collection" as col;
+import "dart:core" as core;
+
+import "dart:collection";
+
+abstract class LegacyImplementBaseCore<E extends col::LinkedListEntry<mai::LegacyImplementBaseCore::E> = col::LinkedListEntry<dynamic>> extends core::Object implements col::LinkedList<mai::LegacyImplementBaseCore::E> {
+  synthetic constructor •() → mai::LegacyImplementBaseCore<mai::LegacyImplementBaseCore::E>
+    : super core::Object::•()
+    ;
+}
+class LegacyImplementFinalCore extends core::Object implements core::MapEntry<core::int, core::int> {
+  synthetic constructor •() → mai::LegacyImplementFinalCore
+    : super core::Object::•()
+    ;
+  get key() → core::int
+    return 0;
+  get value() → core::int
+    return 1;
+  method toString() → core::String
+    return "Bad";
+}