Fix ClassElement.allSupertypes to include interfaces of mixins (issue 29767)

Fix ClassElement.allSupertypes to include interfaces of mixins (issue 29767)

R=jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/ebe2ce681e50e6d8f40c4c2c233de6c93a214706

[Brian Wilkerson, 2017-08-17 21:50:23]

brianwilkerson@google.com changed reviewers:
+ jmesserly@google.com

[Brian Wilkerson, 2017-08-17 21:50:24]

Note that this also changes some of the types being added so that they carry
along type arguments when they used to only have type parameters. None of the
tests were affected by this (which is a bit disturbing), but it might have an
impact on some clients.

[Jennifer Messerly, 2017-08-17 21:57:29]

one question, otherwise lgtm!


Yeah it doesn't look like allSupertypes is used except by:

pkg/analysis_server/lib/src/services/completion/dart/inherited_reference_contributor.dart
pkg/analysis_server/lib/src/services/correction/flutter_util.dart

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1157: for (InterfaceType type in
currentType.interfaces) {
oh wow, nice catch!

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1164: for (InterfaceType type in
type.interfaces) {
there's also the weird case where a mixin's mixin is considered part of its
superclass:


class M1 { m() => print('M1.m()'); }

// this only works when M2 is declared with ClassTypeAlias syntax,
// and it mixes in exactly one other mixin
class M2 = Object with M1;

class B { m() => print('B.m()'); }
class C extends B with M2 {} // M2 is a legal mixin, strangely

main() {
  new C().m(); // prints M1.m
}

[Brian Wilkerson, 2017-08-17 22:49:25]

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1164: for (InterfaceType type in
type.interfaces) {
Is it still the case that classes used as mixins can only extend Object? If not,
does the superclass of the mixin need to be included in this list?

If the answer to either is "yes", then I think the for loop on line 1160 can be
replaced by

for (InterfaceType type in currentType.mixins) {
  typesToVisit.add(type);
}

[Jennifer Messerly, 2017-08-17 23:14:39]

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/3000183002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1164: for (InterfaceType type in
type.interfaces) {
On 2017/08/17 22:49:25, Brian Wilkerson wrote:
> Is it still the case that classes used as mixins can only extend Object? If
not,
> does the superclass of the mixin need to be included in this list?
> 
> If the answer to either is "yes", then I think the for loop on line 1160 can
be
> replaced by
> 
> for (InterfaceType type in currentType.mixins) {
>   typesToVisit.add(type);
> }

oh yeah, good call! yeah with super mixins, the superclass of the mixin would be
considered as part of the `all supertypes` as well

[Brian Wilkerson, 2017-08-18 00:36:07]

Updated, ptal

[Jennifer Messerly, 2017-08-18 01:32:35]

lgtm++ !

[Brian Wilkerson, 2017-08-18 14:30:09]

Description was changed from

==========
Fix ClassElement.allSupertypes to include interfaces of mixins (issue 29767)
==========

to

==========
Fix ClassElement.allSupertypes to include interfaces of mixins (issue 29767)

R=jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ebe2ce681e50e6d8f40c4c2c233de6c93a214706
==========

[Brian Wilkerson, 2017-08-18 14:30:10]

Committed patchset #2 (id:20001) manually as
ebe2ce681e50e6d8f40c4c2c233de6c93a214706 (presubmit successful).