dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.19k stars 1.57k forks source link

Analyzer should be able to parse documentation comments on method parameters #43185

Open Maistho opened 4 years ago

Maistho commented 4 years ago

The analyzer currently doesn't seem to pick up documentation comments on method parameters.

This could be useful for some code generation tools, indicated by https://github.com/rrousselGit/freezed/issues/236#issuecomment-680070351

In this example the analyzer shows the documentation for the method itself instead of the documentation for the parameter. Code example with analyzer highlighting the method Code example with analyzer highlighting the parameter


When using a constructor, it's possible to get the documentation for the property, but not when using a parameter to initialize some value. Code example 2 with analyzer highlighting the constructor Code example 2 with analyzer highlighting the first property

I think the example below should show "This is a parameter" in the analyzer Code example 2 with analyzer highlighting the second property

Code examples ```dart class Example { /// Some documentation about a method void method({ /// Some documentation about a parameter String aParameter, }) { // noop } void otherMethod() { method(aParameter: 'test'); } } ``` ```dart class Example2 { /// This is the constructor Example2({ this.aProperty, /// This is a parameter String anotherProperty, }) { this.anotherProperty = anotherProperty ?? 'some default'; } /// This is a property final String aProperty; /// This is another property String anotherProperty; static void example() { Example2( aProperty: 'test', anotherProperty: 'test2', ); } } ```

Sorry for the large images, I couldn't think of a better way to showcase the issue.

mraleph commented 4 years ago

I think this is in alignment with Effective Dart guidelines on documentation comments: https://dart.dev/guides/language/effective-dart/documentation - documentation comments on individual parameters are not something that is supported or encouraged. Parameters are expected to be documented in the method documentation comment.

I however defer to @munificent for style judgement and @devoncarew for further triage in the analyzer.

bwilkerson commented 4 years ago

The class NormalFormalParameter does have a documentationComment getter. I'm guessing that that isn't being populated, even though it seems like it ought to be.

devoncarew commented 4 years ago

I suspect that tooling (IDEs, dartdoc and such) should not support dartdoc comments on params if that's not aligned with how we want doc comments to be written (via the style guide). package:analyzer should likely also follow that as well, otherwise the use of dartdoc comments on params might creep into the ecosystem. I don't know how limiting that is for other use cases; for the one linked to in this issue, perhaps an annotation on the param could help guide codegen?

bwilkerson commented 4 years ago

I suspect that tooling (IDEs, dartdoc and such) should not support dartdoc comments on params if that's not aligned with how we want doc comments to be written (via the style guide).

I agree. I don't think server should, for example, return these docs in the hover text for methods.

package:analyzer should likely also follow that as well, otherwise the use of dartdoc comments on params might creep into the ecosystem.

That's less clear to me; the use of dartdoc comments on params in the ecosystem hasn't happened yet, and analyzer has had that API for a very long time (and I'm fairly sure it worked at one point, even though it appears to not be working now).

The analyzer APIs currently support a documentation comment anywhere that metadata is allowed in the grammar, and consistency is generally a good thing in API design. I suppose it's possible that it's important enough to not support documentation comments on parameters to justify removing the getter from the API, but it isn't clear to me that it's worth the cost of a breaking change and an inconsistent API.

What is clear to me, though, is that the getter either needs to work or it needs to be removed.

devoncarew commented 4 years ago

package:analyzer should likely also follow that as well, otherwise the use of dartdoc comments on params might creep into the ecosystem.

That's less clear to me; the use of dartdoc comments on params in the ecosystem hasn't happened yet

Yeah, I don't have a strong opinion here wrt package:analyzer supporting this (more so for the tools).

munificent commented 4 years ago

I think we (as in the entire Dart ecosystem) have been pretty happy with just documenting parameters in prose with the method's main doc comment. It's been that way for ages, and is used for tiny apps and enormous wildly popular frameworks. If the guideline can handle all of those cases, I think it's good enough to just stick with it.

rrousselGit commented 4 years ago

I disagree with the conclusion drawn.

The fact that in the current state of the ecosystem people don't put comments on parameters doesn't mean that people are happy with it.

Commenting parameter is useful primarily for users of a package ; but comments on parameters are currently ignored by the IDE. Which means there is now no value in writing some, since the chance that users will open the sources to read the documentation is relatively low due to poor developer experience.

It also isn't true to say that we don't have parameter documentation. There are some situations where we do:

class Example {
  Example({this.a});

  /// Some documentation
  final a;
}

In this snippet, Example(a: ) brings up the documentation of a.

In fact, parameter documentation is likely the main reason why Flutter Widgets use public variables to store parameters.