Integrate recent parameter override logic

Integrate recent parameter override logic

R=leafp@google.com, paulberry@google.com, vsm@google.com

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

[Brian Wilkerson, 2015-08-25 21:49:16]

brianwilkerson@google.com changed reviewers:
+ leafp@google.com, paulberry@google.com, vsm@google.com

[Brian Wilkerson, 2015-08-25 21:49:17]

This carries forward the changes from
https://codereview.chromium.org/1311863005/, with the same kind of semantic
changes as in the original port.

[Paul Berry, 2015-08-25 22:11:14]

lgtm, though I didn't scrutinize it in a great deal of detail, since I'm hoping
Leaf and Vijay will do that :)

[Leaf, 2015-08-25 23:06:37]

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:195: for (int i =
overriddenParameters.length - 1; i >= 0; i--) {
I think using 
  overridenParameter = overridenParameters.lastWhere(...) 
instead of a loop here would make the code clearer, but that might be my
misspent youth showing through.

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:196: overriddenParameter =
overriddenParameters[index];
I think this should be i instead of index?

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:222: if (type is TypeParameterType) {
Hmm, is this to address the possibility that the overridden method came from a
generic class, and we have an uninstantiated type?  I think this probably means
we should be using the instantiated types here (Members maybe they're called?).

[Brian Wilkerson, 2015-08-26 15:25:37]

PTAL

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:195: for (int i =
overriddenParameters.length - 1; i >= 0; i--) {
Done

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:196: overriddenParameter =
overriddenParameters[index];
Subsumed by previous change.

https://codereview.chromium.org/1311433005/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong_mode.dart:222: if (type is TypeParameterType) {
> Hmm, is this to address the possibility that the overridden method came from a
> generic class, and we have an uninstantiated type?

Yes, but we do have the instantiated type, so this test was unnecessary (and
wrong). I've remove it and added some tests to cover a small number of cases
involving generic types.

[Leaf, 2015-08-26 17:11:09]

Nice!  lgtm.

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong_mode.dart:241: * it appears at the given
[index] in it's enclosing element's list of
it's -> its.

[vsm, 2015-08-26 17:14:18]

lgtm - thanks for migrating this!

[Brian Wilkerson, 2015-08-26 17:35:39]

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

[Brian Wilkerson, 2015-08-26 17:36:15]

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong_mode.dart:241: * it appears at the given
[index] in it's enclosing element's list of
Done

[vicb, 2015-08-29 06:10:42]

victor.berchet@gmail.com changed reviewers:
+ victor.berchet@gmail.com

[vicb, 2015-08-29 06:10:42]

I think this CL should be amended to support SDK v1.11, see inline comments.

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong_mode.dart:422: overriddenMethods =
overriddenMethods ??
If the analyzer is supposed to work with SDK v1.11 (as constrained in the
pubspec), ?? should not be used. It causes us troubles with Angular2.

see
https://github.com/dart-lang/sdk/commit/335b87c701d4176a669a318d50f345d4b7977...

[Brian Wilkerson, 2015-08-29 16:15:05]

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong_mode.dart (right):

https://codereview.chromium.org/1311433005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong_mode.dart:422: overriddenMethods =
overriddenMethods ??
If only we had a tool that could tell us when we made a mistake like that :-(

Fixed by https://codereview.chromium.org/1301333007/.