Skip to content
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

Unable to write doc comment or metadata on primary constructor #3300

Open
srawlins opened this issue Aug 22, 2023 · 6 comments
Open

Unable to write doc comment or metadata on primary constructor #3300

srawlins opened this issue Aug 22, 2023 · 6 comments

Comments

@srawlins
Copy link
Member

I believe that neither the spec for extension types nor the proposed spec for primary constructors allow for doc comments or metadata (annotations) on a primary constructor.

Maybe that's fine. Let's examine each of these cases:

Cannot specify doc comment on an extension type's primary constructor

Well, an extension type's primary constructor can only have one parameter. The meaning of that parameter is always the same. Maybe it is difficult to conceive of a case where a doc comment specific to the primary constructor is important.

If the ability to specify doc comments is important, maybe we can just introduce a doc comment directive, e.g.

/// An extension type.
///
/// {@primary-constructor}
/// Wraps an [int] as a [V1].
/// {@end-primary-constructor}
extension type V1(int it) {

(Doc comment directives use kebab case; it is important to use as many cases in a programming language as possible.)

Cannot specify metadata on an extension type's primary constructor

This one worries me because I have to imagine there are plenty of cases where users desire to annotate a constructor. Maybe many cases can be handled by just assuming an annotation on the extension type is intended for the primary constructor. Maybe not.

I want to highlight the 3 primary purposes of annotations:

  • trigger static analysis, like @visibleForTesting, @experimental, etc.
  • trigger/inform code generation. I'd like to look at what code generation systems might use metadata on constructors, especially built_value, json_serializable, freezed.
  • (future) trigger macro execution. The possibilities are pretty open wide here. CC @jakemac53

Cannot specify doc comment on a (class's, etc.) primary constructor

Move the constructor (and fields) to be written explicitly, and then document the constructor as usual. The same goes for documenting the fields.

Cannot specify metadata on a (class's, etc.) primary constructor

Move the constructor (and fields) to be written explicitly, and then annotate as usual.

@leafpetersen
Copy link
Member

I'm not sure about the metadata, but the answer in Kotlin to the questions about the doc comments, is that all of that goes in the doc comment for the class.

@lrhn
Copy link
Member

lrhn commented Aug 23, 2023

Putting the constructor documentation into the class documentation worked for Kotlin because they have doc-tags like @param and @return to say that the next paragraph applies to a specific part of the declaration.

Dart has so far chosen not to have doc tags, and rely on convention like a paragraph starting with "Returns" being the documentation of the returned value, and an early reference to a parameter starting the parameter documentation, like "/// If [isFoo] is true, then ....".

That means that our tools can't easily provide the documentation for just a single parameter the way Java/Kotlin can.
And convention alone probably won't help us provide documentation for just the constructor taken from the class documentation.

We can document the individual parameters if we want to, by taking DartDoc on the primary constructor parameter as documentation for the implicit field:

class Foo(
  /// The number of bars.
  int bar,
  /// ...
) ...

But there is no good place to put documentation for the constructor.

Maybe if we put the constructor last:

class Foo implements FooBar
    /// Constructs a foo.
    /// 
    /// The `bar` is the number of bars.
    new(
       /// Number of bars.
       final int bar,
    ) {
  .. 
} 

Where you can document the constructor by putting a comment before the primary constructor declaration, and the fields out introduces by putting comments on the field-declaring parameters.

At that point, we might as well just declare it inside the body as:

class Foo implements FooBar {
    /// Constructs a foo.
    /// 
    /// The `bar` is the number of bars.
    primary new(
       /// Number of bars.
       final int bar,
    ) {
  .. 
}

(Can still use dartdoc on "field parameters" as property documentation.)

@eernstg
Copy link
Member

eernstg commented Aug 23, 2023

.. there is no good place to put documentation for the constructor.

True, but this might be OK. Primary constructors (as I've been thinking about them while writing this feature specification, and I think I'm not alone in that respect) are optimized for simple and small declarations. This might justify the advice that the documentation of the constructor is integrated into the documentation of the class, and DartDoc pages would show the documentation for the class as the documentation of the primary constructor as well. In that case it isn't necessary to introduce tags like @primaryConstructor.

If there is a need to add more detail then the primary constructor in the header can be turned into a primary constructor in the body:

/// An extension type.
extension type V1 {
  /// Wraps an [int] as a [V1].
  primary V1(int it);
  ...
}

@rrousselGit
Copy link

rrousselGit commented Aug 23, 2023

It's a small tangent, but honestly I wish Dart allowed docs on parameters.
We can already do:

class Foo {
  Foo({
    // The IDE will show a's docs when the parameter is hovered
    this.a,
  });
  /// This is documented
  final int? a;
}

To me it's counter intuitive that we don't allow:

class Foo {
  Foo({
    // Allow the IDE to still show docs
    /// Documentation for a
    int? a,
  }) : _a = a;

  final int? _a;
}

If we could do that, then docs on primary constructors likely wouldn't be an issue. Right?

@lrhn
Copy link
Member

lrhn commented Aug 27, 2023

To me it's counter intuitive that we don't allow:

class Foo {
  Foo({
    // Allow the IDE to still show docs
    /// Documentation for a
    int? a,
  }) : _a = a;

  final int? _a; // <- changed to `_a`
}

There are several things in play here:

  • The current current behavior of linking initializing formals to the dartdoc of the field they initialize. (Three things!)
  • Allowing DartDoc on primary constructor field parameters, which then become the documentation of the field.
  • Allowing DartDoc declared directly on normal parameters. We can do that. (But DartDoc doesn't have a good place to display that doc in the current generated HTML format, so it'll need a little thinking and tinkering.)
  • Allowing a public parameter to easily initialize a private field (or another field with a different name).

We have the first one, which is why this.a gets specialized docs on hover. That's nice, but tricky, because it works only because of peeking through an implementation detail.
The fact that you write this.a) instead of int a): this.a = a is an implementation detail, one is just a shorthand which you can choose to use in some cases. We'd never want to give one of them access to special semantics, because then we force an implementation choice. But hover-docs gives one of them special behavior, by providing the doc of the initialized field only for the initializing formal.
Which makes some sense because the shown doc might refer to the name, and only the initializing formal is guaranteed to have the same name.
The tool providing the hover-info (analysis server, I think) could choose to recognize a this.a = a initializer as well, and show the doc for the field on hover over the int a parameter. That only really works when the two have the same name, otherwise the doc might not read correctly. (I don't think changing any occurrence of [fieldName] or `fieldName` to [parameterName] and `parameterName` is going to be viable. Too easy to make mistakes for the `fieldName` references, when we don't know what they refer to.)

I'd definitely want the second item: Allowing you to write DartDoc on a primary constructor field parameter, one which implicitly introduces a field, so that that field can get DartDoc too. It'd be completely fair to treat the result as equivalent to a documented field and an initializing formal with the same name. That should just work.

Then there is what I believe you are suggesting here: Allowing docs on parameters in general, to be shown on hover.
That's probably a viable feature that the DartDoc and analyzer teams can figure out, if we want.
It risks conflicting with docs on primary constructor field parameters, but if we get the same doc on the field and the parameter anyway, it's probably fine.

And then there is the discussion of whether we can allow an primary constructor initializing formal to have a private name, and still expose a public-named parameter, like final int _a introducing a field named _ and a parameter named a (which is important if it's a named parameter). At that point, providing docs on the primary constructor field parameter would introduce the same docs for a field and a parameter with different names. If one of those names is private, it's probably OK to just write the doc using the public name, because that's all other people will see.
If we have a way to change the name more drastically, like final int? x as int y, which is equivalent to a final int? x field, a final int y parameter and a this.x = y initializer, then it's probably not possible to provide a single documentation that applies to both. And maybe it's OK, its not a feature we need to support, as long as we support declaring and documenting the field manually, and documenting the parameter as well.

(Again, let's not try top automatically rename references inside docs. Maybe we could allow dartdoc macros to be parameterized, so you can reuse, but also change the name:

default Floo(
   /// {@template isFastDoc name}
   /// Whether this floo is fast.
   ///
   /// If [@{templatearg name}] is `null`, it's unknown whether the floo is fast.
   /// Any attempt to [Floo.actFast] may fail, so you should use [Floo.tryActFast].
   /// {@endtemplate)
   /// {@macro isFastDoc #fast}  <!-- the #fast is checked to resolve to a name in scope, use "fast" if you don't want that -->
   bool? fast,
) : isFast = fast;
/// {@macro isFastDoc #isFast}
final bool? isFast;

But also, you can just copy, it's only two occurrences, so there are limits to how much effort is worth it.)

@rrousselGit
Copy link

rrousselGit commented Aug 30, 2023

To be clear, I wasn't necessarily talking about private parameters here but rather initializers in general.
Flutter suffers from this quite a bit. Lots of widgets defer to initializers. Private fields are only one case. For example I've also seen:

Class({required List list}):
  list = list.map(...).toList();

or:

Class({Color? primaryColor, this.scheme}):
  primaryColor = primaryColor ?? scheme?[0] ?? fallback;

Heck sometimes constructor parameters don't even have an associated field.

This makes dartdoc a bit unpredictable. Sometimes the IDE shows useful doc; sometimes it doesn't.

And it can vary between two versions of the same package. Because after a refactoring, maybe an initializer was added/removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants