Added `dynamicReflected..Type`, corrected type expressions.

Added `dynamicReflected..Type`, corrected type expressions.

Added methods `dynamicReflectedType` and `hasDynamicReflectedType` to
class mirrors and type annotation related mirrors (variable, parameter,
and method mirrors), returning the fully dynamic instantiation of the
given [Type] object (e.g., `List` rather than `List<String>`).

Added a new public class `TypeValue` to 'mirrors.dart', such that we
can use expressions like `const TypeValue<List<int>>().type`; this is
in fact convenient in test code as well, and presumably in client code.

Corrected the code that produces type expressions for various things
(esp. values returned by `dynamicReflectedType`) such that it includes
type arguments and uses `TypeValue` when needed.

It is not easy to cover generic types in both pre- and post-transform
code, and the features around reflected types have brought up new
issues: In pre-transform code we cannot obtain `List` from a given
`List<T>` for some `T`, so `dynamicReflectedType` cannot be
implemented in all cases; but since `hasDynamicReflectedType` returns
false in these cases, users can still write reliable code. Similarly,
`reflectedType` cannot be implemented in cases like `List<E>` used
as a type annotation in the class `List`: We do not have the required
primitives to look up `E` in a given list, so we cannot produce a
class mirror and then a variable/parameter mirror that holds the
[Type] value for `E`, so `hasReflectedType` must return false here.
This means that client code must in general test with the `has..`
methods (which was obvious anyway, even if they didn't always do it)
and the `has..` methods may not return the same value in pre- and
post-transform code, which is a new kind of incompatibility.
However, it is a worse kind of incompatibility if pre-transform code
returns `List<String>` and post-transform code returns `List`, and
the behavior changes because an `==` test with `List<String>` now
fails after transformation.

Rewrote the functions named `_typeCode` and similar names
extensively to support these things.

Added a new test for `{has,}dynamicReflectedCode`.

R=sigurdm@google.com

Committed: https://github.com/dart-lang/reflectable/commit/10da7e398ce439314dacfb276e2a58ecbabce5b4

[eernst, 2015-11-26 10:02:00]

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

[eernst, 2015-11-26 10:02:01]

Works with bottest.

Expected extension before landing: Some `bestEffortReflectedType` method that
tries both `reflectedType` and `dynamicReflectedType`. Note that such a method
would _not_ return the same thing in pre-/post-transform code..

[sigurdm, 2015-11-26 10:18:04]

LGTM

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

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:312: Type get reflectedType;
Document conditions for availability.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:345: /// Return true iff this class mirror is
capable of delivering its
Returns

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:346: /// [dynamicReflectedType].
Explain when this is/is not the case.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:349: /// Return the [Type] object representing the
fully dynamic instantiation of
Returns ...

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:355: /// respectively `Map<dynamic, dynamic>`.
Write about the relation to hasDynamicReflectedType, and when it is available.

[eernst, 2015-11-26 11:24:00]

Review response, in particular: Added several longish comments about the
conditions under which `reflected..Type` is available.

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

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:312: Type get reflectedType;
On 2015/11/26 10:18:03, sigurdm wrote:
> Document conditions for availability.

Done.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:345: /// Return true iff this class mirror is
capable of delivering its
On 2015/11/26 10:18:03, sigurdm wrote:
> Returns

Done, fixed 10 occurrences of `/// [verb-not-in-present-tense]` globally.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:346: /// [dynamicReflectedType].
On 2015/11/26 10:18:03, sigurdm wrote:
> Explain when this is/is not the case.

Done.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:349: /// Return the [Type] object representing the
fully dynamic instantiation of
On 2015/11/26 10:18:03, sigurdm wrote:
> Returns ...

Done.

https://codereview.chromium.org/1473073009/diff/1/reflectable/lib/mirrors.dar...
reflectable/lib/mirrors.dart:355: /// respectively `Map<dynamic, dynamic>`.
On 2015/11/26 10:18:03, sigurdm wrote:
> Write about the relation to hasDynamicReflectedType, and when it is available.

Done.

[eernst, 2015-11-26 11:25:30]

Description was changed from

==========
Added `dynamicReflected..Type`, corrected type expressions.

Added methods `dynamicReflectedType` and `hasDynamicReflectedType` to
class mirrors and type annotation related mirrors (variable, parameter,
and method mirrors), returning the fully dynamic instantiation of the
given [Type] object (e.g., `List` rather than `List<String>`).

Added a new public class `TypeValue` to 'mirrors.dart', such that we
can use expressions like `const TypeValue<List<int>>().type`; this is
in fact convenient in test code as well, and presumably in client code.

Corrected the code that produces type expressions for various things
(esp. values returned by `dynamicReflectedType`) such that it includes
type arguments and uses `TypeValue` when needed.

It is not easy to cover generic types in both pre- and post-transform
code, and the features around reflected types have brought up new
issues: In pre-transform code we cannot obtain `List` from a given
`List<T>` for some `T`, so `dynamicReflectedType` cannot be
implemented in all cases; but since `hasDynamicReflectedType` returns
false in these cases, users can still write reliable code. Similarly,
`reflectedType` cannot be implemented in cases like `List<E>` used
as a type annotation in the class `List`: We do not have the required
primitives to look up `E` in a given list, so we cannot produce a
class mirror and then a variable/parameter mirror that holds the
[Type] value for `E`, so `hasReflectedType` must return false here.
This means that client code must in general test with the `has..`
methods (which was obvious anyway, even if they didn't always do it)
and the `has..` methods may not return the same value in pre- and
post-transform code, which is a new kind of incompatibility.
However, it is a worse kind of incompatibility if pre-transform code
returns `List<String>` and post-transform code returns `List`, and
the behavior changes because an `==` test with `List<String>` now
fails after transformation.

Rewrote the functions named `_typeCode` and similar names
extensively to support these things.

Added a new test for `{has,}dynamicReflectedCode`.
==========

to

==========
Added `dynamicReflected..Type`, corrected type expressions.

Added methods `dynamicReflectedType` and `hasDynamicReflectedType` to
class mirrors and type annotation related mirrors (variable, parameter,
and method mirrors), returning the fully dynamic instantiation of the
given [Type] object (e.g., `List` rather than `List<String>`).

Added a new public class `TypeValue` to 'mirrors.dart', such that we
can use expressions like `const TypeValue<List<int>>().type`; this is
in fact convenient in test code as well, and presumably in client code.

Corrected the code that produces type expressions for various things
(esp. values returned by `dynamicReflectedType`) such that it includes
type arguments and uses `TypeValue` when needed.

It is not easy to cover generic types in both pre- and post-transform
code, and the features around reflected types have brought up new
issues: In pre-transform code we cannot obtain `List` from a given
`List<T>` for some `T`, so `dynamicReflectedType` cannot be
implemented in all cases; but since `hasDynamicReflectedType` returns
false in these cases, users can still write reliable code. Similarly,
`reflectedType` cannot be implemented in cases like `List<E>` used
as a type annotation in the class `List`: We do not have the required
primitives to look up `E` in a given list, so we cannot produce a
class mirror and then a variable/parameter mirror that holds the
[Type] value for `E`, so `hasReflectedType` must return false here.
This means that client code must in general test with the `has..`
methods (which was obvious anyway, even if they didn't always do it)
and the `has..` methods may not return the same value in pre- and
post-transform code, which is a new kind of incompatibility.
However, it is a worse kind of incompatibility if pre-transform code
returns `List<String>` and post-transform code returns `List`, and
the behavior changes because an `==` test with `List<String>` now
fails after transformation.

Rewrote the functions named `_typeCode` and similar names
extensively to support these things.

Added a new test for `{has,}dynamicReflectedCode`.

R=sigurdm@google.com

Committed:
https://github.com/dart-lang/reflectable/commit/10da7e398ce439314dacfb276e2a5...
==========

[eernst, 2015-11-26 11:25:31]

Committed patchset #2 (id:20001) manually as
10da7e398ce439314dacfb276e2a58ecbabce5b4 (presubmit successful).