Support typeArguments, isOriginalDeclaration, and get originalDeclaration in ClassMirror.

Support typeArguments, isOriginalDeclaration, and get originalDeclaration in ClassMirror.

R=ahe@google.com

Committed: https://code.google.com/p/dart/source/detail?r=27662

[ahe, 2013-09-18 15:13:51]

LGTM!

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:437: int typeArgIndex = JS('int',
'#.indexOf("<")', mangledName);
Drop the JS expression and just use:

int typeArgIndex = mangledName.indexOf("<");

We should fix the type inferrer instead.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:439: mirror =  new JsTypeBoundClassMirror(
Remove extra space.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:594: List<TypeMirror> get typeArguments =>
new List();
Add newline before method.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:732: class JsTypeBoundClassMirror
implements ClassMirror {
Add newline before class.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:732: class JsTypeBoundClassMirror
implements ClassMirror {
Add documentation to explain how this is different from JsClassMirror.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:733: 
Remove empty line.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:735: var _typeArgs;
Document that this variable holds values of different types.

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
File tests/lib/mirrors/typearguments_mirror_test.dart (right):

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
tests/lib/mirrors/typearguments_mirror_test.dart:7: 
Remove extra line.

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
tests/lib/mirrors/typearguments_mirror_test.dart:20: Expect.equals(cm1,
reflectClass(new Foo<String>().runtimeType));
Consider using stringify.dart, it is much easier.

[zarah, 2013-09-19 14:15:33]

PTAL

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:437: int typeArgIndex = JS('int',
'#.indexOf("<")', mangledName);
On 2013/09/18 15:13:51, ahe wrote:
> Drop the JS expression and just use:
> 
> int typeArgIndex = mangledName.indexOf("<");
> 
> We should fix the type inferrer instead.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:439: mirror =  new JsTypeBoundClassMirror(
On 2013/09/18 15:13:51, ahe wrote:
> Remove extra space.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:594: List<TypeMirror> get typeArguments =>
new List();
On 2013/09/18 15:13:51, ahe wrote:
> Add newline before method.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:732: class JsTypeBoundClassMirror
implements ClassMirror {
On 2013/09/18 15:13:51, ahe wrote:
> Add newline before class.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:732: class JsTypeBoundClassMirror
implements ClassMirror {
On 2013/09/18 15:13:51, ahe wrote:
> Add documentation to explain how this is different from JsClassMirror.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:733: 
On 2013/09/18 15:13:51, ahe wrote:
> Remove empty line.

Done.

https://codereview.chromium.org/23707046/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:735: var _typeArgs;
On 2013/09/18 15:13:51, ahe wrote:
> Document that this variable holds values of different types.

Done.

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
File tests/lib/mirrors/typearguments_mirror_test.dart (right):

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
tests/lib/mirrors/typearguments_mirror_test.dart:7: 
On 2013/09/18 15:13:51, ahe wrote:
> Remove extra line.

Done.

https://codereview.chromium.org/23707046/diff/1/tests/lib/mirrors/typeargumen...
tests/lib/mirrors/typearguments_mirror_test.dart:20: Expect.equals(cm1,
reflectClass(new Foo<String>().runtimeType));
On 2013/09/18 15:13:51, ahe wrote:
> Consider using stringify.dart, it is much easier.

Done.

[ahe, 2013-09-19 14:38:08]

LGTM!

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:441: // remove the '<' in the beginning
and '>' in the end.
Not a proper sentence, please use uppercase for the first letter.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:737: * [typeVariables] will return a list
of the type arguments, in constrast
typeVariables -> typeArguments.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:743: /**
Add newline before comment.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/mirr...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/mirr...
sdk/lib/_internal/lib/mirrors_patch.dart:21: patch ClassMirror reflectClass(Type
key) => js.reflectType(key).originalDeclaration;
Long line.

[zarah, 2013-09-19 14:55:08]

Committed patchset #2 manually as r27662 (presubmit successful).

[zarah, 2013-09-19 15:02:10]

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:441: // remove the '<' in the beginning
and '>' in the end.
On 2013/09/19 14:38:08, ahe wrote:
> Not a proper sentence, please use uppercase for the first letter.

Done.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:737: * [typeVariables] will return a list
of the type arguments, in constrast
On 2013/09/19 14:38:08, ahe wrote:
> typeVariables -> typeArguments.

Done.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:743: /**
On 2013/09/19 14:38:08, ahe wrote:
> Add newline before comment.

Done.

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/mirr...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://codereview.chromium.org/23707046/diff/6001/sdk/lib/_internal/lib/mirr...
sdk/lib/_internal/lib/mirrors_patch.dart:21: patch ClassMirror reflectClass(Type
key) => js.reflectType(key).originalDeclaration;
On 2013/09/19 14:38:08, ahe wrote:
> Long line.

Done.