google / reflectable.dart

Reflectable is a Dart library that allows programmers to eliminate certain usages of dynamic reflection by specialization of reflective code to an equivalent implementation using only static techniques. The use of dynamic reflection is constrained in order to ensure that the specialized code can be generated and will have a reasonable size.
https://pub.dev/packages/reflectable
BSD 3-Clause "New" or "Revised" License
374 stars 56 forks source link

Add support for generic type arguments in annotations #283

Open jacksonjude opened 1 year ago

jacksonjude commented 1 year ago

Overview

Adds support for generic annotations, a feature added in Dart SDK 2.14.

Take this class for example:

@reflector
class Person {
  @TypeHelper<Soul>(42, "The meaning")
  String? life;
}

class TypeHelper<T> {
  const TypeHelper(int var1, String var2);
}

class Soul { }

The annotation would have previously been identified as having the type argument dynamic:

ClassMirror personClassMirror = reflector.reflectType(Person) as ClassMirror;
VariableMirror field = personClassMirror.declarations["life"] as VariableMirror;
var annotation = field.metadata[0];
print(annotation.runtimeType); // TypeHelper<dynamic>

It will now be correctly identified as having the type argument Soul:

print(annotation.runtimeType); // TypeHelper<Soul>

This is done by appending the type arguments (if any exist) after the annotation name during the _extractMetadataCode function of builder_implementation.dart.

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

eernstg commented 1 year ago

Hello @jacksonjude, thanks for the PR! This is something reflectable should support. I'm really busy now, but I'll take a look as soon as possible.

jacksonjude commented 1 year ago

Hey there, sorry if you are still busy, but can you take another look at this?

eernstg commented 1 year ago

Sorry about the delay — yep, I'm still busy, but I will take a look at it tomorrow.

jacksonjude commented 1 year ago

Thanks again for the input! We need to make the code a bit more robust. Here is the beginnings of a test that is intended to verify that the new feature is working:

@c
library test_reflectable.test.metadata_test;

import 'package:reflectable/reflectable.dart';
import 'package:test/test.dart';
import 'metadata_test.reflectable.dart';

class MyReflectable extends Reflectable {
  const MyReflectable()
      : super(metadataCapability, instanceInvokeCapability,
            staticInvokeCapability, declarationsCapability, libraryCapability);
}

const myReflectable = MyReflectable();

const c = [
  Bar<num>({'a': 14})
];

class K {
  static const p = 2;
}

@myReflectable
@Bar<Object?>({
  b: deprecated,
  c: Deprecated('tomorrow'),
  1 + 2: (d ? 3 : 4),
  identical(1, 2): 's',
  K.p: 6
})
@Bar<Never>({})
@Bar<void>.namedConstructor({})
@c
class Foo {
  @Bar<bool>({})
  @Bar<Bar<Bar>>({})
  @Bar<MyReflectable>.namedConstructor({})
  @Bar<void Function()>.namedConstructor({})
  @c
  void foo() {}
  var x = 10;
}

class Bar<X> {
  final Map<Object, X> m;
  const Bar(this.m);
  const Bar.namedConstructor(this.m);

  @override
  String toString() => 'Bar<$X>($m)';
}

void main() {
  initializeReflectable();

  test('metadata on class', () {
    expect(myReflectable.reflectType(Foo).metadata, const [
      MyReflectable(),
      Bar<Object?>({
        b: deprecated,
        c: Deprecated('tomorrow'),
        3: 3,
        false: 's',
        2: 6,
      }),
      [
        Bar<num>({'a': 14})
      ],
    ]);

    var fooMirror = myReflectable.reflectType(Foo) as ClassMirror;
    expect(fooMirror.declarations['foo']!.metadata, const [
      Bar<bool>({}),
      Bar<Bar<Bar<dynamic>>>({}),
      Bar<MyReflectable>({}),
      Bar<void Function()>({}),
      [
        Bar<num>({'a': 14})
      ],
    ]);

    // The synthetic accessors do not have metadata.
    expect(fooMirror.instanceMembers['x']!.metadata, []);
    expect(fooMirror.instanceMembers['x=']!.metadata, []);

    // Test metadata on libraries
    expect(myReflectable.reflectType(Foo).owner!.metadata, [c]);
  });
}

The test probably needs to be corrected here and there (but it already shows that the new code gets a null reference error, so it's a useful starting point).

The test should be added to the repository at test_reflectable/test/generic_metadata_test.dart. The lower bound of the SDK constraint in test_reflectable/pubspec.yaml` should be changed to 2.17 (such that the generic-metadata feature is included in the language).

One more question: the variables b and d don't seem to be defined in the sample test code you have provided. Is this intentional?

eernstg commented 1 year ago

(No, I haven't forgotten this. Just busy again. Will try to get to it real soon now. ;-)

eernstg commented 1 year ago

(Busy, busy, busy, but I will take a look later today.)

codekeyz commented 6 months ago

@eernstg

Sten435 commented 5 months ago

@codekeyz @eernstg Any updates on this ?

It has been a year ?