Simplify how inheritance is computed.

[srawlins]

The motivation here is that Inheritable.computeCanonicalEnclosingContainer is a complicated subroutine,
even after several refactorings. This is the code that decides where to link inherited members. So questions like "ah ha, List has a .isEmpty, but it inherits the docs from... Iterable.isEmpty." It's the wild west out there, with docs coming from indirect supertypes, interfaces, mixins, etc.

A big portion of the complexity comes from choosing an order in which to search the supertypes. We have to take a directed acyclic graph, and choose an order. Prior to this change, the ordering comes from each container class's implementation of get inheritanceChain. I think on paper this sounds like a good idea: let a Mixin compute its inheritance chain, let an Enum compute its inheritance chain. In practice, it spread complexity out too much, and a combined implementation is actually quite compact.

So in this change we remove InheritingContainer.inheritanceChain, the expandInheritanceChain extension getter, and Inheritable._inheritance. We replace all of this code with Inheritable._enclosingSuperTypes, which returns all of the various supertypes of the enclosing element, in a specific order. This getter performs a fairly simple walk up the inheritance tree (extended types, implemented types, mixed-in types, and superclass constraints).

The single call to Inheritable._inheritance is now a call to Inheritable._enclosingSuperTypes. computeCanonicalEnclosingContainer calls this in order to collect its set of candidates for the canonical enclosing container. _enclosingSuperTypes handles Object differently, which allows the impl of computeCanonicalEnclosingContainer to be much simplified, no longer concerning itself with any previous candidate.

This is very nearly a no-op. There is some change in the handling of Object and it's members. Because of this, we tweak the enum_test to no longer use --no-link-to-remote, so that the links to Object.hashCode and Enum.index continue to work.

I tested this with some manual verification of the Dart SDK docs, Flutter SDK docs, and the collection package docs.

[sigurdm]

I worry that using recursion here could lead to stackoverflows...
Perhaps better to manually maintain a stack of nodes being processed.

[srawlins]

Yeah I get what you mean. I think I tried that initially, and couldn't map this recursive algorithm into a loop with a stack. I think since it is a kind of depth-first walk, with the deepest containers in the hierarchy being added first. It would require a second mapping maybe, where you also track the containers that you will add to the stack.

In any case, it shouldn't be a problem because (a) the very first thing we do is check if container is in the processed set, and return quick if it is (so any illegal loops would not break this algorithm). And it's pretty unfathomable to have a straight-line hierarchy of more than... 100 containers? Even 10 is a wild stretch, much less 100 or 1,000 or 10,000.

[sigurdm]

And it's pretty unfathomable to have a straight-line hierarchy of more than... 100 containers?

Hard to imagine writing by hand - yes. But nothing in the spec prevents it, and our tools should be able to handle also bizarre generated code.