-
Notifications
You must be signed in to change notification settings - Fork 132
Add dart documentation comment specification version 1.0.0 #4130
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
base: main
Are you sure you want to change the base?
Conversation
70470e8 to
dbac532
Compare
|
The questions below are mainly questions about future versions of this spec.
/// Some doc comment about class C
class C {
/// Some doc comments about the constructor
C([int p]);
}
/// Another doc comment about class C
augment class C {
/// Comment about the constructor
augment C([int p = 0]) {
// Augmentation added the defaulf value and the body
}
}
/// Some doc comment. Is it related to the class or to the constructor?
class C(var int x); |
srawlins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤩 Very cool, very exciting!
| This document does not cover: | ||
|
|
||
| * The implementation details of specific documentation tools (e.g., `dartdoc`, `analysis server`). | ||
| * The syntax and behavior of tool-specific directives (e.g., `@template`, `@canonicalFor`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you consider these in-scope in a later version? It would be very beneficial for DAS and dartdoc to agree on how these function. And new ones can be specified like @snippet, @sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely could! Or we can clean up directives.md and add new ones there.
|
|
||
| ### **2.3. Content Format (Markdown)** | ||
|
|
||
| The text within a documentation comment block is parsed as Markdown, allowing for rich text formatting. This includes headings, lists, code blocks, and emphasis, which are converted for instance to HTML in the generated documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably worth mentioning that they are parsed as CommonMark markdown, as "markdown" is not a well-specified syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For the record, "MarkDown" is also specified, https://daringfireball.net/projects/markdown/, it's just that nobody actually uses that specification. That's why CommonMark was created.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you're correct. It is GitHub-flavored Markdown, an extension of CommonMark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated it to say GFM! Thanks!
| * **Display:** The getter and setter should be presented as a single item in the final documentation. | ||
| * **Precedence:** | ||
| * Documentation for the property should only be placed on the getter. | ||
| * The tooling should *issue a warning* if both a getter and its corresponding setter have unique doc comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does inheritance factor in here?
If I override a documented field with a setter, can I document the setter?
If a supertype has a documented setter, and an undocumented getter, and I override the getter, can I document the overriding getter (thus making both the getter and setter documented)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I override a documented field with a setter, can I document the setter?
I would say no. The mental model here is that a property (whether defined via a field or a getter/setter pair) is a single semantic entity. The getter is the "anchor" for that entity's documentation.
If a class inherits a getter and overrides the setter, the property's documentation remains anchored to the getter. Documenting the overriding setter would probably have something to do with the new mutable behavior but the original documentation would still be relevant, so this new doc shouldn't replace the old. Otherwise we create scenario where the documentation is split with no single source of truth. To prevent ambiguity, tools should ignore documentation on the setter if a getter exists.
If a supertype has a documented setter, and an undocumented getter, and I override the getter, can I document the overriding getter (thus making both the getter and setter documented)?
In this system, an undocumented getter takes precedence over a documented setter. The presence of a getter determines the documentation slot. If the getter is undocumented, the property is effectively undocumented.
munificent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it!
|
|
||
| Documentation tools should handle this property with the following rules: | ||
|
|
||
| * **Display:** The getter and setter should be presented as a single item in the final documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. The hover should know that it is a setter, but display the singular documentation for the property, and that documentation should cover both is read and write aspect. This would also avoid having documentation that isn't shown; if you painstakingly write documentation on the setter it would be unfortunate that is only shown when you hover and not elsewhere.
As a side note, DAS is a bit inconsistent with this. For instance if you ask for usage on a setter, you still get all the results that read the property, indicating that it is using the conceptual property in this case.
|
|
||
| * **Doc Comment:** A comment intended to be processed by documentation generation tools, like dartdoc. | ||
| * **Element**: A specific, declared element in the Dart code, such as a class, method, function, variable, or type parameter. | ||
| * **Identifier:** An individual name in the code (e.g., `MyClass`, myMethod, prefix). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-quote myMethod too, probably also prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| * The *syntax* for writing documentation comments. | ||
| * The rules governing where documentation comments can be *placed*. | ||
| * The mechanism for *resolving* references between elements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is an "element"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is covered in the terminology section, now with updated definition. I guess I could swap 1.2 and 1.3. It feels a bit backwards to define terms before explaining what the doc is about.
| ### **1.3. Terminology** | ||
|
|
||
| * **Doc Comment:** A comment intended to be processed by documentation generation tools, like dartdoc. | ||
| * **Element**: A specific, declared element in the Dart code, such as a class, method, function, variable, or type parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Cyclic definition: An element is a (specific declared) element. Then some examples, but one has to guess what they have in common.
The concept of "element" is an implementation detail in our tools, it's not a term that the language specification uses. It's not a word we can assume a reader will understand by themselves, so if we want to use it here, it needs to be defined, non-circularly. Or just use "declaration" or maybe "named declaration".)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I would like to keep element for this doc. Updated the definition.
| * Fields (constants and variables) (e.g., `[myField]`, `[MyClass.myField]`) | ||
| * Getters and Setters (See [Section 6.2.2](#6.2.2.-getters-and-setters) for full details) | ||
| * Constructors (e.g., `[MyClass.new]`, `[MyClass.named]`, `[MyClass.named()]`) | ||
| * Enum constants (e.g., `[MyEnum.value]`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth mentioning that there is no syntactic distinction between references to static and instance members.
(There is also an ambiguity, since [Class.name] can refer both to a constructor and an instance method, and a class declaration can have both. Need to say how it's disambiguated. Prefix new may still work.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
The disambiguation part is handled further in Section 5.5
|
|
||
| * Parameters of the documented method/function (e.g., `[parameterName]`) | ||
| * Type parameters of the documented element (e.g., `[T]`) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Could this section simply say "any declaration whose name is in scope other than import prefixes"? Plus qualified names where the first identifier must denote "any declaration in scope".)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the core idea yes. I still think it makes sense to have this section with the examples of the references. Especially for those that have a bit of special syntax, like [MyClass.new] or if we decide to support references to methods with trailing parentheses.
| | | | | | +------------------------------------------------------+ | | | | | | ||
| | | | | | | 2. Method Type Parameter Scope (e.g., <R>) | | | | | | | ||
| | | | | | | +--------------------------------------------------+ | | | | | | | ||
| | | | | | | | 1. Formal Parameter Scope (e.g., parameter names)| | | | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And for initializing constructors, use the initializer list scope instead, since that contains the names of initializing formals and super parameters too.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we get away with still calling it formal parameter scope? I mean do you see any examples where we can refer to a name in the initializer list?
|
|
||
| #### **5.3.1. Instance Methods and Constructors** | ||
|
|
||
| This applies to doc comments on methods, constructors, and operators within a class, enum, mixin, extension, or extension type. The search begins with the method's own parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(But not setters. Maybe say that explicitly.
They have a parameter list too, but I can see that's inconsistent with treating them as properties. If I do:
class C {
void cake() {}
/// Is it [cake]?
set bar(int cake) {}
}it looks like DartDoc refuses to make [cake] a link. It probably resolves to the setter parameter, but there is no target for a setter parameter to link to. If that's the case, a setter doc does start at the parameter scope.
I guess that answers my "anything but import prefixes" question above in the negative. "Anything but import prefixes and setter parameters."?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it applies to setters too. (Although we don't consider documentation above the setter - except in the case where no getter exists.)
| class MyClass<T> { | ||
| T? value; | ||
|
|
||
| MyClass.named(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Consider:
/// A non-redirecting generative constructor.
///
/// Lookup examples:
/// * [value]: Resolves to the parameter.
/// * [other]: Resolves to the parameter.
/// * [MyClass.value]: Resolves to the instance variable.
/// ... (more?) ...
MyClass.value(this.value, int other);)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added example for the constructor but this spec proposes that [MyClass.value] will resolve to the constructor and not the field.
|
|
||
| This applies to doc comments on fields within a class, enum, or mixin. Since fields have no parameters, the search starts at the member level, but otherwise follows the same route as instance methods. | ||
|
|
||
| * **Starting Scope**: Class Member Scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And getters and setters are treated like fields?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, lookup-wise I would say like methods. But getters/setters are a bit of a special case, that's why there is a dedicated section for those (section 6.2.2)
|
|
||
| #### **5.3.6. Typedefs** | ||
|
|
||
| The starting scope for a typedef doc comment depends on what it is an alias for. For function and record types, the scope starts at the parameter/field level. For all other types, it starts at the typedef's own type parameter scope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This may need to be accounted for in the definition of what an "element" is. Type aliases for structural types do not introduce any declarations, their field/parameter names are not what I would usually consider expect to be an "element".)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added it both to the element definition and to section 4.3.
|
|
||
| * **Notes on Generics:** | ||
| * The namespace does not include the element's own type parameters. For a class `MyClass<T>`, a reference like `[MyClass.T]` is invalid because T is not a member of `MyClass`'s namespace. | ||
| * References are also made to the generic declaration, not a specific instantiation (e.g., write `[List.filled]`, not `[List<int>.filled]`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or else?
Currently it seem like the type argument is ignored, but the linking works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linking works in dartdoc but not in the IDE. I think we should not support it and it is kind of a bug that we resolve it in dartdoc.
| * **Example:** To resolve the reference `[collection.BoolList.empty]`: | ||
| * The identifier collection resolves to an import prefix. | ||
| * The identifier BoolList resolves to a class element within the collection library's public namespace. | ||
| * The identifier empty resolves to a named constructor element within the BoolList class's member namespace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today List.List.empty also resolves to the List.empty constructor in the List namespace. (Which can be convenient?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion this is a bug in dartdoc. The correct reference is [List.empty]. (And it also doesn't resolve in the IDE)
| * **Namespace:** The set of all members declared within that element, including: | ||
| * Instance Members (fields, methods, etc.) | ||
| * Static Members | ||
| * Constructors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also type-aliases (typedef) probably works like the type they alias, if they alias a specific type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and then they fall under the "class-like" category, no?
|
Did I miss anything about actual HTTPS style links? That's been a problem w/ the analyzer lint for a while! |
d5502da to
c876086
Compare
The exact details for doc comments for augmentations and primary constructors are not completely decided. This spec will be updated once the design is final. |
|
Thanks so much for all the questions and comments! I have now addressed all of them. PTAL @lrhn @sigurdm @srawlins @johnniwinther @bwilkerson |
No description provided.