Move TypeParameterizedElementForLink and TypeParameterElementForLink to impl.

Move TypeParameterizedElementForLink and TypeParameterElementForLink to impl.

There is much more to do - do the same not just for classes, but also
for typedefs and executables; this is just minimally valuable (maybe)
change.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

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

[Paul Berry, 2016-05-17 19:28:03]

lgtm

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

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1857: * The context in which this
element is resynthesized.
Nit: the comment should specify that this value is `null` if the element was not
resynthesized from a summary.

(Otherwise a reader might assume that it is never null).

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:5370: * TODO(scheglov) make
private after MLI
Can you expand on this comment?  It's not clear to me what "MLI" refers to.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/link.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/link.dart:958: */
Nit: remove this comment and add an "@override" annotation, since this is now
overriding an abstract method in ResynthesizerContext.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/resynthesize.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:705: * TODO(scheglov) remove
after MLI
The meaning of "MLI" isn't obvious here either.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:1668: * TODO(scheglov) clean up
after MLI
Ditto

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:1829: if (classElement.name ==
'Comparable') {
Why do we need special case code for `Comparable`?  This seems like a red flag
that we might have a bug.

Assuming this doesn't indicate a bug, we should at least have a TODO comment
here explaining what would have to happen to make this special case code
unnecessary.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
File pkg/analyzer/test/src/summary/resynthesize_test.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/resynthesize_test.dart:677: // TODO(scheglov) MLI
restore testing annotations for type parameters
Another unclear usage of "MLI"

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/resynthesize_test.dart:678: if (original is
TypeParameterElement) {} else {
Nit: use "is!" to avoid the empty if-clause.

How long do you anticipate needing to keep this code disabled?  If it's just for
a day or two, then I'm not concerned.  But if it stays disabled longer, I worry
that this will become a gap in our testing, and I especially want to avoid that
because I'm guessing that the resynthesis code is going to be touched a lot in
your current arc of work, and it would be good to keep our test coverage high
while that's happening.

[scheglov, 2016-05-17 19:42:21]

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

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:1857: * The context in which this
element is resynthesized.
On 2016/05/17 19:28:02, Paul Berry wrote:
> Nit: the comment should specify that this value is `null` if the element was
not
> resynthesized from a summary.
> 
> (Otherwise a reader might assume that it is never null).

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:5370: * TODO(scheglov) make
private after MLI
On 2016/05/17 19:28:02, Paul Berry wrote:
> Can you expand on this comment?  It's not clear to me what "MLI" refers to.

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/link.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/link.dart:958: */
On 2016/05/17 19:28:02, Paul Berry wrote:
> Nit: remove this comment and add an "@override" annotation, since this is now
> overriding an abstract method in ResynthesizerContext.

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/resynthesize.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:705: * TODO(scheglov) remove
after MLI
On 2016/05/17 19:28:02, Paul Berry wrote:
> The meaning of "MLI" isn't obvious here either.

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:1668: * TODO(scheglov) clean up
after MLI
On 2016/05/17 19:28:02, Paul Berry wrote:
> Ditto

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/resynthesize.dart:1829: if (classElement.name ==
'Comparable') {
On 2016/05/17 19:28:02, Paul Berry wrote:
> Why do we need special case code for `Comparable`?  This seems like a red flag
> that we might have a bug.
> 
> Assuming this doesn't indicate a bug, we should at least have a TODO comment
> here explaining what would have to happen to make this special case code
> unnecessary.

I forgot to remove this debug code after fixing the problem.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
File pkg/analyzer/test/src/summary/resynthesize_test.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/resynthesize_test.dart:677: // TODO(scheglov) MLI
restore testing annotations for type parameters
On 2016/05/17 19:28:02, Paul Berry wrote:
> Another unclear usage of "MLI"

Done.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/resynthesize_test.dart:678: if (original is
TypeParameterElement) {} else {
On 2016/05/17 19:28:02, Paul Berry wrote:
> Nit: use "is!" to avoid the empty if-clause.
> 
> How long do you anticipate needing to keep this code disabled?  If it's just
for
> a day or two, then I'm not concerned.  But if it stays disabled longer, I
worry
> that this will become a gap in our testing, and I especially want to avoid
that
> because I'm guessing that the resynthesis code is going to be touched a lot in
> your current arc of work, and it would be good to keep our test coverage high
> while that's happening.

Yeah, I don't like turning off tests too.
I will try to restore it ASAP.
Hopefully this feature (annotations of type parameters) is not used often.

[Brian Wilkerson, 2016-05-17 19:42:26]

lgtm

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

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:5161: final
TypeParameterizedElementMixin _enclosingTypeParameterContext;
It looks wrong to me to use a mixin as a type this way. I'm guessing we can't
just use TypeParameterizedElement instead, and I suppose it would be foolish to
define an interface to avoid it, but it messes with my concept of what a mixin
ought to be.

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:5228: return _bound ??=
resynthesizerContext.resolveTypeRef(
Is it safe to assume here that "resynthesizerContext" is not null?

[Brian Wilkerson, 2016-05-17 19:43:57]

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
File pkg/analyzer/test/src/summary/resynthesize_test.dart (right):

https://codereview.chromium.org/1986853003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/resynthesize_test.dart:678: if (original is
TypeParameterElement) {} else {
> Hopefully this feature (annotations of type parameters) is not used often.

If they are not already doing so, the flutter team plans to do this a lot (the
@required annotation).

[scheglov, 2016-05-17 19:51:34]

Description was changed from

==========
Move TypeParameterizedElementForLink and TypeParameterElementForLink to impl.

There is much more to do - do the same not just for classes, but also
for typedefs and executables; this is just minimally valuable (maybe)
change.

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Move TypeParameterizedElementForLink and TypeParameterElementForLink to impl.

There is much more to do - do the same not just for classes, but also
for typedefs and executables; this is just minimally valuable (maybe)
change.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

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

[scheglov, 2016-05-17 19:51:36]

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