From cafd4b29726793b277240ac7d4709c3a0ae1fb8f Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Wed, 23 Oct 2024 08:48:00 -0700 Subject: [PATCH] Remove assertion that is no longer valid. 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. --- .../templates.runtime_renderers.dart | 14 ----------- lib/src/model/inheritable.dart | 9 ++++--- test/classes_test.dart | 17 +++++++++++++ test/end2end/model_special_cases_test.dart | 24 ++++++++++++------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 1d85494053..fa2eef1cea 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -6938,19 +6938,6 @@ class _Renderer_Inheritable extends RendererBase { parent: r); }, ), - 'inheritance': Property( - getValue: (CT_ c) => c.inheritance, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable( - c, remainingNames, 'List'), - renderIterable: (CT_ c, RendererBase r, - List 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 self, @@ -16216,7 +16203,6 @@ const _invisibleGetters = { 'attributes', 'canonicalLibrary', 'canonicalModelElement', - 'inheritance', 'isCovariant', 'isInherited', 'isOverride', diff --git a/lib/src/model/inheritable.dart b/lib/src/model/inheritable.dart index 5c494e0fe2..b2c96bc78b 100644 --- a/lib/src/model/inheritable.dart +++ b/lib/src/model/inheritable.dart @@ -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) && @@ -96,7 +96,11 @@ mixin Inheritable on ContainerMember { return super.computeCanonicalEnclosingContainer(); } - List get inheritance { + /// A roughly ordered list of this element's enclosing container's inheritance + /// chain. + /// + /// See [InheritingContainer.inheritanceChain] for details. + List get _inheritance { var inheritance = [ ...(enclosingElement as InheritingContainer).inheritanceChain, ]; @@ -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; } diff --git a/test/classes_test.dart b/test/classes_test.dart index 584e24b627..fbe6abd1b2 100644 --- a/test/classes_test.dart +++ b/test/classes_test.dart @@ -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'; @@ -182,5 +183,21 @@ class C implements A, _B {} 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. } diff --git a/test/end2end/model_special_cases_test.dart b/test/end2end/model_special_cases_test.dart index 3f085cbbc2..cd06e638fa 100644 --- a/test/end2end/model_special_cases_test.dart +++ b/test/end2end/model_special_cases_test.dart @@ -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'; @@ -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() + .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