Skip to content

Commit 9c34ff7

Browse files
kallentuCommit Queue
kallentu
authored and
Commit Queue
committed
[analyzer] Produce error when omitting base/final/sealed on base/final superclasses through legacy libraries.
When a post-feature library implements a pre-feature library declaration that has a final core library class as a super declaration, it should not produce a base/final/sealed transitivity error. Subclasses of a base core library class will still emit this error. Otherwise, if base/sealed/final is omitted in a non-implement case, we want to report these errors. Bug: #52315 Change-Id: Icdd4f63f69b061be5eabc7fd30b703d0358018a7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302365 Commit-Queue: Kallen Tu <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent 73dea24 commit 9c34ff7

File tree

6 files changed

+208
-29
lines changed

6 files changed

+208
-29
lines changed

‎pkg/analyzer/lib/src/error/base_or_final_type_verifier.dart

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,14 @@ class BaseOrFinalTypeVerifier {
162162
ClassOrMixinElementImpl element, ClassOrMixinElementImpl superElement,
163163
{NamedType? implementsNamedType}) {
164164
ClassOrMixinElementImpl? baseOrFinalSuperElement;
165-
if (superElement.isBase || superElement.isFinal) {
165+
if (superElement.isBase ||
166+
superElement.isFinal ||
167+
(!superElement.library.featureSet.isEnabled(Feature.class_modifiers) &&
168+
element.library.featureSet.isEnabled(Feature.class_modifiers))) {
166169
// The 'base' or 'final' modifier may be an induced modifier. Find the
167170
// explicitly declared 'base' or 'final' in the hierarchy.
171+
// In the case where the super element is in a pre-feature library, we
172+
// need to check if there's an indirect core library super element.
168173
baseOrFinalSuperElement = _getExplicitlyBaseOrFinalElement(superElement);
169174
} else {
170175
// There are no restrictions on this element's modifiers.
@@ -213,11 +218,24 @@ class BaseOrFinalTypeVerifier {
213218

214219
if (!element.isBase && !element.isFinal && !element.isSealed) {
215220
if (baseOrFinalSuperElement.isFinal) {
221+
// If you can't extend, implement or mix in a final element outside of
222+
// its library anyways, it's not helpful to report a subelement
223+
// modifier error.
216224
if (baseOrFinalSuperElement.library != element.library) {
217-
// If you can't extend, implement or mix in a final element outside of
218-
// its library anyways, it's not helpful to report a subelement
219-
// modifier error.
220-
return false;
225+
// In the case where the 'baseOrFinalSuperElement' is a core
226+
// library element and we are subtyping from a super element that's
227+
// from a pre-feature library, we want to produce a final
228+
// transitivity error.
229+
//
230+
// For implements clauses with the above scenario, we avoid
231+
// over-reporting since there will already be a
232+
// [FinalClassImplementedOutsideOfLibrary] error.
233+
if (superElement.library.featureSet
234+
.isEnabled(Feature.class_modifiers) ||
235+
!baseOrFinalSuperElement.library.isInSdk ||
236+
implementsNamedType != null) {
237+
return false;
238+
}
221239
}
222240
_errorReporter.reportErrorForElement(
223241
CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED,

‎pkg/analyzer/lib/src/test_utilities/mock_sdk.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ abstract class LinkedHashSet<E> implements Set<E> {
218218
}
219219
}
220220
221+
abstract base mixin class LinkedListEntry<E extends LinkedListEntry<E>> { }
222+
221223
abstract mixin class ListMixin<E> implements List<E> { }
222224
223225
abstract mixin class MapMixin<K, V> implements Map<K, V> { }

‎pkg/analyzer/test/src/diagnostics/subtype_of_base_is_not_base_final_or_sealed_test.dart

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,27 @@ class B extends A {}
5757
]);
5858
}
5959

60+
test_class_extends_outside_viaLanguage219AndCore() async {
61+
final a = newFile('$testPackageLibPath/a.dart', r'''
62+
// @dart=2.19
63+
import 'dart:collection';
64+
abstract class A implements LinkedListEntry<Never> {}
65+
''');
66+
67+
await resolveFile2(a.path);
68+
assertNoErrorsInResult();
69+
70+
await assertErrorsInCode(r'''
71+
import 'a.dart';
72+
abstract class B extends A {}
73+
''', [
74+
error(CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED,
75+
32, 1,
76+
text:
77+
"The type 'B' must be 'base', 'final' or 'sealed' because the supertype 'LinkedListEntry' is 'base'."),
78+
]);
79+
}
80+
6081
test_class_implements() async {
6182
await assertErrorsInCode(r'''
6283
base class A {}
@@ -81,6 +102,47 @@ mixin B implements A {}
81102
]);
82103
}
83104

105+
test_class_implements_outside() async {
106+
newFile('$testPackageLibPath/a.dart', r'''
107+
base class A {}
108+
''');
109+
110+
await assertErrorsInCode(r'''
111+
import 'a.dart';
112+
class B implements A {}
113+
''', [
114+
error(CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED,
115+
23, 1,
116+
text:
117+
"The type 'B' must be 'base', 'final' or 'sealed' because the supertype 'A' is 'base'."),
118+
error(CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY, 36,
119+
1),
120+
]);
121+
}
122+
123+
test_class_implements_outside_viaLanguage219AndCore() async {
124+
final a = newFile('$testPackageLibPath/a.dart', r'''
125+
// @dart=2.19
126+
import 'dart:collection';
127+
abstract class A implements LinkedListEntry<Never> {}
128+
''');
129+
130+
await resolveFile2(a.path);
131+
assertNoErrorsInResult();
132+
133+
await assertErrorsInCode(r'''
134+
import 'a.dart';
135+
abstract class B implements A {}
136+
''', [
137+
error(CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED,
138+
32, 1,
139+
text:
140+
"The type 'B' must be 'base', 'final' or 'sealed' because the supertype 'LinkedListEntry' is 'base'."),
141+
error(CompileTimeErrorCode.BASE_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY, 45,
142+
1),
143+
]);
144+
}
145+
84146
test_class_sealed_extends() async {
85147
await assertErrorsInCode(r'''
86148
base class A {}

‎pkg/analyzer/test/src/diagnostics/subtype_of_final_is_not_base_final_or_sealed_test.dart

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,33 @@ class B extends A {}
4545
]);
4646
}
4747

48+
test_class_extends_outside_viaLanguage219AndCore() async {
49+
final a = newFile('$testPackageLibPath/a.dart', r'''
50+
// @dart=2.19
51+
import 'dart:core';
52+
class A implements MapEntry<int, int> {
53+
int get key => 0;
54+
int get value => 1;
55+
}
56+
''');
57+
58+
await resolveFile2(a.path);
59+
assertNoErrorsInResult();
60+
61+
await assertErrorsInCode(r'''
62+
import 'a.dart';
63+
class B extends A {
64+
int get key => 0;
65+
int get value => 1;
66+
}
67+
''', [
68+
error(CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED,
69+
23, 1,
70+
text:
71+
"The type 'B' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'."),
72+
]);
73+
}
74+
4875
test_class_implements() async {
4976
await assertErrorsInCode(r'''
5077
final class A {}
@@ -69,6 +96,50 @@ mixin B implements A {}
6996
]);
7097
}
7198

99+
test_class_implements_outside() async {
100+
// No [SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED] reported outside of
101+
// library.
102+
newFile('$testPackageLibPath/a.dart', r'''
103+
final class A {}
104+
''');
105+
106+
await assertErrorsInCode(r'''
107+
import 'a.dart';
108+
class B implements A {}
109+
''', [
110+
error(CompileTimeErrorCode.FINAL_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY, 36,
111+
1),
112+
]);
113+
}
114+
115+
test_class_implements_outside_viaLanguage219AndCore() async {
116+
// No [SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED] reported outside of
117+
// library to avoid over-reporting when we have a
118+
// [FINAL_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY] error.
119+
final a = newFile('$testPackageLibPath/a.dart', r'''
120+
// @dart=2.19
121+
import 'dart:core';
122+
class A implements MapEntry<int, int> {
123+
int get key => 0;
124+
int get value => 1;
125+
}
126+
''');
127+
128+
await resolveFile2(a.path);
129+
assertNoErrorsInResult();
130+
131+
await assertErrorsInCode(r'''
132+
import 'a.dart';
133+
class B implements A {
134+
int get key => 0;
135+
int get value => 1;
136+
}
137+
''', [
138+
error(CompileTimeErrorCode.FINAL_CLASS_IMPLEMENTED_OUTSIDE_OF_LIBRARY, 36,
139+
1),
140+
]);
141+
}
142+
72143
test_class_on() async {
73144
await assertErrorsInCode(r'''
74145
final class A {}

‎tests/language/class_modifiers/base_transitivity/base_class_different_library_error_test.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,12 @@ base mixin BaseMixinImplement implements BaseClass {}
472472
// Implementing a legacy class that implements a core library base class.
473473

474474
abstract class LegacyImplement<E extends LinkedListEntry<E>>
475-
implements LegacyImplementBaseCore<E> {}
476-
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
475+
// ^^^^^^^^^^^^^^^
476+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
477+
// [cfe] The type 'LegacyImplement' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
478+
implements
479+
LegacyImplementBaseCore<E> {}
480+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
477481
// [analyzer] COMPILE_TIME_ERROR.INVALID_USE_OF_TYPE_OUTSIDE_LIBRARY
478482
// [cfe] The class 'LinkedList' can't be implemented outside of its library because it's a base class.
479483

‎tests/language/class_modifiers/trans_legacy/legacy_superdeclaration_error_test.dart

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,108 +44,130 @@ abstract base class ImplementsLegacyMixinOnFinal implements LegacyMixinOnFinal {
4444
// Not allowed to omit base on classes with base/final superclasses.
4545

4646
abstract class ExtendsLegacyImplementsFinal extends LegacyImplementsFinal {
47-
// ^
47+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
48+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
4849
// [cfe] The type 'ExtendsLegacyImplementsFinal' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
4950
}
5051

5152
abstract class ExtendsLegacyImplementsFinal2 = LegacyImplementsFinal
52-
// ^
53+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
54+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
5355
// [cfe] The type 'ExtendsLegacyImplementsFinal2' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
5456
with
5557
_AnyMixin;
5658

5759
abstract class ExtendsLegacyExtendsFinal extends LegacyExtendsFinal {}
58-
// ^
60+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
61+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
5962
// [cfe] The type 'ExtendsLegacyExtendsFinal' must be 'base', 'final' or 'sealed' because the supertype 'ListQueue' is 'final'.
6063

6164
abstract class ExtendsLegacyExtendsFinal2 = LegacyExtendsFinal2 with _AnyMixin;
62-
// ^
65+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
66+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
6367
// [cfe] The type 'ExtendsLegacyExtendsFinal2' must be 'base', 'final' or 'sealed' because the supertype 'ListQueue' is 'final'.
6468

6569
abstract class ExtendsLegacyMixesInFinal extends LegacyMixesInFinal {}
66-
// ^
70+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
71+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
6772
// [cfe] The type 'ExtendsLegacyMixesInFinal' must be 'base', 'final' or 'sealed' because the supertype 'BigInt' is 'final'.
6873

6974
abstract class ExtendsLegacyMixesInFinal2 = LegacyMixesInFinal2 with _AnyMixin;
70-
// ^
75+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
76+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
7177
// [cfe] The type 'ExtendsLegacyMixesInFinal2' must be 'base', 'final' or 'sealed' because the supertype 'BigInt' is 'final'.
7278

7379
abstract class ExtendsLegacyImplementsBase extends LegacyImplementsBase {}
74-
// ^
80+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
81+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
7582
// [cfe] The type 'ExtendsLegacyImplementsBase' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
7683

7784
abstract class ExtendsLegacyImplementsBase2 = LegacyImplementsBase
78-
// ^
85+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
86+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
7987
// [cfe] The type 'ExtendsLegacyImplementsBase2' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
8088
with
8189
_AnyMixin;
8290

8391
abstract class MixesInLegacyImplementsFinal with LegacyImplementsFinal {}
84-
// ^
92+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
93+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
8594
// [cfe] The type 'MixesInLegacyImplementsFinal' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
8695

8796
abstract class MixesInLegacyImplementsFinal2 = Object
88-
// ^
97+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
98+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
8999
// [cfe] The type 'MixesInLegacyImplementsFinal2' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
90100
with
91101
LegacyImplementsFinal;
92102

93103
abstract class MixesInLegacyMixesInFinal with LegacyMixesInFinal2 {}
94-
// ^
104+
// ^^^^^^^^^^^^^^^^^^^^^^^^^
105+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
95106
// [cfe] The type 'MixesInLegacyMixesInFinal' must be 'base', 'final' or 'sealed' because the supertype 'BigInt' is 'final'.
96107

97108
abstract class MixesInLegacyMixesInFinal2 = Object with LegacyMixesInFinal2;
98-
// ^
109+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
110+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
99111
// [cfe] The type 'MixesInLegacyMixesInFinal2' must be 'base', 'final' or 'sealed' because the supertype 'BigInt' is 'final'.
100112

101113
abstract class MixesInLegacyImplementsBase with LegacyImplementsBase {}
102-
// ^
114+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
115+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
103116
// [cfe] The type 'MixesInLegacyImplementsBase' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
104117

105118
abstract class MixesInLegacyImplementsBase2 = Object with LegacyImplementsBase;
106-
// ^
119+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
120+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
107121
// [cfe] The type 'MixesInLegacyImplementsBase2' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
108122

109123
abstract class MixesInMixinOnFinal extends LegacyImplementsFinal
110-
// ^
124+
// ^^^^^^^^^^^^^^^^^^^
125+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
111126
// [cfe] The type 'MixesInMixinOnFinal' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
112127
with
113128
LegacyMixinOnFinal {}
114129

115130
abstract class MixesInMixinOnFinal2 = LegacyImplementsFinal
116-
// ^
131+
// ^^^^^^^^^^^^^^^^^^^^
132+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
117133
// [cfe] The type 'MixesInMixinOnFinal2' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
118134
with
119135
LegacyMixinOnFinal;
120136

121137
abstract class MixesInMixinOnBase extends LegacyMixinOnBaseSuper
122-
// ^
138+
// ^^^^^^^^^^^^^^^^^^
139+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
123140
// [cfe] The type 'MixesInMixinOnBase' must be 'base', 'final' or 'sealed' because the supertype 'LinkedListEntry' is 'base'.
124141
with
125142
LegacyMixinOnBase {}
126143

127144
abstract class MixesInMixinOnBase2 = LegacyMixinOnBaseSuper
128-
// ^
145+
// ^^^^^^^^^^^^^^^^^^^
146+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
129147
// [cfe] The type 'MixesInMixinOnBase2' must be 'base', 'final' or 'sealed' because the supertype 'LinkedListEntry' is 'base'.
130148
with
131149
LegacyMixinOnBase;
132150

133151
abstract class MixesInMixinImplementsFinal with LegacyMixinImplementsFinal {}
134-
// ^
152+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
153+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
135154
// [cfe] The type 'MixesInMixinImplementsFinal' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
136155

137156
abstract class MixesInMixinImplementsFinal2 = Object
138-
// ^
157+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
158+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
139159
// [cfe] The type 'MixesInMixinImplementsFinal2' must be 'base', 'final' or 'sealed' because the supertype 'MapEntry' is 'final'.
140160
with
141161
LegacyMixinImplementsFinal;
142162

143163
abstract class MixesInMixinImplementsBase with LegacyMixinImplementsBase {}
144-
// ^
164+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
165+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
145166
// [cfe] The type 'MixesInMixinImplementsBase' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
146167

147168
abstract class MixesInMixinImplementsBase2 = Object
148-
// ^
169+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
170+
// [analyzer] COMPILE_TIME_ERROR.SUBTYPE_OF_BASE_OR_FINAL_IS_NOT_BASE_FINAL_OR_SEALED
149171
// [cfe] The type 'MixesInMixinImplementsBase2' must be 'base', 'final' or 'sealed' because the supertype 'LinkedList' is 'base'.
150172
with
151173
LegacyMixinImplementsBase;

0 commit comments

Comments
 (0)