New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyzer: I need the user visible import URL for a type #31556

Open
matanlurey opened this Issue Dec 6, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@matanlurey
Contributor

matanlurey commented Dec 6, 2017

Problem

Internally we have a dependency injection package that uses code generation.

Basically it looks at the following pattern:

import 'dart:io';

import 'package:some_di/some_di.dart';

import 'app.g.dart' as generated;

@module
class HttpModule {
  @provide
  HttpClient provideHttpClient() => new HttpClient();
}

abstract class AppInjector {
  factory AppInjector() = generated.$AppInjector;
}

And generates some boilerplate code like so:

import 'dart:_http';
import 'app.dart';

class $AppInjector implements AppInjector {
  HttpClient provideHttpClient() => new HttpClient();
}

See the problem? It has "resolved" HttpClient to belonging to dart:_http, which is not a user-importable library. This is because when we ask for the library URL of an Element, we get back the absolute location - which is good for canonicalization, but bad for generating valid code, in this case.

What is the best way to get back dart:io, instead, using the analyzer API?

@bwilkerson

This comment has been minimized.

Show comment
Hide comment
@bwilkerson

bwilkerson Dec 6, 2017

Member

@matanlurey I don't believe that analyzer currently has an API for that. The analysis server has the same problem when trying to import as part of a quick fix (see https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart#L1318 for one example of from server).

@jcollins-g I believe that DartDoc has solved this problem. Any chance that we could move that API into analyzer for others to use?

Member

bwilkerson commented Dec 6, 2017

@matanlurey I don't believe that analyzer currently has an API for that. The analysis server has the same problem when trying to import as part of a quick fix (see https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart#L1318 for one example of from server).

@jcollins-g I believe that DartDoc has solved this problem. Any chance that we could move that API into analyzer for others to use?

@jcollins-g

This comment has been minimized.

Show comment
Hide comment
@jcollins-g

jcollins-g Dec 6, 2017

Contributor

TL;DR: Yes, dartdoc has solved this problem, and no, it is unlikely we could move that API quickly.

@bwilkerson @matanlurey Yes, this is the essence of dartdoc-style canonicalization, which is a different animal from canonicalization as the analyzer thinks of it. There are two versions of this problem: the easy version, and the hard version.

The easy version of the problem Dartdoc has always had a solution for, with varying degrees of accuracy. It goes like this: If the symbol is named in such a way that it is private or it is in a private context (e.g. the private 'dart:_http' library or under 'lib/src'), that's not the Official Version and so ignore it. If a public context exists for it, that's the official version (and just accept that there might be more than one). For the SDK, that's reasonably easy to determine with SdkLibrary.isDocumented. For other packages we can detect 'lib/src' subdirectories. Underscore prefixes help with the rest. While dartdoc's object model (see below) makes this somewhat simpler to code there, it's probably possible to hack this in to analyzer. It just won't be a port. Your example is probably fixable with the easy version.

The hard version is what I am usually talking about with dartdoc, and has been a roughly year-long process to get right. That is, to the easy version, add import-export graph and inheritance tree traversals to determine which one to use when there is more than a single public candidate, falling back to a scoring algorithm and finally, hinting if the graph is ambiguous. This is necessary for packages like Flutter and Angular which have public libraries reexporting each others' content, but still want to have a concept of one of them "owning" the object.

The hard version is more dependent on Dartdoc's object model. Briefly, dartdoc keeps a ModelElement for every single perspective a symbol is visible from (a unique Class object for every Library it is reexported in, a unique Method object for every class in which it is inherited (and therefore even more since classes are expanded as above, and so on). It does this so that when documenting, we can note that from class Foo, hashCode is inherited from Object. We also calculate whether the symbol is private based on the particular perspective represented by the ModelElement. Canonicalization is therefore implemented by having individual ModelElements calculate which other ModelElement is canonical for the analyzer element they share in common. Dartdoc essentially undoes analyzer's canonicalization and then builds an entirely new one.

Without that object model and the code used to build it, I'm not sure that the canonicalization API would be possible to port over. It'd more likely be a rewrite and I haven't thought about it enough to know how to do it. I'd like to evolve dartdoc's design to be more amenable to moving this over, but that's also not a small amount of work.

Contributor

jcollins-g commented Dec 6, 2017

TL;DR: Yes, dartdoc has solved this problem, and no, it is unlikely we could move that API quickly.

@bwilkerson @matanlurey Yes, this is the essence of dartdoc-style canonicalization, which is a different animal from canonicalization as the analyzer thinks of it. There are two versions of this problem: the easy version, and the hard version.

The easy version of the problem Dartdoc has always had a solution for, with varying degrees of accuracy. It goes like this: If the symbol is named in such a way that it is private or it is in a private context (e.g. the private 'dart:_http' library or under 'lib/src'), that's not the Official Version and so ignore it. If a public context exists for it, that's the official version (and just accept that there might be more than one). For the SDK, that's reasonably easy to determine with SdkLibrary.isDocumented. For other packages we can detect 'lib/src' subdirectories. Underscore prefixes help with the rest. While dartdoc's object model (see below) makes this somewhat simpler to code there, it's probably possible to hack this in to analyzer. It just won't be a port. Your example is probably fixable with the easy version.

The hard version is what I am usually talking about with dartdoc, and has been a roughly year-long process to get right. That is, to the easy version, add import-export graph and inheritance tree traversals to determine which one to use when there is more than a single public candidate, falling back to a scoring algorithm and finally, hinting if the graph is ambiguous. This is necessary for packages like Flutter and Angular which have public libraries reexporting each others' content, but still want to have a concept of one of them "owning" the object.

The hard version is more dependent on Dartdoc's object model. Briefly, dartdoc keeps a ModelElement for every single perspective a symbol is visible from (a unique Class object for every Library it is reexported in, a unique Method object for every class in which it is inherited (and therefore even more since classes are expanded as above, and so on). It does this so that when documenting, we can note that from class Foo, hashCode is inherited from Object. We also calculate whether the symbol is private based on the particular perspective represented by the ModelElement. Canonicalization is therefore implemented by having individual ModelElements calculate which other ModelElement is canonical for the analyzer element they share in common. Dartdoc essentially undoes analyzer's canonicalization and then builds an entirely new one.

Without that object model and the code used to build it, I'm not sure that the canonicalization API would be possible to port over. It'd more likely be a rewrite and I haven't thought about it enough to know how to do it. I'd like to evolve dartdoc's design to be more amenable to moving this over, but that's also not a small amount of work.

@matanlurey

This comment has been minimized.

Show comment
Hide comment
@matanlurey

matanlurey Dec 7, 2017

Contributor

I don't need this immediately, as I have workarounds.

This is something good to keep in mind for cleanup work after the CFE, however. I'll update you if this ends up needing a higher priority, for now it's ~P2.

Contributor

matanlurey commented Dec 7, 2017

I don't need this immediately, as I have workarounds.

This is something good to keep in mind for cleanup work after the CFE, however. I'll update you if this ends up needing a higher priority, for now it's ~P2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment