handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153

Hixie · 2016-04-28T22:15:26Z

Sometimes, a library, like the Flutter rendering library, wants to refer to another library, like the Flutter widgets library, in the documentation, but doesn't want to introduce a dependency in the code. In this case, for example, we definitely do not want any sort of runtime relationship from the rendering library to the widgets library.

We can't use import for this because it risks code accidentally depending through the import.

One possible solution would be to have something like /// @import which would cause the dartdoc code to import it. Another would be for the dartdoc tool to be able to receive a list of imports to imply at the top of every file. I don't really mind how we solve this.

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

pq · 2016-05-19T20:28:11Z

cc @keertip for thoughts.

pq · 2016-05-19T20:43:21Z

Here's an example as food for thought from rendering.dart:

/// The Flutter rendering tree.
/// 
/// To use, import `package:flutter/rendering.dart`.
///
/// The [RenderObject] hierarchy is used by the Flutter Widgets
/// library to implement its layout and painting back-end. Generally,
/// while you may use custom [RenderBox] classes for specific effects
/// in your applications, most of the time your only interaction with
/// the [RenderObject] hierarchy will be in debugging layout issues.
///
/// If you are developing your own library or application directly on
/// top of the rendering library, then you will want to have a binding
/// (see [BindingBase]). You can use [RenderingFlutterBinding], or you
/// can create your own binding. If you create your own binding, it
/// needs to import at least [SchedulerBinding], [GestureBinding],
/// [ServicesBinding], and [RendererBinding]. The rendering library
/// does not automatically create a binding, but relies on one being
/// initialized with those features.

lints to:

[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 13, col 10)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 17, col 11)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 17, col 39)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 19, col 31)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 19, col 51)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 20, col 6)
[lint] Only reference in scope identifiers in doc comments. (packages/flutter/lib/rendering.dart, line 20, col 29)

devoncarew · 2016-07-27T02:47:19Z

The latest thinking on this is to support something like fully qualified symbol references in square brackets (dartdoc specific imports might get a little hairy). Something like [widgets.Draggable] or [package:flutter/widgets Draggable].

/cc @pq @bwilkerson

pq · 2016-07-27T05:37:28Z

👍 I'm in favor in taking the cue from Java's qualified names.

Curious where you weigh in @bwilkerson ?

Hixie · 2016-07-28T20:19:06Z

I could totally work with fully-qualified identifiers in square brackets (assuming they don't render that way, at least).

pq · 2016-07-28T20:44:09Z

@bwilkerson, @scheglov : how does this proposal sit with you?

scheglov · 2016-07-28T21:26:28Z

I like the idea of fully qualified names.

eseidelGoogle · 2016-08-18T05:07:56Z

FWIW this came up again tonight. https://docs.flutter.io/flutter/material/Icons-class.html Icon and IconButton (the classes which consume the data in icons.dart) can't be linked since they're not imported.

pq · 2016-08-18T05:14:19Z

@scheglov : am I right that this wouldn't be too hard to add?

scheglov · 2016-08-18T05:17:24Z

Yes, I think it wouldn't be too hard.

eseidelGoogle · 2016-08-18T05:33:52Z

In case it's helpful, here is the output of running flutter analyze with the comment_references lint on. Currently its disabled with a comment saying it's blocked by this bug:
https://gist.github.com/eseidelGoogle/618570930fb1752e0f5bc7afa29aae91

I don't think this is a huge deal by any means, just wanted you to have full context.

qchong · 2016-09-27T21:33:36Z

We're also getting feedback from the UX team that the missing links in the Flutter API docs is a usability problem.

@scheglov Ping. Is there a plan to fix this issue?

scheglov · 2016-09-27T23:52:49Z

I don't have any specific plans about fixing this issue yet.

sethladd · 2016-10-04T22:13:46Z

If we fix this, we'll probably also help crossdart.info link to more names. Right now, I can't link to Widget from Button's crossdart page: https://www.crossdart.info/p/flutter/0.0.21/src/material/button.dart.html

I'd imagine that Widget would be a link.

Hixie · 2016-10-04T22:40:35Z

That last thing seems like a separate bug. The code knows what Widget it so the hyperlink should work. This bug is just about hyperlinks from dartdocs themselves to identifiers that are otherwise not in scope.

sethladd · 2016-12-01T16:32:46Z

I think @devoncarew and @keertip have an idea of how to handle this? Comments here can help speed up implementation :)

devoncarew · 2016-12-01T16:57:16Z

I believe where this was left:

for resolving references to symbols that are not currently imported (think referencing a material component from the flutter widget layer), we would need those dartdoc referenced to be fully qualified - to include enough information for us to resolve them. Something like [package:flutter/material.dart MaterialApp]
when the analyzer is not able to resolve a symbol reference in some dartdoc, dartdoc itself will try to resolve; it should be able to locate anything currently being documented in that session (so, anything in the current context). It the above example, it would resolve to material/MaterialApp-class.html

kevmoo · 2016-12-01T17:06:28Z

That would rock! I'd argue that we'd want to avoid too much (or any?) logic in dartdoc that does magic lookup. We'd want the editing experience to clearly align with what dartdoc does. We want the link to warn if they are referencing something that doesn't exist, etc...

…

On Thu, Dec 1, 2016 at 8:57 AM, Devon Carew ***@***.***> wrote: I believe where this was left: - for resolving references to symbols that are not currently imported (think referencing a material component from the flutter widget layer), we would need those dartdoc referenced to be fully qualified - to include enough information for us to resolve them. Something like [package:flutter/material.dart MaterialApp] - when the analyzer is not able to resolve a symbol reference in some dartdoc, dartdoc itself will try to resolve; it should be able to locate anything currently being documented in that session (so, anything in the current context). It the above example, it would resolve to material/MaterialApp-class.html — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCiis8_K32xzHMk1uiABLTho_-PkLNks5rDvxtgaJpZM4ISYYT> .

sethladd · 2016-12-01T17:24:13Z

We want the link to warn if they are referencing something that doesn't exist

Is there an issue open on the analyzer for this "extended link" syntax and support?

devoncarew · 2016-12-01T17:30:13Z

There isn't an issue - putting the logic into dartdoc was the resolution in order to avoid implementation complexity in the analyzer and analysis server (see dart-lang/sdk#27471 (comment)).

Hixie · 2016-12-01T21:37:42Z

I really would like to avoid having to fully qualify every reference. That would make the dartdocs unreadable, and it turns out that people look at the source a LOT. (Yay jump to definition.)

If there is only one identifier with the given name in the entire session, why not just assume that's what it means? And if there are duplicates, then require disambiguation. If we do this then we can even have it fail the entire process if any reference can't be resolved.

devoncarew · 2016-12-01T21:47:46Z

So, [MaterialApp], which would work if MaterialApp was unique, and [package:flutter/material.dart MaterialApp] (or similar fully-qualified syntax) if there was a need to disambiguate?

Hixie · 2016-12-01T22:08:34Z

That would be sufficiently concise for me I think.

astashov · 2016-12-17T18:16:21Z

I'm looking into it

@Hixie

@Hixie noticed, that sometimes, a library, like the Flutter rendering library, wants to refer to another library, like the Flutter widgets library, in the documentation, but doesn't want to introduce a dependency in the code. Currently, there’s no mechanisms in dartdoc, which allows that. This commit adds that. You can use either a short name (e.g. [icon]) or a fully qualified name (like, [material.Icon.icon]), in the HTML docs, it always will be shown as a short name though (is that a desired behavior?). Dartdoc will go over all the libraries, classes, methods, properties, constants of the package that is being documented, and will try to resolve the symbol. If it’s successful, it will add the link to it, same way as when the symbol is in scope. Some outstanding questions: * What to do in case of ambiguity here? Show a warning, crash with an error message, ignore? * Do we want to show short name in case a fully qualified name is provided? I saw in the comments in the ticket (dart-lang#1153 (comment)) that @Hixie would want that. * Anything else? Testing: Unit tests, of course, also tried to generate Flutter docs with that, and observed that the links for previously non-resolved symbols work.

astashov · 2016-12-17T21:53:32Z

Created initial PR for that. There's still some outstanding questions:

What to do in case of ambiguity here? Show a warning, crash with an error message, ignore? Doesn't seem like super critical thing, so I guess a warning would work, but would love to hear other arguments :)
Do we want to show short name in case a fully qualified name is provided? I saw in the comments in the ticket (handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153 (comment)) that @Hixie wants that, right?

Thanks!

Hixie · 2016-12-19T04:42:02Z

In case of ambiguity, I would fail hard.
Ideally we'd have analyzer warnings that let us know this is going to happen.

Please always give just the short name in the output, yes.

BTW once we have this we'll ideally also want to fail hard if there's every a [link] that is not resolvable. (Again, ideally, with an analyzer warning to let us know it'll happen before we try to run the docs.)

@Hixie

* Resolve non-imported symbols in comments [#1153] @Hixie noticed, that sometimes, a library, like the Flutter rendering library, wants to refer to another library, like the Flutter widgets library, in the documentation, but doesn't want to introduce a dependency in the code. Currently, there’s no mechanisms in dartdoc, which allows that. This commit adds that. You can use either a short name (e.g. [icon]) or a fully qualified name (like, [material.Icon.icon]), in the HTML docs, it always will be shown as a short name though (is that a desired behavior?). Dartdoc will go over all the libraries, classes, methods, properties, constants of the package that is being documented, and will try to resolve the symbol. If it’s successful, it will add the link to it, same way as when the symbol is in scope. Some outstanding questions: * What to do in case of ambiguity here? Show a warning, crash with an error message, ignore? * Do we want to show short name in case a fully qualified name is provided? I saw in the comments in the ticket (#1153 (comment)) that @Hixie would want that. * Anything else? Testing: Unit tests, of course, also tried to generate Flutter docs with that, and observed that the links for previously non-resolved symbols work. * Address @devoncarew feedback * Add types to declarations * Add filename/line number to the warnings * Move the code for gathering all the model elements of a package to the package's accessor and cache it. * Address another round of feedback From now on, if the reference was, for example, `[key]`, we won't match `material.Widget.key`, we will only match `material.key` (i.e. top-level `key` symbol). If you still want to match `Widget.key`, you should use either `[Widget.key]` or `[material.Widget.key]`.

sethladd · 2016-12-27T17:51:09Z

Thanks!!

gnprice · 2023-11-01T21:34:56Z

For cross-reference: the idea early in this thread of having a way to make imports only for dartdoc purposes now has a proposed design at:

Introduce a syntax for importing elements just for doc comment references sdk#50702

The dartdoc tool resolves these references anyway, ever since this issue was resolved in 2016. But the analyis server (and hence jump-to-definition in one's editor) doesn't, which is tracked at:

Analysis Server doesn't provide navigation information for non-imported dartdoc symbols in comments sdk#44637

Hixie mentioned this issue Apr 28, 2016

Fix cross-refs in rendering lib dartdocs flutter/flutter#3617

Closed

devoncarew added the customer-flutter Issues originating from important to Flutter label May 5, 2016

pq referenced this issue in dart-archive/vm_service_client May 20, 2016

comment linter issues

dd8ed1b

Hixie added type-enhancement A request for a change that isn't a bug P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jun 16, 2016

keertip added this to the hackathon milestone Jul 21, 2016

devoncarew mentioned this issue Jul 27, 2016

Some classes not hyperlinked #1152

Closed

keertip mentioned this issue Sep 27, 2016

ScrollableBlock isn't linked from Block API docs for Flutter #1238

Closed

devoncarew mentioned this issue Oct 3, 2016

support some form of fully qualified references in dartdoc documentation dart-lang/sdk#27471

Open

Hixie added P0 A serious issue requiring immediate resolution and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Oct 18, 2016

devoncarew modified the milestone: hackathon Nov 22, 2016

devoncarew changed the title ~~dartdoc-specific imports~~ handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) Dec 1, 2016

astashov mentioned this issue Dec 14, 2016

flutter docs are missing lots of classes #1236

Closed

astashov mentioned this issue Dec 17, 2016

Resolve non-imported symbols in comments [#1153] #1298

Merged

devoncarew closed this as completed in #1298 Dec 27, 2016

This was referenced Aug 18, 2018

comment_references lint doesn't support dartdoc's idea of "in scope" flutter/flutter#20765

Closed

comment_references lint doesn't support dartdoc's idea of "in scope" dart-lang/sdk#57783

Open

a14n mentioned this issue May 3, 2019

Loads of unresolved doc references flutter/flutter#31931

Closed

a14n mentioned this issue May 15, 2019

Make resolution of comment references homogeneous dart-lang/sdk#36974

Closed

alexander-doroshko mentioned this issue Jan 11, 2021

Analysis Server doesn't provide navigation information for non-imported dartdoc symbols in comments dart-lang/sdk#44637

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153

handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153

Hixie commented Apr 28, 2016

pq commented May 19, 2016

pq commented May 19, 2016

devoncarew commented Jul 27, 2016 •

edited

Loading

pq commented Jul 27, 2016

Hixie commented Jul 28, 2016

pq commented Jul 28, 2016

scheglov commented Jul 28, 2016

eseidelGoogle commented Aug 18, 2016

pq commented Aug 18, 2016

scheglov commented Aug 18, 2016

eseidelGoogle commented Aug 18, 2016

qchong commented Sep 27, 2016

scheglov commented Sep 27, 2016

sethladd commented Oct 4, 2016

Hixie commented Oct 4, 2016

sethladd commented Dec 1, 2016

devoncarew commented Dec 1, 2016

kevmoo commented Dec 1, 2016 via email

sethladd commented Dec 1, 2016

devoncarew commented Dec 1, 2016

Hixie commented Dec 1, 2016

devoncarew commented Dec 1, 2016

Hixie commented Dec 1, 2016

astashov commented Dec 17, 2016

astashov commented Dec 17, 2016

Hixie commented Dec 19, 2016

sethladd commented Dec 27, 2016

gnprice commented Nov 1, 2023 •

edited

Loading

handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153

handle dartdoc references to non-imported symbols (was: dartdoc-specific imports) #1153

Comments

Hixie commented Apr 28, 2016

pq commented May 19, 2016

pq commented May 19, 2016

devoncarew commented Jul 27, 2016 • edited Loading

pq commented Jul 27, 2016

Hixie commented Jul 28, 2016

pq commented Jul 28, 2016

scheglov commented Jul 28, 2016

eseidelGoogle commented Aug 18, 2016

pq commented Aug 18, 2016

scheglov commented Aug 18, 2016

eseidelGoogle commented Aug 18, 2016

qchong commented Sep 27, 2016

scheglov commented Sep 27, 2016

sethladd commented Oct 4, 2016

Hixie commented Oct 4, 2016

sethladd commented Dec 1, 2016

devoncarew commented Dec 1, 2016

kevmoo commented Dec 1, 2016 via email

sethladd commented Dec 1, 2016

devoncarew commented Dec 1, 2016

Hixie commented Dec 1, 2016

devoncarew commented Dec 1, 2016

Hixie commented Dec 1, 2016

astashov commented Dec 17, 2016

astashov commented Dec 17, 2016

Hixie commented Dec 19, 2016

sethladd commented Dec 27, 2016

gnprice commented Nov 1, 2023 • edited Loading

devoncarew commented Jul 27, 2016 •

edited

Loading

gnprice commented Nov 1, 2023 •

edited

Loading