[analyzer] Disallow implementing a legacy library subclass of a final…

… class. Checks an interface's supertypes for final classes and produces an error if the implementing bypasses a legacy library. This behaviour should only happen when a post-feature library implements a pre-feature library declaration that has a final class as a super declaration. Similar error to https://dart-review.googlesource.com/c/sdk/+/298320 Bug: #52078 Change-Id: Ie16edb2b231957dad7502fdab3d5faba93bc6773 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300861 Commit-Queue: Kallen Tu <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
dart-lang · May 8, 2023 · 6de1afd · 6de1afd
1 parent 5a046ce
commit 6de1afd
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 60 deletions.
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -2893,52 +2893,51 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     ImplementsClause? implementsClause,
     OnClause? onClause,
   ) {
-    List<Element> elementsToCheck(InterfaceType type) {
-      final element = type.element;
-      if (element.library.featureSet.isEnabled(Feature.class_modifiers)) {
-        return [element];
-      } else {
-        return [
-          element,
-          ...element.allSupertypes.map((e) => e.element),
-        ];
-      }
-    }
-
     if (superclass != null) {
       final type = superclass.type;
       if (type is InterfaceType) {
-        final elements = elementsToCheck(type);
-        for (final element in elements) {
-          if (element is ClassElementImpl &&
-              element.isFinal &&
-              !element.isSealed &&
-              element.library != _currentLibrary &&
-              !_mayIgnoreClassModifiers(element.library)) {
-            errorReporter.reportErrorForNode(
-                CompileTimeErrorCode.FINAL_CLASS_EXTENDED_OUTSIDE_OF_LIBRARY,
-                superclass,
-                [element.name]);
-          }
+        final element = type.element;
+        if (element is ClassElementImpl &&
+            element.isFinal &&
+            !element.isSealed &&
+            element.library != _currentLibrary &&
+            !_mayIgnoreClassModifiers(element.library)) {
+          errorReporter.reportErrorForNode(
+              CompileTimeErrorCode.FINAL_CLASS_EXTENDED_OUTSIDE_OF_LIBRARY,
+              superclass,
+              [element.name]);
         }
       }
     }
     if (implementsClause != null) {
       for (NamedType namedType in implementsClause.interfaces) {
         final type = namedType.type;
         if (type is InterfaceType) {
-          final elements = elementsToCheck(type);
-          for (final element in elements) {
+          final implementedInterfaces = [
+            type,
+            ...type.element.allSupertypes,
+          ].map((e) => e.element).toList();
+          for (final element in implementedInterfaces) {
             if (element is ClassElement &&
                 element.isFinal &&
                 !element.isSealed &&
                 element.library != _currentLibrary &&
                 !_mayIgnoreClassModifiers(element.library)) {
+              // If the final interface is an indirect interface and is in a
+              // different library that has class modifiers enabled, there is a
+              // nearer declaration that would emit an error, if any.
+              if (element != type.element &&
+                  type.element.library.featureSet
+                      .isEnabled(Feature.class_modifiers)) {
+                continue;
+              }
+
               errorReporter.reportErrorForNode(
                   CompileTimeErrorCode
                       .FINAL_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY,
                   namedType,
                   [element.name]);
+              break;
             }
           }
         }

diff --git a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
@@ -539,7 +539,7 @@ final class Null extends Object {
   }
 }
 
-class MapEntry<K, V> {
+final class MapEntry<K, V> {
   final K key;
   final V value;
   const factory MapEntry(K key, V value) = MapEntry<K, V>._;

diff --git a/pkg/analyzer/test/src/diagnostics/final_class_extended_outside_of_library_test.dart b/pkg/analyzer/test/src/diagnostics/final_class_extended_outside_of_library_test.dart
@@ -36,24 +36,28 @@ final class Bar extends Foo {}
     ]);
   }
 
-  test_outside_viaLanguage219() async {
-    newFile('$testPackageLibPath/a.dart', r'''
-final class A {}
+  test_outside_viaLanguage219AndCore() async {
+    // There is no error when extending a pre-feature class that subtypes a
+    // class in the core libraries.
+    final a = newFile('$testPackageLibPath/a.dart', r'''
+// @dart=2.19
+import 'dart:core';
+class A implements MapEntry<int, int> {
+  int get key => 0;
+  int get value => 1;
+}
 ''');
 
-    newFile('$testPackageLibPath/b.dart', r'''
-// @dart = 2.19
+    await resolveFile2(a.path);
+    assertNoErrorsInResult();
+
+    await assertNoErrorsInCode(r'''
 import 'a.dart';
-class B extends A {}
+final class B extends A {
+  int get key => 0;
+  int get value => 1;
+}
 ''');
-
-    await assertErrorsInCode(r'''
-import 'b.dart';
-final class C extends B {}
-''', [
-      error(
-          CompileTimeErrorCode.FINAL_CLASS_EXTENDED_OUTSIDE_OF_LIBRARY, 39, 1),
-    ]);
   }
 
   test_outside_viaTypedef_inside() async {

diff --git a/pkg/analyzer/test/src/diagnostics/final_class_implemented_outside_of_library_test.dart b/pkg/analyzer/test/src/diagnostics/final_class_implemented_outside_of_library_test.dart
@@ -37,21 +37,25 @@ final class Bar implements Foo {}
     ]);
   }
 
-  test_class_outside_viaLanguage219() async {
-    int;
-    newFile('$testPackageLibPath/a.dart', r'''
-final class A {}
+  test_class_outside_viaLanguage219AndCore() async {
+    final a = newFile('$testPackageLibPath/a.dart', r'''
+// @dart=2.19
+import 'dart:core';
+class A implements MapEntry<int, int> {
+  int get key => 0;
+  int get value => 1;
+}
 ''');
 
-    newFile('$testPackageLibPath/b.dart', r'''
-// @dart = 2.19
-import 'a.dart';
-class B implements A {}
-''');
+    await resolveFile2(a.path);
+    assertNoErrorsInResult();
 
     await assertErrorsInCode(r'''
-import 'b.dart';
-final class C implements B {}
+import 'a.dart';
+final class B implements A {
+  int get key => 0;
+  int get value => 1;
+}
 ''', [
       error(CompileTimeErrorCode.FINAL_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY, 42,
           1),

diff --git a/.../language/class_modifiers/base_transitivity/final_class_different_library_error_test.dart b/.../language/class_modifiers/base_transitivity/final_class_different_library_error_test.dart
@@ -189,7 +189,7 @@ base mixin BaseMixinImplement implements FinalClass {}
 
 class LegacyImplement implements LegacyImplementFinalCore {
 //                               ^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
   int get key => 0;
   int get value => 1;

diff --git a/tests/language/class_modifiers/trans_legacy/legacy_superdeclaration_error_test.dart b/tests/language/class_modifiers/trans_legacy/legacy_superdeclaration_error_test.dart
@@ -12,33 +12,33 @@ import "legacy_lib.dart";
 abstract base class ImplementsLegacyImplementsFinal
     implements LegacyImplementsFinal {
 //             ^^^^^^^^^^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
-// [analyzer] unspecified
 }
 
 abstract base class ImplementsLegacyExtendsFinal implements LegacyExtendsFinal {
 //                                                          ^^^^^^^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
-// [analyzer] unspecified
 }
 
 abstract class ImplementsLegacyMixesInFinal implements LegacyMixesInFinal {
 //                                                     ^^^^^^^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
-// [analyzer] unspecified
 }
 
 abstract base class ImplementsLegacyImplementsBase
     implements LegacyImplementsBase {
-//             ^^^^^^^^^^^^^^^^^^^^^
+//             ^^^^^^^^^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
-// [analyzer] unspecified
 }
 
 abstract base class ImplementsLegacyMixinOnFinal implements LegacyMixinOnFinal {
 //                                                          ^^^^^^^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
 // [cfe] unspecified
-// [analyzer] unspecified
 }
 
 // Not allowed to omit base on classes with base/final superclasses.
@@ -175,4 +175,4 @@ abstract class MixesInMixinImplementsBase2 = Object
 // Helpers.
 mixin _AnyMixin {}
 
-void main() {}
+void main() {}