Skip to content

Commit

Permalink
Remove assertion that is no longer valid. (#3913)
Browse files Browse the repository at this point in the history
I actually can't imagine how it was ever valid. The
`InheritingContainer.inheritanceChain` implementations can always result in a
list that contains two instances of Object.

Also make `inheritance` private, and improve some related tests.
  • Loading branch information
srawlins authored Oct 23, 2024
1 parent f5baa7c commit 73344a2
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 25 deletions.
14 changes: 0 additions & 14 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6938,19 +6938,6 @@ class _Renderer_Inheritable extends RendererBase<Inheritable> {
parent: r);
},
),
'inheritance': Property(
getValue: (CT_ c) => c.inheritance,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'List<InheritingContainer>'),
renderIterable: (CT_ c, RendererBase<CT_> r,
List<MustachioNode> ast, StringSink sink) {
return c.inheritance.map((e) => _render_InheritingContainer(
e, ast, r.template, sink,
parent: r));
},
),
'isCovariant': Property(
getValue: (CT_ c) => c.isCovariant,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down Expand Up @@ -16216,7 +16203,6 @@ const _invisibleGetters = {
'attributes',
'canonicalLibrary',
'canonicalModelElement',
'inheritance',
'isCovariant',
'isInherited',
'isOverride',
Expand Down
9 changes: 6 additions & 3 deletions lib/src/model/inheritable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ mixin Inheritable on ContainerMember {
Container? previous;
Container? previousNonSkippable;
Container? found;
for (var c in inheritance.reversed) {
for (var c in _inheritance.reversed) {
// Filter out mixins.
if (c.containsElement(searchElement)) {
if ((packageGraph.inheritThrough.contains(previous) &&
Expand Down Expand Up @@ -96,7 +96,11 @@ mixin Inheritable on ContainerMember {
return super.computeCanonicalEnclosingContainer();
}

List<InheritingContainer> get inheritance {
/// A roughly ordered list of this element's enclosing container's inheritance
/// chain.
///
/// See [InheritingContainer.inheritanceChain] for details.
List<InheritingContainer> get _inheritance {
var inheritance = [
...(enclosingElement as InheritingContainer).inheritanceChain,
];
Expand All @@ -120,7 +124,6 @@ mixin Inheritable on ContainerMember {
if (inheritance.last != object) {
inheritance.add(object);
}
assert(inheritance.where((e) => e == object).length == 1);
return inheritance;
}

Expand Down
17 changes: 17 additions & 0 deletions test/classes_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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 'package:dartdoc/src/special_elements.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

Expand Down Expand Up @@ -182,5 +183,21 @@ class C<T> implements A<T>, _B<T> {}
expect(c.publicInterfaces.first.modelElement, library.classes.named('A'));
}

void test_inheritanceOfObjectInstanceMethod() async {
// This code is written such that `Inheritable._inheritance` for
// `A.toString()` includes two copies of Object; one from walking up B's
// chain, and then the second from walking up C's chain.
var library = await bootPackageWithLibrary('''
class A implements B, C {}
class B implements C {}
class C implements D {}
class D implements Object {}
''');

var object = library.packageGraph.specialClasses[SpecialClass.object]!;
var toString = library.classes.named('A').instanceMethods.named('toString');
expect(toString.canonicalEnclosingContainer, object);
}

// TODO(srawlins): Test everything else about classes.
}
24 changes: 16 additions & 8 deletions test/end2end/model_special_cases_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
library;

import 'package:async/async.dart';
import 'package:dartdoc/src/element_type.dart';
import 'package:dartdoc/src/matching_link_result.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/model_utils.dart';
Expand Down Expand Up @@ -413,14 +414,21 @@ void main() {
.singleWhere((f) => f.name == 'hashCode');
var objectModelElement =
sdkAsPackageGraph.specialClasses[SpecialClass.object];
// If this fails, EventTarget might have been changed to no longer
// inherit from Interceptor. If that's true, adjust test case to
// another class that does.
expect(hashCode.inheritance.any((c) => c.name == 'Interceptor'), isTrue);
// If EventTarget really does start implementing hashCode, this will
// fail.
expect(hashCode.href,
equals('${htmlBasePlaceholder}dart-core/Object/hashCode.html'));
expect(
eventTarget.superChain,
contains(isA<ParameterizedElementType>()
.having((t) => t.name, 'name', 'Interceptor')),
reason: 'EventTarget appears to no longer subtype Interceptor, which '
'makes the premise of this test invalid. To keep the test case '
'valid, we need to use a class that subclasses Interceptor.',
);
expect(
hashCode.href,
equals('${htmlBasePlaceholder}dart-core/Object/hashCode.html'),
reason:
"EventTarget appears to have an explicit override of 'hashCode', "
'which makes this test case invalid.',
);
expect(hashCode.canonicalEnclosingContainer, equals(objectModelElement));
expect(
eventTarget.publicSuperChainReversed
Expand Down

0 comments on commit 73344a2

Please sign in to comment.