Update reflectable such that test_reflectable can run tests using pub run build_runner test
#119
Conversation
… build_runner test`
Also removed unused imports.
…generator input file
…er generalization
| @@ -835,7 +852,7 @@ class _ReflectorDomain { | |||
| classDomain._instanceMembers.forEach(members.add); | |||
|
|
|||
| // Add all the formal parameters (as a single, global set) which | |||
| // are declared by any of the methods in `classDomain._declarations` | |||
| // are declared by any of the methods in `classDomain._declarations` | |||
| @@ -856,8 +873,7 @@ class _ReflectorDomain { | |||
| } else if (variable is TopLevelVariableElement) { | |||
| topLevelVariables.add(variable); | |||
| } else { | |||
| throw unimplementedError( | |||
| "This kind of variable is not yet supported"); | |||
| logger.error("This kind of variable is not yet supported"); | |||
There was a problem hiding this comment.
Can you add the typename to the error message for easier diagnostics?
| // Requested: just the name of the typedef; get it and return. | ||
| int dartTypeNumber = typedefs.containsKey(dartType) | ||
| ? typedefs[dartType] | ||
| : typedefNumber++; |
There was a problem hiding this comment.
Could you use typedefs.size instead of typedefNumber?
There was a problem hiding this comment.
Resolved IRL: Could not do that due to ordering conflicts in the collection of information about typedefs.
| typeVariablesInScope = new Set<String>(); | ||
| } | ||
| for (TypeParameterElement element in dartType.typeFormals) { | ||
| typeVariablesInScope.add(element.name); |
There was a problem hiding this comment.
Nit: You can use Set.addAll
| } else { | ||
| // Requested: the spelled-out generic function type; continue. | ||
| if (typeVariablesInScope == null) { | ||
| typeVariablesInScope = new Set<String>(); |
There was a problem hiding this comment.
Nit: Could be
typeVariablesInScope ?= new Set<String>();
| importCollector, typeVariablesInScope, typedefs, logger, | ||
| useNameOfGenericFunctionType: useNameOfGenericFunctionType); | ||
| String typeArguments = ""; | ||
| if (!dartType.typeFormals.isEmpty) { |
There was a problem hiding this comment.
Nit: we have Set.isNotEmpty
There was a problem hiding this comment.
Fixed several similar occurrences.
| _capabilities, constructorName, _getEvaluatedMetadata(metadata)); | ||
| Resolver resolver, | ||
| TransformLogger logger) { | ||
| var result = _supportsNewInstance(_capabilities, constructorName, |
There was a problem hiding this comment.
Nit: Could you just
return _supportsNewInstance(...);
?
There was a problem hiding this comment.
Done (was an leftover from printf debugging).
| @@ -2596,22 +2773,26 @@ class _Capabilities { | |||
| bool supportsInstanceInvoke( | |||
| String methodName, | |||
| Iterable<ElementAnnotation> metadata, | |||
| Iterable<ElementAnnotation> getterMetadata) { | |||
| Iterable<ElementAnnotation> getterMetadata, | |||
| TransformLogger logger) { | |||
| var v = _supportsInstanceInvoke( | |||
There was a problem hiding this comment.
Nit: Could you just
return _supportsInstanceInvoke(...);
?
| @@ -3774,6 +3973,7 @@ initializeReflectable() { | |||
| if (const bool.fromEnvironment("reflectable.pause.at.exit")) { | |||
| _processedEntryPointCount++; | |||
| } | |||
| _resolver.release(); | |||
There was a problem hiding this comment.
Could we wrap this code in a local function, so we only have to release the resolver in one spot?
There was a problem hiding this comment.
Reorganized the code without abstractions, because they seemed to require a non-local return. See https://dartpad.dartlang.org/e6351d655334455c511288345c5fbaec for details.
One rather voluminous change in this PR is about function types: Until now, reflectable was not aware of inline function types (like an inline
int Function(), as opposed to the ones which were defined indirectly defined via atypedef). This PR introduces support for the new function type syntax.There is one outstanding issue: We cannot recognize generic function types, because they must be defined in terms of a
typedef, and the correspondingTypeobjects are not equal even though two giventypedefs define the same generic function type. However, it is still possible to use generic function types if they are defined via atypedefand the client is careful to always use the sametypedeffor the same function type. This will be fixed, but this is a change toTypeand not a change to reflectable.A number of other changes are associated with the "administration" of the package, e.g.,
.gitignorewas updated to match the new paths used by Dart tools for generated files, CHANGELOG.md and README.md were updated, and so was pubspec.yaml. Changes tobuild.yamlwere necessary in order to get the desired behavior of the build package (which handles code generation and accepts the reflectable code generator as a kind of plugin).