Cannot reflect `flutter/material.dart` URIs for `Container` type #305

pattobrien · 2022-12-12T15:21:40Z

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 using package: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 for dart.ui.Color but it does not work for Container from package:flutter:

import 'package:flutter/material.dart';

import 'package:reflectable_analyzer/reflectable_analyzer.dart';

@GlobalQuantifyCapability(r'\.Color$', analyzerReflector)
@GlobalQuantifyCapability(r'\.Container$', analyzerReflector)
import 'package:reflectable/reflectable.dart' hide SourceLocation;

import 'reflector.reflectable.dart';

void main() {
  initializeReflectable();
}

The above code incorrectly generates the following:

...
      <m.LibraryMirror>[
        r.LibraryMirrorImpl( 
            r'dart.ui', // Correct library mirror for ```dart.ui.Color```
            Uri.parse(r'reflectable://0/dart.ui'),
            ...
        ),
        r.LibraryMirrorImpl(
            r'',  // Blank library mirror for ```flutter.material.Container```
            Uri.parse(r'reflectable://1/'),
            ...
        ),
      ],
...

 <m.TypeMirror>[
        r.NonGenericClassMirrorImpl(
            r'Color',
            r'dart.ui.Color', // Correct qualified name
            ...
        ),
       r.NonGenericClassMirrorImpl(
            r'Container',
            r'.Container', // Expected ```flutter.material.Container``` or something similar
            ...
       ),

The text was updated successfully, but these errors were encountered:

eernstg · 2022-12-13T18:16:23Z

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?

pattobrien · 2022-12-13T19:00:09Z

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!

eernstg · 2022-12-13T19:19:15Z

Thanks!

@jakemac53, does it ring a bell that 'package:flutter/material.dart' may appear to be non-existing during code generation?

jakemac53 · 2022-12-13T21:30:36Z

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.

pattobrien · 2022-12-14T03:07:47Z

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+ 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.

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?

eernstg · 2022-12-14T11:21:50Z

@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.

eernstg · 2022-12-14T13:07:32Z

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?

pattobrien · 2022-12-14T14:22:13Z

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).

eernstg · 2022-12-14T17:31:27Z

(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.

pattobrien · 2022-12-14T20:01:41Z

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! :)

eernstg · 2022-12-15T12:10:26Z

In your example, you're testing equality between a Type and a Type.

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?:

Background: The reflectable code generation is invoked with a particular library L as its 'entry point', and it will use the transitive import closure of L as the set of libraries which are considered during code generation. In other words, reflectable is capable of working with all the code in these libraries, and no code at all outside this set of libraries. So if we have a mirror then it's a mirror of something which is declared in this set of libraries.
Returning to the topic: It sounds like L doesn't import the code which is generated by reflectable at all (or we don't care), the real purpose of the generated code is to be imported by a program which will check a lint (or many), cf. https://github.com/pattobrien/reflectable_flutter_analyzer/blob/a77557c92b2c111b5306655a8ca5e8304e538115/packages/design_system_lints/lib/src/colored_box.dart.

Very interesting! 😄

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.

eernstg · 2023-02-13T14:56:07Z

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. #310.

This PR deletes the legacy tests (they contain non-null-safe code and cannot be executed in the future). The PR also adjusts the SDK bounds in order to prepare for Dart 3. The update revealed that one test, 'test_reflectable/test/new_instance_native_test.dart', performed a reflective invocation of the list constructor named `List`, which is no longer supported. The test was adjusted to call another constructor of `List`. Finally, this PR changes the computation of the `uri` of a `LibraryMirror` such that it uses the URI of the corresponding `AssetId` whenever possible. This should be helpful in the situation where there is a need to find the actual source code, as requested in #305. The change in the result returned by the `uri` getter may seem like a breaking change, but it is hardly breaking in practice because the result returned previously was essentially meaningless (using the scheme reflectable which is not used anywhere else, and consisting mostly of a number which reflected the numbering of libraries being loaded by the analyzer during code generation). The new behavior is tested in the updated version of 'reflect_type_test.dart'.

eernstg added the enhancement label Dec 14, 2022

eernstg mentioned this issue Feb 13, 2023

Upgrade to prepare for Dart 3; use working uris when available #310

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cannot reflect `flutter/material.dart` URIs for `Container` type #305

Cannot reflect `flutter/material.dart` URIs for `Container` type #305

Cannot reflect flutter/material.dart URIs for Container type #305

Cannot reflect flutter/material.dart URIs for Container type #305

Comments

Cannot reflect `flutter/material.dart` URIs for `Container` type #305

Cannot reflect `flutter/material.dart` URIs for `Container` type #305