[analyzer] Report an error when implementing a base class through a sealed class.
Bug: https://github.com/dart-lang/sdk/issues/51810
Change-Id: I478eff8484fa5d3bddfb9cf1c744e6d65a72f6c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290400
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
diff --git a/pkg/analyzer/lib/src/error/base_or_final_type_verifier.dart b/pkg/analyzer/lib/src/error/base_or_final_type_verifier.dart
index ae00190..ae6cd9e 100644
--- a/pkg/analyzer/lib/src/error/base_or_final_type_verifier.dart
+++ b/pkg/analyzer/lib/src/error/base_or_final_type_verifier.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/diagnostic/diagnostic.dart';
@@ -24,16 +25,21 @@
_errorReporter = errorReporter;
/// Check to ensure the subelement of a base or final element must be base,
- /// final, or sealed. Otherwise, an error is reported on that element.
+ /// final, or sealed and that base elements are not implemented outside of its
+ /// library. Otherwise, an error is reported on that element.
///
/// See [CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED],
- /// [CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED]
- void checkElement(ClassOrMixinElementImpl element) {
+ /// [CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED],
+ /// [CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY].
+ void checkElement(
+ ClassOrMixinElementImpl element, ImplementsClause? implementsClause) {
final supertype = element.supertype;
if (supertype != null && _checkSupertypes([supertype], element)) {
return;
}
- if (_checkSupertypes(element.interfaces, element)) {
+ if (implementsClause != null &&
+ _checkInterfaceSupertypes(implementsClause.interfaces, element,
+ areImplementedInterfaces: true)) {
return;
}
if (_checkSupertypes(element.mixins, element)) {
@@ -47,6 +53,28 @@
}
/// Returns true if a 'base' or 'final' subtype modifier error is reported for
+ /// an interface in [interfaces].
+ bool _checkInterfaceSupertypes(
+ List<NamedType> interfaces, ClassOrMixinElementImpl subElement,
+ {bool areImplementedInterfaces = false}) {
+ for (NamedType interface in interfaces) {
+ final interfaceType = interface.type;
+ if (interfaceType is InterfaceType) {
+ final interfaceElement = interfaceType.element;
+ if (interfaceElement is ClassOrMixinElementImpl) {
+ // Return early if an error has been reported to prevent reporting
+ // multiple errors on one element.
+ if (_reportRestrictionError(subElement, interfaceElement,
+ implementsNamedType: interface)) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
+
+ /// Returns true if a 'base' or 'final' subtype modifier error is reported for
/// a supertype in [supertypes].
bool _checkSupertypes(
List<InterfaceType> supertypes, ClassOrMixinElementImpl subElement,
@@ -135,7 +163,7 @@
/// Reports an error based on the modifier of the [superElement].
bool _reportRestrictionError(
ClassOrMixinElementImpl element, ClassOrMixinElementImpl superElement,
- {bool isSuperclassConstraint = false}) {
+ {bool isSuperclassConstraint = false, NamedType? implementsNamedType}) {
ClassOrMixinElementImpl? baseOrFinalSuperElement;
if (superElement.isBase || superElement.isFinal) {
// The 'base' or 'final' modifier may be an induced modifier. Find the
@@ -151,9 +179,6 @@
return false;
}
if (baseOrFinalSuperElement != null &&
- !element.isBase &&
- !element.isFinal &&
- !element.isSealed &&
!_mayIgnoreClassModifiers(baseOrFinalSuperElement.library)) {
// The context message links to the explicitly declared 'base' or 'final'
// super element and is only added onto the error if 'base' or 'final' is
@@ -170,27 +195,44 @@
)
];
- if (baseOrFinalSuperElement.isFinal) {
- if (!isSuperclassConstraint &&
- baseOrFinalSuperElement.library != element.library) {
- // If you can't extend, implement or mix in a final element outside of
- // its library anyways, it's not helpful to report a subelement
- // modifier error.
- return false;
+ // It's an error to implement a class if it has a supertype from a
+ // different library which is marked base.
+ if (implementsNamedType != null &&
+ superElement.isSealed &&
+ baseOrFinalSuperElement.library != element.library) {
+ if (baseOrFinalSuperElement.isBase) {
+ _errorReporter.reportErrorForNode(
+ CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY,
+ implementsNamedType,
+ [baseOrFinalSuperElement.displayName],
+ contextMessage);
+ return true;
}
- _errorReporter.reportErrorForElement(
- CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED,
- element,
- [element.displayName, baseOrFinalSuperElement.displayName],
- superElement.isSealed ? contextMessage : null);
- return true;
- } else if (baseOrFinalSuperElement.isBase) {
- _errorReporter.reportErrorForElement(
- CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED,
- element,
- [element.displayName, baseOrFinalSuperElement.displayName],
- superElement.isSealed ? contextMessage : null);
- return true;
+ }
+
+ if (!element.isBase && !element.isFinal && !element.isSealed) {
+ if (baseOrFinalSuperElement.isFinal) {
+ if (!isSuperclassConstraint &&
+ baseOrFinalSuperElement.library != element.library) {
+ // If you can't extend, implement or mix in a final element outside of
+ // its library anyways, it's not helpful to report a subelement
+ // modifier error.
+ return false;
+ }
+ _errorReporter.reportErrorForElement(
+ CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED,
+ element,
+ [element.displayName, baseOrFinalSuperElement.displayName],
+ superElement.isSealed ? contextMessage : null);
+ return true;
+ } else if (baseOrFinalSuperElement.isBase) {
+ _errorReporter.reportErrorForElement(
+ CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED,
+ element,
+ [element.displayName, baseOrFinalSuperElement.displayName],
+ superElement.isSealed ? contextMessage : null);
+ return true;
+ }
}
}
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 680039c..4f1ffad 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -2059,8 +2059,8 @@
enclosingClass = outerType;
}
- baseOrFinalTypeVerifier
- .checkElement(node.declaredElement as ClassOrMixinElementImpl);
+ baseOrFinalTypeVerifier.checkElement(
+ node.declaredElement as ClassOrMixinElementImpl, node.implementsClause);
}
@override
@@ -2068,8 +2068,8 @@
checkUnreachableNode(node);
node.visitChildren(this);
elementResolver.visitClassTypeAlias(node);
- baseOrFinalTypeVerifier
- .checkElement(node.declaredElement as ClassOrMixinElementImpl);
+ baseOrFinalTypeVerifier.checkElement(
+ node.declaredElement as ClassOrMixinElementImpl, node.implementsClause);
}
@override
@@ -2989,8 +2989,8 @@
enclosingClass = outerType;
}
- baseOrFinalTypeVerifier
- .checkElement(node.declaredElement as ClassOrMixinElementImpl);
+ baseOrFinalTypeVerifier.checkElement(
+ node.declaredElement as ClassOrMixinElementImpl, node.implementsClause);
}
@override
diff --git a/pkg/analyzer/test/src/diagnostics/base_class_implemented_outside_of_library_test.dart b/pkg/analyzer/test/src/diagnostics/base_class_implemented_outside_of_library_test.dart
index 615b25b..4dbd49d 100644
--- a/pkg/analyzer/test/src/diagnostics/base_class_implemented_outside_of_library_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/base_class_implemented_outside_of_library_test.dart
@@ -5,6 +5,7 @@
import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
+import '../../generated/test_support.dart';
import '../dart/resolution/context_collection_resolution.dart';
main() {
@@ -37,6 +38,58 @@
]);
}
+ test_class_outside_sealed() async {
+ final a = newFile('$testPackageLibPath/a.dart', r'''
+base class A {}
+''');
+
+ await assertErrorsInCode(r'''
+import 'a.dart';
+sealed class B extends A {}
+base class C implements B {}
+''', [
+ error(
+ CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY,
+ 69,
+ 1,
+ text:
+ "The class 'A' can't be implemented outside of its library because it's a base class.",
+ contextMessages: [
+ ExpectedContextMessage(a.path, 11, 1,
+ text:
+ "The type 'B' is a subtype of 'A', and 'A' is defined here.")
+ ],
+ ),
+ ]);
+ }
+
+ test_class_outside_sealed_noBase() async {
+ // Instead of emitting [SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED], we
+ // tell the user that they can't implement an indirect base supertype.
+ final a = newFile('$testPackageLibPath/a.dart', r'''
+base class A {}
+''');
+
+ await assertErrorsInCode(r'''
+import 'a.dart';
+sealed class B extends A {}
+class C implements B {}
+''', [
+ error(
+ CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY,
+ 64,
+ 1,
+ text:
+ "The class 'A' can't be implemented outside of its library because it's a base class.",
+ contextMessages: [
+ ExpectedContextMessage(a.path, 11, 1,
+ text:
+ "The type 'B' is a subtype of 'A', and 'A' is defined here.")
+ ],
+ ),
+ ]);
+ }
+
test_class_outside_viaTypedef_inside() async {
newFile('$testPackageLibPath/foo.dart', r'''
base class Foo {}
@@ -67,6 +120,41 @@
]);
}
+ test_classTypeAlias_inside() async {
+ await assertNoErrorsInCode(r'''
+base class A {}
+sealed class B extends A {}
+mixin M {}
+base class C = Object with M implements B;
+''');
+ }
+
+ test_classTypeAlias_outside() async {
+ final a = newFile('$testPackageLibPath/a.dart', r'''
+base class A {}
+''');
+
+ await assertErrorsInCode(r'''
+import 'a.dart';
+sealed class B extends A {}
+mixin M {}
+base class C = Object with M implements B;
+''', [
+ error(
+ CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY,
+ 96,
+ 1,
+ text:
+ "The class 'A' can't be implemented outside of its library because it's a base class.",
+ contextMessages: [
+ ExpectedContextMessage(a.path, 11, 1,
+ text:
+ "The type 'B' is a subtype of 'A', and 'A' is defined here.")
+ ],
+ ),
+ ]);
+ }
+
test_enum_inside() async {
await assertNoErrorsInCode(r'''
base class Foo {}
diff --git a/tests/language/class_modifiers/base_transitivity/base_class_different_library_error_test.dart b/tests/language/class_modifiers/base_transitivity/base_class_different_library_error_test.dart
index 9360566..f3ef842 100644
--- a/tests/language/class_modifiers/base_transitivity/base_class_different_library_error_test.dart
+++ b/tests/language/class_modifiers/base_transitivity/base_class_different_library_error_test.dart
@@ -40,48 +40,52 @@
// Implementing through a sealed class.
class SimpleSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
+// ^
// [cfe] The type 'SimpleSealedExtendImplement' must be 'base', 'final' or 'sealed' because the supertype 'BaseClass' is 'base'.
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
base class BaseSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
// [cfe] unspecified
interface class InterfaceSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
+// ^
// [cfe] The type 'InterfaceSealedExtendImplement' must be 'base', 'final' or 'sealed' because the supertype 'BaseClass' is 'base'.
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
final class FinalSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
// [cfe] unspecified
sealed class SealedSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
// [cfe] unspecified
mixin class MixinClassSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
+// ^
// [cfe] The type 'MixinClassSealedExtendImplement' must be 'base', 'final' or 'sealed' because the supertype 'BaseClass' is 'base'.
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
base mixin class BaseMixinClassSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
// [cfe] unspecified
mixin MixinSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
+// ^
// [cfe] The type 'MixinSealedExtendImplement' must be 'base', 'final' or 'sealed' because the supertype 'BaseClass' is 'base'.
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
base mixin BaseMixinSealedExtendImplement implements SealedExtend {}
-// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-// [analyzer] unspecified
+// ^^^^^^^^^^^^
+// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
// [cfe] unspecified
// Using a sealed class as an `on` type
