Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata is not available for synthetic getters/setters #78

Closed
lassedamgaard opened this issue Mar 1, 2016 · 5 comments
Closed

Metadata is not available for synthetic getters/setters #78

lassedamgaard opened this issue Mar 1, 2016 · 5 comments

Comments

@lassedamgaard
Copy link

So I was porting over some code based on raw mirrors that processes annotations, including those on superclasses and mixins and ran into the issue that the synthetic getter/setter method mirrors that wrap variables accessed through InstanceMirror#instanceMembers do not expose metadata. I assume this is a known issue, but the discrepancy is highly confusing and I could not find it documented anywhere.

As far as I understand my only option here is to use ClassMirror#declarations which means walking the class hierarchy manually. In all fairness this is also what I did with raw mirrors but it would be really nice and much less confusing if reflectable could provide the metadata for all instance members. At least is should be documented.

The following program illustrates my issues:

import 'package:reflectable/reflectable.dart';

class ReflectableClass extends Reflectable {
  const ReflectableClass()
      : super(const InvokingMetaCapability(MyAnnotation), metadataCapability,
            declarationsCapability);
}

class MyAnnotation {
  const MyAnnotation();
}

@ReflectableClass()
class Foo {
  @MyAnnotation()
  var kittens;

  @MyAnnotation()
  get puppies {}
}

@ReflectableClass()
class Bar extends Foo {
  @MyAnnotation()
  var ponies;

  @MyAnnotation()
  get unicorns {}
}

void main() {
  final barMirror = const ReflectableClass().reflectType(Bar);
  print('bar instance members:');
  barMirror.instanceMembers.forEach(_print);

  print('\nbar declarations:');
  barMirror.declarations.forEach(_print);
}

_print(name, mirror) =>
    print('\t${mirror.runtimeType} on $name metadata: ${mirror.metadata}');

Output:

bar instance members:
    ImplicitGetterMirrorImpl on kittens metadata: []
    ImplicitSetterMirrorImpl on kittens= metadata: []
    MethodMirrorImpl on puppies metadata: [Instance of 'MyAnnotation']
    ImplicitGetterMirrorImpl on ponies metadata: []
    ImplicitSetterMirrorImpl on ponies= metadata: []
    MethodMirrorImpl on unicorns metadata: [Instance of 'MyAnnotation']

bar declarations:
    VariableMirrorImpl on ponies metadata: [Instance of 'MyAnnotation']
    MethodMirrorImpl on unicorns metadata: [Instance of 'MyAnnotation']

@eernstg eernstg self-assigned this Mar 1, 2016
@eernstg
Copy link
Collaborator

eernstg commented Mar 1, 2016

Right, we consider the synthetic getters/setters to have no metadata. However, it would be trivial to change ImplicitAccessorMirrorImpl to have List<Object> get metadata => _variableMirror.metadata rather than List<Object> get metadata => <Object>[], and not much worse for pre-transformed code. I'll think about it, but most likely there is no problem with that change, and then I'll do it later this week.

@lassedamgaard
Copy link
Author

That would be awesome Erik. Just out of curiosity, could you describe why they're considered to not have metadata? It sounds like you made a conscious choice to omit it.

@eernstg
Copy link
Collaborator

eernstg commented Mar 2, 2016

They don't have metadata because metadata arises when a declaration is syntactically associated with a phrase on the form @ <qualified> (. <identifier>)? (<arguments>)?)*; synthetic getters/setters have no syntax, so they are not syntactically associated with anything, including anything on that form. ;-)

But it still makes sense to say that there are no benefits in insisting on such a strict interpretation (your examples illustrate that quite well), so we might as well deliver the metadata of the associated variable declaration. It is still possible for programmers to check isSynthetic and ignore the metadata, if they prefer the strict interpretation.

@lassedamgaard
Copy link
Author

That makes sense. For the record I just implemented this using ClassMirror#declarations which considering how trivial that is, is probably preferable if - as in my case - the distinction between properties and variables matters.

@eernstg eernstg added the wontfix label Mar 9, 2016
@eernstg
Copy link
Collaborator

eernstg commented Mar 9, 2016

Given that the model is nicely consistent today ("no syntax means no metadata"), and given that this model seems to work reasonably well in practicel, I'll change this to wontfix and close it.

@eernstg eernstg closed this as completed Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants