Support metadata for methods and fields in the transformed view.

Support metadata for methods and fields in the transformed view.
BUG=
R=floitsch@google.com

Committed: https://github.com/dart-lang/reflectable/commit/34d577b482fc0fb31b970ee5f2ccdf78f1cd3be4

[sigurdm, 2015-07-14 08:39:35]

sigurdm@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-07-14 09:47:28]

LGTM, although I'm not sure how clashing class-names are handled.
Please add a test, where you have two libraries that import different libraries
that contain classes of the same name and use those as annotations:

== a1.dart
class A { final x; const A() : this.x = 499; }
== a2.dart
class A { final x; const A() : this.x = 42; }
== lib1.dart
import 'a1.dart';

class B { @A() foo() => 499; }
== lib2.dart
import 'a2.dart';

class C { @A() foo() => 499; }
== main.dart
import 'lib1.dart';
import 'lib2.dart';
import '<reflectable>';

main() {
  <get the annotations of B.foo and C.foo>;
}

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
File reflectable/lib/src/mirrors_unimpl.dart (right):

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:526: final int ownerIndex;
Is 'ownerIndex' a "public" field?

Also: I have an idea what 'name' and 'reflectable' are (although 'reflectable'
is probably a bad name), but not really what the int-descriptor, ownerIndex and
reflectable are.
It could be that the superclass or other classes higher up have the
documentation (and I'm just looking out of context), but if not, please add some
documentation.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:532: : _metadata = metadata == null
Add parenthesis:
(metadata == null) ? null : ...

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:642: : _metadata = metadata == null ?
null : new UnmodifiableListView(metadata);
long line.
add parenthesis.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
File reflectable/lib/src/transformer_implementation.dart (right):

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1524: /// Returns a String
with the textual representations of the metadata list.
Explain what the arguments are.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1527: String
extractMetadataCode(Iterable<Annotation> nodes,
Explain, why this is called "..Code".
I assume that it is, because we don't get the actual metadata values, but just
the code that is needed to build the constants.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1527: String
extractMetadataCode(Iterable<Annotation> nodes,
Would it make sense to call this method with an element, and then get the 'node'
and the 'metadata' from the element in the function (instead of doing it at the
callsite)?

From what I can see, all call-sites currently have:
extractMetadataCode(element.node.metadata, element.metadata).

Might as well pass in the element here, and get these fields here. (assuming
that there is a common "Element" interface).

[sigurdm, 2015-07-14 13:06:50]

Yes - name clashes is a problem (as is the whole representation of constants as
Strings.) I am not sure how to solve it best. Maybe we can import each library
with a prefix, and reconstruct the structure of the constant, adding all the
right prefixes.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
File reflectable/lib/src/mirrors_unimpl.dart (right):

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:526: final int ownerIndex;
On 2015/07/14 09:47:28, floitsch wrote:
> Is 'ownerIndex' a "public" field?
> 
> Also: I have an idea what 'name' and 'reflectable' are (although 'reflectable'
> is probably a bad name), but not really what the int-descriptor, ownerIndex
and
> reflectable are.
> It could be that the superclass or other classes higher up have the
> documentation (and I'm just looking out of context), but if not, please add
some
> documentation.

They should all be private. Fixed. Added a bit of documentation.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:532: : _metadata = metadata == null
On 2015/07/14 09:47:28, floitsch wrote:
> Add parenthesis:
> (metadata == null) ? null : ...

Done.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/mirrors...
reflectable/lib/src/mirrors_unimpl.dart:642: : _metadata = metadata == null ?
null : new UnmodifiableListView(metadata);
On 2015/07/14 09:47:28, floitsch wrote:
> long line.
> add parenthesis.

Done.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
File reflectable/lib/src/transformer_implementation.dart (right):

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1524: /// Returns a String
with the textual representations of the metadata list.
On 2015/07/14 09:47:28, floitsch wrote:
> Explain what the arguments are.

Done.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1527: String
extractMetadataCode(Iterable<Annotation> nodes,
On 2015/07/14 09:47:28, floitsch wrote:
> Would it make sense to call this method with an element, and then get the
'node'
> and the 'metadata' from the element in the function (instead of doing it at
the
> callsite)?
> 
> From what I can see, all call-sites currently have:
> extractMetadataCode(element.node.metadata, element.metadata).
> 
> Might as well pass in the element here, and get these fields here. (assuming
> that there is a common "Element" interface).

Done.

https://codereview.chromium.org/1236983004/diff/1/reflectable/lib/src/transfo...
reflectable/lib/src/transformer_implementation.dart:1527: String
extractMetadataCode(Iterable<Annotation> nodes,
On 2015/07/14 09:47:28, floitsch wrote:
> Explain, why this is called "..Code".
> I assume that it is, because we don't get the actual metadata values, but just
> the code that is needed to build the constants.

Done.

[sigurdm, 2015-07-14 13:07:26]

I added some (failing) testcases to show the problem.

[sigurdm, 2015-07-14 13:07:53]

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