dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
634 stars 170 forks source link

`comment_references` should allow references to elements of a returned record #5075

Closed stereotype441 closed 2 months ago

stereotype441 commented 2 months ago

Historically, it's been common to document the parameters of a method by using square brackets to refer to the parameter names. For example, the doc comment for List.operator == (in the SDK) uses square brackets to refer to the parameter other:

  /// Whether this list is equal to [other].
  ///
  /// Lists are, by default, only equal to themselves.
  /// Even if [other] is also a list, the equality comparison
  /// does not compare the elements of the two lists.
  bool operator ==(Object other);

Note that Dartdoc doesn't generate a link for other (see https://api.dart.dev/stable/3.5.1/dart-core/List/operator_equals.html), so the only relevant functionality here is that the comment_references lint allows the name to be other.

I think that if the return type of the method being documented is a record, it should be permissible to use square brackets to refer to members of the record. For example:

  /// Returns a list of [Foo] and a list of [Bar].
  ///
  /// [foos] is the list of [Foo] and [bars] is the list of [Bar].
  ({List<Foo> foos, List<Bar> bars}) getFoosAndBars();

Similar for unnamed record elements, using the usual $N syntax. For example:

  /// Returns a list of [Foo] and a list of [Bar].
  ///
  /// [$1] is the list of [Foo] and [$2] is the list of [Bar].
  (List<Foo>, List<Bar>) getFoosAndBars();
bwilkerson commented 2 months ago

Note that Dartdoc doesn't generate a link for other (see https://api.dart.dev/stable/3.5.1/dart-core/List/operator_equals.html), so the only relevant functionality here is that the comment_references lint allows the name to be other.

Dartdoc ignores it, but navigation within an IDE will navigate to the declaration of the parameter. We do that by resolving it to a ParameterElement. We don't have an element representing the fields in a record, so we have nothing to resolve it to and no way to navigate for those kind of references.

That isn't an argument against allowing such references, but it does mean that we'd need to do more work in the lint. The lint currently uses the presence or absence of a resolved element to decide whether or not to report the reference. That would have to change.

srawlins commented 2 months ago

I'd love this. I think the more common case is desiring to refer to a function-typed element's parameter. E.g. to be able to refer to value in Iterable.reduce, (E Function(E combine(E value, E element))).

Two questions:

  1. How to disambiguate between elements? There isn't really a namespacing available. E.g. given a function with a signature Map<({int a, int b}), ({int a, ({int a, int b})})> Function(({int a, int b}) c, List<({int a, int b})> d), how to refer to each a?
  2. This request is for comment_references. But that lint rule depends almost entirely on analyzer's resolution. Are you specifically requesting that the lint rule separate from analyzer's resolution, and allow [foos], even when the analyzer does not resolve [foos], does not provide go-to-definition, go-to-references or renaming? Or is the request for analyzer to resolve the text as well?
github-actions[bot] commented 2 months ago

Without additional information we're not able to resolve this issue. Feel free to add more info or respond to any questions above and we can reopen the case. Thanks for your contribution!