Skip to content

Commit 95f4208

Browse files
srawlinssrawlins
authored
Simplify Inheritable.computeCanonicalEnclosingContainer. (#4047)
This method is very complicated, so any simplification is handy. * It used to start with a very long if-then body, and the else body was rather lost far below. Instead we can implement some simple short circuiting, if we're not dealing with an inherited element. * It used to carry around a `found` variable, breaking the loop when it found a container. But it's simpler to just return at that point. * The final return value is always `null`, representing a state where no canonical enclosing container is found. Just return `null` to simplify understandability. Otherwise this is a no-op.
1 parent e4f9451 commit 95f4208

File tree

1 file changed

+46
-54
lines changed

1 file changed

+46
-54
lines changed

‎lib/src/model/inheritable.dart‎

Lines changed: 46 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -53,63 +53,55 @@ mixin Inheritable on ContainerMember {
5353

5454
@override
5555
Container? computeCanonicalEnclosingContainer() {
56-
if (isInherited) {
57-
var searchElement = element.baseElement;
58-
// TODO(jcollins-g): generate warning if an inherited element's definition
59-
// is in an intermediate non-canonical class in the inheritance chain?
60-
Container? found;
61-
var reverseInheritance = _inheritance.reversed.toList();
62-
for (var i = 0; i < reverseInheritance.length; i++) {
63-
var container = reverseInheritance[i];
64-
if (container.containsElement(searchElement)) {
65-
var previousIsHiddenAndNotDefining = i > 0 &&
66-
_isHiddenInterface(reverseInheritance[i - 1]) &&
67-
container != definingEnclosingContainer;
68-
var thisIsHiddenAndDefining = _isHiddenInterface(container) &&
69-
container == definingEnclosingContainer;
70-
// If the previous container in the search is one of the "hidden"
71-
// interfaces, and it's not this member's defining container, OR if
72-
// this container in the search is one of the "hidden" interfaces,
73-
// and it is also this member's defining container, then we can just
74-
// immediately return the canonical enclosing container of the
75-
// overridden member in the previous, non-hidden container in the
76-
// inheritance.
77-
if (previousIsHiddenAndNotDefining || thisIsHiddenAndDefining) {
78-
var previousVisible = reverseInheritance
79-
.take(i)
80-
.lastWhere((e) => !_isHiddenInterface(e));
81-
var membersInPreviousVisible = previousVisible.allModelElements
82-
.where((e) => e.name == name)
83-
.whereType<Inheritable>()
84-
.whereNotType<Field>();
85-
assert(
86-
membersInPreviousVisible.length == 1,
87-
'found multiple members named "$name" in '
88-
'"${previousVisible.name}": '
89-
'${membersInPreviousVisible.toList()}');
90-
return membersInPreviousVisible.first.canonicalEnclosingContainer;
91-
}
92-
var canonicalContainer = packageGraph
93-
.findCanonicalModelElementFor(container) as Container?;
94-
// TODO(jcollins-g): invert this lookup so traversal is recursive
95-
// starting from the ModelElement.
96-
if (canonicalContainer != null) {
97-
assert(canonicalContainer.isCanonical);
98-
assert(canonicalContainer.containsElement(searchElement));
99-
found = canonicalContainer;
100-
break;
101-
}
56+
if (!isInherited || definingEnclosingContainer is Extension) {
57+
return super.computeCanonicalEnclosingContainer();
58+
}
59+
60+
var searchElement = element.baseElement;
61+
// TODO(jcollins-g): generate warning if an inherited element's definition
62+
// is in an intermediate non-canonical class in the inheritance chain?
63+
var reverseInheritance = _inheritance.reversed.toList();
64+
for (var i = 0; i < reverseInheritance.length; i++) {
65+
var container = reverseInheritance[i];
66+
if (container.containsElement(searchElement)) {
67+
var previousIsHiddenAndNotDefining = i > 0 &&
68+
_isHiddenInterface(reverseInheritance[i - 1]) &&
69+
container != definingEnclosingContainer;
70+
var thisIsHiddenAndDefining = _isHiddenInterface(container) &&
71+
container == definingEnclosingContainer;
72+
// If the previous container in the search is one of the "hidden"
73+
// interfaces, and it's not this member's defining container, OR if this
74+
// container in the search is one of the "hidden" interfaces, and it is
75+
// also this member's defining container, then we can immediately return
76+
// the canonical enclosing container of the overridden member in the
77+
// previous, non-hidden container in the inheritance.
78+
if (previousIsHiddenAndNotDefining || thisIsHiddenAndDefining) {
79+
var previousVisible = reverseInheritance
80+
.take(i)
81+
.lastWhere((e) => !_isHiddenInterface(e));
82+
var membersInPreviousVisible = previousVisible.allModelElements
83+
.where((e) => e.name == name)
84+
.whereType<Inheritable>()
85+
.whereNotType<Field>();
86+
assert(
87+
membersInPreviousVisible.length == 1,
88+
'found multiple members named "$name" in '
89+
'"${previousVisible.name}": '
90+
'${membersInPreviousVisible.toList()}');
91+
return membersInPreviousVisible.first.canonicalEnclosingContainer;
92+
}
93+
var canonicalContainer =
94+
packageGraph.findCanonicalModelElementFor(container) as Container?;
95+
// TODO(jcollins-g): invert this lookup so traversal is recursive
96+
// starting from the ModelElement.
97+
if (canonicalContainer != null) {
98+
return canonicalContainer;
10299
}
103100
}
104-
if (found != null) {
105-
return found;
106-
}
107-
} else if (definingEnclosingContainer is! Extension) {
108-
// TODO(jcollins-g): factor out extension logic into [Extendable].
109-
return packageGraph.findCanonicalModelElementFor(enclosingElement)
110-
as Container?;
111101
}
112-
return super.computeCanonicalEnclosingContainer();
102+
103+
// None of the super-containers are canonical.
104+
return null;
113105
}
114106

115107
/// Whether [c] is a "hidden" interface.

0 commit comments

Comments
 (0)