Constants reviver #182
Constants reviver #182
Conversation
| mapMap(duration30s.namedArguments, | ||
| value: (_, v) => new ConstantReader(v).anyValue), | ||
| { | ||
| 'seconds': 30, |
lib/src/revive.dart
Outdated
|
|
||
| /// Returns a revivable instance of [object]. | ||
| /// | ||
| /// Optionally specify the [clazz] type that contains the constructor. |
There was a problem hiding this comment.
Document the behavior if clazz isn't provided?
Are we 99.9% sure we're not going to add more arguments here? Do we really like this to be optional-positional instead of optional-named?
lib/src/revive.dart
Outdated
| import 'utils.dart'; | ||
|
|
||
| /// Base interface for a meta-analyzed [DartObject]. | ||
| abstract class Revivable {} |
There was a problem hiding this comment.
Weird to have an empty base class w/ no members – and one implementation?
| }); | ||
| }); | ||
|
|
||
| group('Reviable', () { |
lib/src/revive.dart
Outdated
| DartObject object, | ||
| ClassElement clazz, | ||
| ) { | ||
| final invocation = (object as DartObjectImpl).getInvocation(); |
There was a problem hiding this comment.
getInvocation may return null – need to handle that!
|
@kevmoo PTAL, lots of changes. |
matanlurey
force-pushed
the
constants-reviver
branch
from
645be2e to
5266aa1
Compare
| _object.toIntValue() ?? | ||
| _object.toStringValue() ?? | ||
| _object.toListValue() ?? | ||
| _object.toMapValue(); |
There was a problem hiding this comment.
You're missing symbol here – it's supported on DartObject
You'll just want to wrap it in new Symbol if it returns a String.
| library ??= object.type.element.library; | ||
| var url = Uri.parse(urlOfElement(object.type.element)); | ||
| final clazz = object?.type?.element as ClassElement; | ||
| for (final e in clazz.fields.where( |
There was a problem hiding this comment.
why does this need to be a for loop if the first line is a return? Is this a typical pattern to handle the 0 or first case?
would this be more readable as
var matchingFields = clazz.fields.where((f) => ...);
if (matchingFields.isNotEmpty) {
return new Revivable._(source: url, accessor: matchinfFields.first.name);
}
There was a problem hiding this comment.
Or just firstWhere – or even better – singeWhere – assuming more than one would be a bug
| /// | ||
| /// Optionally specify the [library] type that contains the reference. | ||
| /// | ||
| /// Returns [null] if not revivable. |
There was a problem hiding this comment.
Do we have known cases we know won't work? Could we expand this?
4 participants
Partial support of #178.
Still missing as of this PR:
enum, though it should work out of the box anyway.nullwhen something is not resolvable, ideally I'd like to either throw or return a specialized instance (Unrevivable) that explains why not (usually due to a private constructor, or not being able to trace how the object was createdGood starting point though I think.