Skip to content

Commit c5ef843

Browse files
authored
Correctly handle intermediate abstract classes containing (external?) implementations (#2449)
* Add test * It's Object that is the problem... * update test * adjustment * Fix up test * Cleanups
1 parent d320b5e commit c5ef843

File tree

3 files changed

+59
-1
lines changed

3 files changed

+59
-1
lines changed

lib/src/model/class.dart

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,39 @@ class Class extends Container
376376
var cmap = inheritance.getInheritedConcreteMap2(element);
377377
var imap = inheritance.getInheritedMap2(element);
378378

379+
var inheritanceChainElements;
380+
379381
var combinedMap = <String, ExecutableElement>{};
380382
for (var nameObj in cmap.keys) {
381383
combinedMap[nameObj.name] = cmap[nameObj];
382384
}
383385
for (var nameObj in imap.keys) {
384-
combinedMap[nameObj.name] ??= imap[nameObj];
386+
if (combinedMap[nameObj.name] != null) {
387+
// Elements in the inheritance chain starting from [this.element]
388+
// down to, but not including, [Object].
389+
inheritanceChainElements ??=
390+
inheritanceChain.map((c) => c.element).toList();
391+
// [packageGraph.specialClasses] is not available yet.
392+
bool _isDartCoreObject(ClassElement e) =>
393+
e.name == 'Object' && e.library.name == 'dart.core';
394+
assert(inheritanceChainElements
395+
.contains(imap[nameObj].enclosingElement) ||
396+
_isDartCoreObject(imap[nameObj].enclosingElement));
397+
398+
// If the concrete object from [InheritanceManager3.getInheritedConcreteMap2]
399+
// is farther from this class in the inheritance chain than the one
400+
// provided by InheritedMap2, prefer InheritedMap2. This
401+
// correctly accounts for intermediate abstract classes that have
402+
// method/field implementations.
403+
if (inheritanceChainElements
404+
.indexOf(combinedMap[nameObj.name].enclosingElement) <
405+
inheritanceChainElements
406+
.indexOf(imap[nameObj].enclosingElement)) {
407+
combinedMap[nameObj.name] = imap[nameObj];
408+
}
409+
} else {
410+
combinedMap[nameObj.name] = imap[nameObj];
411+
}
385412
}
386413

387414
__inheritedElements = combinedMap.values.toList();

test/end2end/model_test.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,6 +1313,27 @@ void main() {
13131313
});
13141314

13151315
group('Class edge cases', () {
1316+
test('Overrides from intermediate abstract classes are picked up correctly',
1317+
() {
1318+
var IntermediateAbstractSubclass = fakeLibrary.allClasses
1319+
.firstWhere((c) => c.name == 'IntermediateAbstractSubclass');
1320+
var operatorEquals = IntermediateAbstractSubclass.inheritedOperators
1321+
.firstWhere((o) => o.name == 'operator ==');
1322+
expect(operatorEquals.definingEnclosingContainer.name,
1323+
equals('IntermediateAbstract'));
1324+
});
1325+
1326+
test(
1327+
'Overrides from intermediate abstract classes that have external implementations via the SDK are picked up correctly',
1328+
() {
1329+
var dartCore =
1330+
packageGraph.libraries.firstWhere((l) => l.name == 'dart:core');
1331+
var intClass = dartCore.allClasses.firstWhere((c) => c.name == 'int');
1332+
var operatorEqualsInt = intClass.inheritedOperators
1333+
.firstWhere((o) => o.name == 'operator ==');
1334+
expect(operatorEqualsInt.definingEnclosingContainer.name, equals('num'));
1335+
});
1336+
13161337
test('Factories from unrelated classes are linked correctly', () {
13171338
var A = packageGraph.localPublicLibraries
13181339
.firstWhere((l) => l.name == 'unrelated_factories')

testing/test_package/lib/fake.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,3 +1228,13 @@ class Super4 implements Super1 {}
12281228
class _Super5 implements _Super2 {}
12291229

12301230
class Super6 implements _Super5 {}
1231+
1232+
1233+
abstract class IntermediateAbstract implements Object {
1234+
/// This is an override.
1235+
@override
1236+
bool operator==(Object other) {}
1237+
}
1238+
1239+
/// This should inherit [==] from [IntermediateAbstract].
1240+
class IntermediateAbstractSubclass extends IntermediateAbstract {}

0 commit comments

Comments
 (0)