Supply class dartdoc when no constructor dartdoc is defined (or generally, in addition to) for hovers

[DanTup]

Raised by @eseidel in Dart-Code/Dart-Code#240. I think this would be best done in the analyzer if you think it's worthwhile (and would also then work for all other editors). I think the reason the first one works is that it just looks like a reference to the class rather than the constructor. I agree that the second one would be best falling back to the class dart doc because the constructor doesn't have one.

---

@eseidel:

Hovering a new using a constructor shows the class docs:

But not hovering an annotation (which uses the same constructor):

No clue if the missing functionality here is in the plugin or the analyzer or what. :)

[marcoms]

I think a case can be made that class docs could be included even if constructor docs are present.

Frequently in Flutter there are very helpful and comprehensive class docs, but the constructor just provides a couple sentences of description. Example:

/// A convenience widget that combines common painting, positioning, and sizing
/// widgets.
///
/// A container first surrounds the child with [padding] (inflated by any
/// borders present in the [decoration]) and then applies additional
/// [constraints] to the padded extent (incorporating the `width` and `height`
/// as constraints, if either is non-null). The container is then surrounded by
/// additional empty space described from the [margin].
///
/// During painting, the container first applies the given [transform], then
/// paints the [decoration] to fill the padded extent, then it paints the child,
/// and finally paints the [foregroundDecoration], also filling the padded
/// extent.
///
/// Containers with no children try to be as big as possible unless the incoming
/// constraints are unbounded, in which case they try to be as small as
/// possible. Containers with children size themselves to their children. The
/// `width`, `height`, and [constraints] arguments to the constructor override
/// this.
///
/// ## Layout behavior
///
/// _See [BoxConstraints] for an introduction to box layout models._
///
/// Since [Container] combines a number of other widgets each with their own
/// layout behavior, [Container]'s layout behavior is somewhat complicated.
///
/// tl;dr: [Container] tries, in order: to honor [alignment], to size itself to
/// the [child], to honor the `width`, `height`, and [constraints], to expand to
/// fit the parent, to be as small as possible.
///
/// More specifically:
///
/// If the widget has no child, no `height`, no `width`, no [constraints],
/// and the parent provides unbounded constraints, then [Container] tries to
/// size as small as possible.
///
/// If the widget has no child and no [alignment], but a `height`, `width`, or
/// [constraints] are provided, then the [Container] tries to be as small as
/// possible given the combination of those constraints and the parent's
/// constraints.
///
/// If the widget has no child, no `height`, no `width`, no [constraints], and
/// no [alignment], but the parent provides bounded constraints, then
/// [Container] expands to fit the constraints provided by the parent.
///
/// If the widget has an [alignment], and the parent provides unbounded
/// constraints, then the [Container] tries to size itself around the child.
///
/// If the widget has an [alignment], and the parent provides bounded
/// constraints, then the [Container] tries to expand to fit the parent, and
/// then positions the child within itself as per the [alignment].
///
/// Otherwise, the widget has a [child] but no `height`, no `width`, no
/// [constraints], and no [alignment], and the [Container] passes the
/// constraints from the parent to the child and sizes itself to match the
/// child.
///
/// The [margin] and [padding] properties also affect the layout, as described
/// in the documentation for those properties. (Their effects merely augment the
/// rules described above.) The [decoration] can implicitly increase the
/// [padding] (e.g. borders in a [BoxDecoration] contribute to the [padding]);
/// see [Decoration.padding].
///
/// {@tool sample}
///
/// This example shows a 48x48 green square (placed inside a [Center] widget in
/// case the parent widget has its own opinions regarding the size that the
/// [Container] should take), with a margin so that it stays away from
/// neighboring widgets:
///
/// ```dart
/// Center(
///   child: Container(
///     margin: const EdgeInsets.all(10.0),
///     color: const Color(0xFF00FF00),
///     width: 48.0,
///     height: 48.0,
///   ),
/// )
/// ```
/// {@end-tool}
/// {@tool sample}
///
/// This example shows how to use many of the features of [Container] at once.
/// The [constraints] are set to fit the font size plus ample headroom
/// vertically, while expanding horizontally to fit the parent. The [padding] is
/// used to make sure there is space between the contents and the text. The
/// `color` makes the box teal. The [alignment] causes the [child] to be
/// centered in the box. The [foregroundDecoration] overlays a nine-patch image
/// onto the text. Finally, the [transform] applies a slight rotation to the
/// entire contraption to complete the effect.
///
/// ```dart
/// Container(
///   constraints: BoxConstraints.expand(
///     height: Theme.of(context).textTheme.display1.fontSize * 1.1 + 200.0,
///   ),
///   padding: const EdgeInsets.all(8.0),
///   color: Colors.teal.shade700,
///   alignment: Alignment.center,
///   child: Text('Hello World', style: Theme.of(context).textTheme.display1.copyWith(color: Colors.white)),
///   foregroundDecoration: BoxDecoration(
///     image: DecorationImage(
///       image: NetworkImage('https://www.example.com/images/frame.png'),
///       centerSlice: Rect.fromLTRB(270.0, 180.0, 1360.0, 730.0),
///     ),
///   ),
///   transform: Matrix4.rotationZ(0.1),
/// )
/// ```
/// {@end-tool}
///
/// See also:
///
///  * [AnimatedContainer], a variant that smoothly animates the properties when
///    they change.
///  * [Border], which has a sample which uses [Container] heavily.
///  * [Ink], which paints a [Decoration] on a [Material], allowing
///    [InkResponse] and [InkWell] splashes to paint over them.
///  * The [catalog of layout widgets](https://flutter.io/widgets/layout/).
class Container extends StatelessWidget {
  /// Creates a widget that combines common painting, positioning, and sizing widgets.
  ///
  /// The `height` and `width` values include the padding.
  ///
  /// The `color` argument is a shorthand for `decoration: new
  /// BoxDecoration(color: color)`, which means you cannot supply both a `color`
  /// and a `decoration` argument. If you want to have both a `color` and a
  /// `decoration`, you can pass the color as the `color` argument to the
  /// `BoxDecoration`.

Since users mostly reference the constructor, in order to access the full documentation, you basically have to Alt+Click and view the source code of the widget. IMO not the best DX.

[DanTup]

Showing class dartdocs always also came up at flutter/flutter#96332.

I wonder whether the issue is more general than constructors though - if you changed a constructor to a static method, would seeing the dartdoc have less value, or is the desire really to see the docs of the return type of all methods/functions?

I also wonder if the same should go for code completion. Although it seems like in VS Code that's already showing the class dartdocs (instead of the constructor) here, and I'm not sure why (I can't reproduce in Android Studio). Seems like they should probably be consistent - although fixing that without also doing this might seem like a step backwards if there is value in seeing the class docs there.

[bwilkerson]

It seems like there are at least three issues here that would be worth thinking about separately.

1. Does it makes sense to display the class' documentation when a constructor without documentation is being referenced?

2. Should the class' documentation always be shown when hovering over a reference to a constructor.

3. What documentation should be displayed for code completion.

It seems to me that one guiding principle for answering these questions (and any question related to how documentation is displayed in the IDE) is to be consistent with dartdoc wherever it makes sense to do so.

So for the first issue, I'd say that it doesn't make sense to display the class' documentation for an undocumented constructor because dartdoc doesn't do so. I think it also makes sense to not do so just from the perspective that the documentation for the class isn't describing the constructor.

But the second issues is a bit different. I wonder whether it's really an observation that without type annotations there's no easy affordance to access the class' documentation. For example, if you wrote

Container c = Container();

then you could hover over the first occurrence of the class name to get the class' documentation and the second occurrence to get the constructor's documentation. Whereas if the first occurrence is replaced by var, then there is no hover help. It might we'll be worthwhile considering possible affordances for accessing the class' documentation in such situations.

For the third issue, I think we should always display the documentation for the element that would be referenced if the suggestion is accepted. If that's not currently the case, then I think we have a bug and we should fix it.

[DanTup]

It might we'll be worthwhile considering possible affordances for accessing the class' documentation in such situations.

I can't think of many ways we could do this besides adding the class documentation to the bottom of the constructor docs. I think the same may apply to return values of non-constructor functions too:

Foo foo = getFoo(); // Can easily access Foo class docs
final foo = getFoo(); // Cannot

Although accepting that also makes me think final a = class.theFoo isn't much different and you might want to see docs about the type of theFoo. This might make the solution more general (always show the class docs for the return type) - although given an issue in VS Code where it shows diagnostics right at the bottom of the docs (microsoft/vscode#73120) I think adding more docs into tooltips there could be bad for usability until that's fixed (it's been open almost 3 years so I'm not hopeful for a fix in the near term).

For the third issue, I think we should always display the documentation for the element that would be referenced if the suggestion is accepted. If that's not currently the case, then I think we have a bug and we should fix it.

I had a quick look at this, and believe it's because of how we fetch the documentation lazily for LSP with suggestion sets. We send two completions to the client (MyClass and MyClass() for the class/constructor respectively), but when we "resolve" them to get the import edit and documentation, we only have MyClass to look the element up (via requestedLibraryElement.exportNamespace.get(requestedName) so both the class and the constructor get the same thing.

I think this problem might go away when migrating to the NotImportedContributor. I filed an issue at Dart-Code/Dart-Code#3787 as a reminder for me to check this once that work is done.

[bwilkerson]

It might we'll be worthwhile considering possible affordances for accessing the class' documentation in such situations.

I can't think of many ways we could do this besides adding the class documentation to the bottom of the constructor docs. I think the same may apply to return values of non-constructor functions too:

A couple of brainstorming ideas:

• Could we add "links" of some kind to the hover text so that users could navigate to the docs for the type?

• We could definitely add a hover for var, final, or const that would display the docs for the inferred type (or explicit type if one is given).

But I definitely agree that appending the class docs to the end of the other might be a poor UX, especially given the size of the docs for most of the Flutter classes. Hence the desire to find a better way to make this information accessible to users. (And there doesn't have to be a single way if multiple ways make sense.)

[DanTup]

Could we add "links" of some kind to the hover text so that users could navigate to the docs for the type?

For VS Code, we could append hyperlinks to the end of the markdown that go to the docs website - though I don't think we can do anything fancy like update the hover content to show another items docs. To do the first, we'd need to be able to construct URLs to get to those docs. This is a request that's been asked for before though, and I do often find myself opening my browser and going to pub.dev/packages/foo and then clicking the API documentation link on the side.

We could definitely add a hover for var, final, or const that would display the docs for the inferred type (or explicit type if one is given).

That may help some, although not in places like the Flutter one where this came up recently:

final a = Container(
  child: ElevatedButton() // Want to see ElevatedButton docs by hovering on this, or when code-completing it
);

Right now you only see:

/// Create an ElevatedButton.
///
/// The [autofocus] and [clipBehavior] arguments must not be null.

[bwilkerson]

That's definitely a harder case. I'm not seeing anywhere obvious where a user could reasonably expect to be able to hover to get those docs. But if we could link to the class' docs externally (or even the constructor's docs because it ought to be easy to get from there to the docs for the class) that might help.

[DanTup]

I think having a hyperlink appended to the dartdoc that has "View docs on pub.dev" or similar would be good. I don't know how simple it is to build the correct URL though (I think it would need to be conditional on it being a package from pub and not just local, and take the version number into account?).

[bwilkerson]

Yes, it probably would. Flutter documentation might need to be handled differently, and in fact might want to be the first case we handle.