Open pattobrien opened 1 year ago
I suspect that 'package:flutter/material.dart' is treated as a special case by the analyzer, and hence there is no access to the declarations in that library.
I'm not sure why that would be necessary, but other platform libraries behave in a similar manner.
I can't immediately experiment with the situation as described:
> flutter pub get
Because every version of flutter_test from sdk depends on source_span 1.9.0 and every version of reflectable_analyzer from path depends on source_span ^1.9.1, flutter_test from sdk is incompatible with reflectable_analyzer from path.
...
Did you get that error, too? Do you have an easy fix for it?
Did you get that error, too? Do you have an easy fix for it?
I assume you're using an older version of the Flutter SDK than I am.. I just pushed a change to the main branch that lowers the source_span
dependency to 1.8.2, so you should be good to try it out now.
I suspect that 'package:flutter/material.dart' is treated as a special case by the analyzer, and hence there is no access to the declarations in that library. I'm not sure why that would be necessary, but other platform libraries behave in a similar manner.
I just tried a couple more use cases ('Provider' from riverpod, 'Directory' from dart:io
) and only dart:xyz
uris and qualified-names are resolving properly, so you're right and I think there's some sort of whitelist on dart-sdk packages. You can see the latest commit for that too.
That said, I'm not 100% familiar with the implementation details of reflectable, but afaik the analyzer simply needs to be pointed to paths from which to create an AnalysisContextCollection and perform analysis. I would assume it's the responsibility of package:build
to get the Flutter SDK directory from the target package's package_config.json
, and forward the SDK path to the analyzer, from which we can inspect the Flutter Types and their respective source URIs.
I would therefore suspect that the issue lies either in reflectable's implementation or (more likely) package:build
, not with analyzer, but of course this is just speculation. Maybe there's even some build configuration that can whitelist Flutter?
I appreciate your help looking into this!
Thanks!
@jakemac53, does it ring a bell that 'package:flutter/material.dart' may appear to be non-existing during code generation?
Build scripts (and thus builders) can't import (even transitively) things from dart:ui, but they should be available to the analyzer, and we have some basic tests to that effect, but they only test dart:ui
and not package:flutter
.
I am not sure why package:flutter wouldn't work.
Okay, thanks for that info. I may have found a root cause:
Debugging the build script of a flutter-based package shows that visibleLibraries contains several hundred libraries, including dart.ui
and Flutter-specific libraries like material
. So I think we can say for sure that builders are able to access Flutter libraries, and that the issue likely resides in reflectable
.
On closer look, only ~25 of these 600+ LibraryElement
s have a name
value (such as dart.ui
or material
). In fact, the file in which Container
is defined (flutter/src/widgets/container.dart
) is one of many files that does not have an explicit library name defined.
I assume that reflectable may be generating each Type's qualified name and library uri solely based on the library name, which could explain why it fails to output a URI for Container (or any other Flutter type for that matter). I would expect that the source URI would be used throughout the generated code instead of library name, since library names are rarely defined and source URIs are always available. wdyt?
@pattobrien:
About the version of flutter: The first thing I did was updating, yielding this:
Flutter 3.3.9 • channel stable • https://github.com/flutter/flutter.git
With the new commit on reflectable_flutter_analyzer
, I just needed to change reflectable: 4.0.4
to reflectable: any
(using reflectable 3.0.10), and I can now run the builder locally, and it produces this:
r.NonGenericClassMirrorImpl(
r'Container',
r'.Container',
134217735,
1,
const prefix0.AnalyzerReflector(),
const <int>[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 36, 37, 49],
...
<Type>[
prefix1.Color,
prefix2.Container,
int,
prefix5.Widget,
prefix6.AlignmentGeometry,
prefix7.EdgeInsetsGeometry,
prefix8.Decoration,
prefix9.BoxConstraints,
....
This shows that the qualifiedName
of the class Container
is .Container
. This is correct in the case where the authors of the enclosing library have chosen to omit the library
name declaration, and it just means that you get a smaller amount of help when trying to disambiguate references to the library (including: specifying the qualified name of any declaration therein).
We have some library mirrors, too:
<m.LibraryMirror>[
r.LibraryMirrorImpl(
r'dart.ui',
Uri.parse(r'reflectable://0/dart.ui'),
const prefix0.AnalyzerReflector(),
const <int>[],
{},
{},
const [],
null),
r.LibraryMirrorImpl(
r'',
Uri.parse(r'reflectable://1/'),
const prefix0.AnalyzerReflector(),
const <int>[],
{},
{},
const [],
null)
],
Again, the first constructor argument is the simpleName
, and it's based on the library
name declaration, using the empty string if there is no such declaration.
So everything seems to work as expected, except that the URIs do not look like the ones we have in an import
directive. They are just specifying enough information to make the distinction: They contain the library index (that makes them unique), and then they contain the library name, if any.
The reason for this is that a LibraryElement
in the analyzer does not have a uri
property (or anything similar), and this is probably because libraries can be in-memory entities (and there may not exist any package:...
or similar URI which will resolve to the relevant file).
On closer look, only ~25 of these 600+ LibraryElements have a name value (such as dart.ui or material). In fact, the file in which Container is defined (flutter/src/widgets/container.dart) is one of many files that does not have an explicit library name defined.
Yes, most libraries omit the library
name declaration. This is inconvenient when a piece of Dart code needs to refer to a library as a first-class entity, but libraries are only first-class entities in a situation where the program uses some kind of reflection, and the community as a whole does not want to provide this service
So we may not be able to find the URI which is used in an import
directive—and that would be a set of URIs, anyway, because you can find the same file using many different URI.
I assume that reflectable may be generating each Type's qualified name and library uri solely based on the library name, which could explain why it fails to output a URI for Container (or any other Flutter type for that matter).
Not solely the library name, because that wouldn't be unique, so you wouldn't be able to distinguish different libraries from each other. But the library name is included (basically, to improve the readability in the cases where there is a library name).
I would expect that the source URI would be used throughout the generated code instead of library name, since library names are rarely defined and source URIs are always available. wdyt?
As mentioned, the source URI is not a unique string for each library.
It would be very nice, however, if we could deliver a usable URI which would resolve to the relevant disk file. I'm not quite sure how that could be done.
There is one way to get something which can be used to compute a 'package' URI, at least in some cases:
String assetId;
try {
assetId = (await _resolver.assetIdForElement(library)).toString();
} catch (_) { assetId = '<Unresolvable>'; }
which yields
test_reflectable|test/type_variable_test.dart
for a library which can be imported using
import 'package:test_reflectable/test/reflect_type_test.dart';
Reflectable could be extended with a new instance variable in each LibraryMirror
, and it could contain a package URI computed from this AssetId
(if it is resolvable, and, e.g., 'dart:core' isn't).
As usual, there's a trade-off: Do we wish to spend more space on the generated code in order to get this feature? Is it going to be controlled by a capability? What should we do if a library mirror is requested for a given library, but that library has an unresolvable AssetId
?
For the specific use case I had in mind (identifying if a runtime DartType is equivalent to a compile-time Type), I was thinking something more along the lines of identifying a Type by the exact file that its declared in (not by export files, for reasons you alluded at re: multiple export files). This would even allow for any edge cases where a single package defines two different Container types.
For example, the type Container
would have a URI of: package:flutter/src/widgets/container.dart#Container
.
In an early prototype of sidecar, I used build_runner to generate such URIs and was able to reliably use them to check types during runtime.. This was inspired by how source_gen
's TypeChecker.fromUrl works, which uses dart:mirrors
behind the scenes.
Ultimately, I would expect this URI be generated for each TypeMirror
; I'm not familiar enough of LibraryMirror
use cases to make an informed decision on if a similar URI-based scheme is needed there as well (my use case doesn't seem to need it, at least).
(identifying if a runtime DartType is equivalent to a compile-time Type)
That should be easy for non-generic classes:
void main() {
dynamic d = 1; // We've completely forgotten the type of this object.
Type runtimeType = d.runtimeType;
if (runtimeType == int) { // Test whether the run-time type is `int`.
...
}
}
If your design relies on getting one or more of those Type
objects from a mirror (that would be a TypeMirror
, probably a ClassMirror
, possibly obtained as the type
of an InstanceMirror
), then you could take a look at reflectedTypeCabability
, cf. https://github.com/google/reflectable.dart/blob/master/test_reflectable/test/reflected_type_test.dart. With that capability, type mirrors will carry a reflectedType
'Equivalent' types in this case are defined here. Note that dynamic
and void
and Object?
are considered to be distinct types even though they contain the same objects (that is, all objects).
For generic types, the value of the reflectedType
of a class mirror is the raw type (so for the class List
it would be List
considered as a type literal, which is the same thing as List<dynamic>
).
I have no idea how you are using these type equivalence tests, but reflectedType
(or indeed plain Type
instances obtained directly without any use of reflection) could be much simpler than going via URIs.
That should be easy for non-generic classes:
In your example, you're testing equality between a Type
and a Type
. The issue is that the analyzer package speaks in terms of a DartType
via the staticType of certain AstNodes, which is an object that includes the type's name and source URI (but doesn't give access to a Type
object!).
This line in this example AstVisitor class (from branch: feat/flutter_type_checkers) is able to reflect the type Color
, since reflectable gives us the 'dart.ui.Color' library ID to work with, and we can then compare to it to the library ID of the DartType
given by the analyzer ASTNode - therefore we're able to compare the DartType
object of a Color
with the Type
of a Color
100% correctly, in order to determine that some analyzed code does in fact match a Color type.
However, that same example code will fail at the Container
line, since reflectable doesnt give us a library ID or a source URI to compare to, and therefore we have no way of determining if a certain DartType
is equal to our Container
Type
(besides comparing the objects' names, which is of course prone to errors if two or more packages or files declare the type Container
).
Since DartType
and Type
are two completely different references and objects (one from runtime, one from analyzer AST output), we cannot directly compare the objects. AFAIK the only way to do so in a robust way is by comparing the name of the object and the librarySource uri of the two objects (which is what package source_gen
does via TypeChecker.fromUrl
and TypeChecker.fromRuntime
). And since we cant simply get the URI of any Type instance at runtime, I'm looking to do so at compile-time using reflectable.
Hope this helps clear things up! :)
In your example, you're testing equality between a
Type
and aType
.
Yes, I couldn't see any other way to map
(identifying if a runtime DartType is equivalent to a compile-time Type)
to the situation where we're performing some run-time type equivalence test (whether or not this is done using reflectable).
the analyzer package speaks in terms of a
DartType
... since we cant simply get the URI of any Type instance at runtime, I'm looking to do so at compile-time using reflectable.
OK, so we're talking about compile-time, not run time. Interesting!
But this sounds like the code generated by reflectable is used with the target program at all?:
Very interesting! :smile:
But reflectable wasn't created in order to support this scenario, so it may or may not work very smoothly.
I assume that your use case is checking flutter programs in relation to their use of Flutter declared entities (that is, classes and other declared entities that are "owned" by Flutter), possibly in order to emit diagnostic messages, possibly in order to generate code, or whatever, but in any case in order to work on the client program which is being analyzed by the analyzer.
This also means that you'd (probably) use reflectable to generate the code for a specific version of Flutter, and then you wouldn't use reflectable at all with the 'main library' of the client program as the entry point.
And then you want to re-connect the Analyzer representation of a given declared entity with a mirror which was generated by reflectable.
Sounds like the approach using AssetId
to add a URI in some new instance variable of LibraryMirror
could actually be helpful.
The upcoming release 4.0.5 contains code to obtain and use a URI from the associated AssetId
of each library, in the case where this information is available, cf. https://github.com/google/reflectable.dart/pull/310.
Hi all -
I'm creating an
analyzer
based utility package, to more-easily allow developers to create lint rules, quick assists, etc, and one of the individual proof-of-concepts I'm designing is a more intuitive API for interacting with AST utilities like type-checkers. For more context, you can check out sidecar (the aforementioned analyzer utility) and reflectable_flutter_analyzer (attempted PoC of a Flutter-based analyzer that uses reflectable package).As the title explains, I'm unable to reflect
flutter/material.dart
type URIs usingpackage:reflectable
, which are needed to perform runtime-checking of a URI match between an analyzer Element's source URI and the expected URI of the Type (e.g. Container). I have this working fordart.ui.Color
but it does not work forContainer
frompackage:flutter
:The above code incorrectly generates the following: