Identifiers in dartdocs with parens /// [functionName()] do not generate navigation regions

[DanTup]

This came up at Dart-Code/Dart-Code#3611 and I was going to look at fixing in the analysis_server, but I think it may require parser changes (@bwilkerson?).

In the Effective Dart docs in the description of identifiers in doc comments it says:

Parentheses are optional, but can make it clearer when you’re referring to a method or constructor.

However given this code:

/**
 * [myFunction]
 * [myFunction()]
 */
myFunction() {}

The second reference does not generate a navigation target, so cannot be used to navigate to the function. I don't know if including parens is common, but since the docs suggest they're useful, it seems like this should work.

@bwilkerson I had a look thinking this might be something handled in the server, but I ended up in /pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart (parseCommentReferencesInText) so perhaps it needs supporting there (I'm less certain of the implications of making changes there, although happy to have a go if it should be straight-forward).

[bwilkerson]

@srawlins and @jcollins-g have been looking at making the analysis server and dartdoc more consistent. I don't know what dartdoc currently does with parens, but if it doesn't accept them then we ought to consider updating the Effective Dart docs.

[jcollins-g]

Parentheses are supported by dartdoc's syntax. It results in a "callable hint", which dartdoc uses in resolution to disambiguate constructors prior to tearoffs and A.new. After the redesign of the reference lookup system it is more generically used as a "this must refer to a callable" signal.

[srawlins]

Thanks for filing @DanTup . I personally prefer to remove the [myFunction()] suggestion from Effective Dart. I am comfortable referring to functions without parentheses, but I can see how they help in identifying an element as "callable." Perhaps a comment like /// See [Future.value] looks like it is referring to a field on Future, rather than a constructor.

We could try to gather some data on how often we see [myFunction()], but I imagine that would be heavily affected by the current state, in which [myFunction()] does not result in a comment reference. :(

My current plans are to add two features to comment references:

• three-name identifiers, like [async.Future.value], to refer to constructors or static functions on types imported with a prefix,
• type arguments, like in [List<int>], [core.List<int>], [List<int>.filled], [async.Future.any<void>],

I'm happy to leave this open to consider supporting [myFunction()], but in the meantime, I think we should remove the text from Effective Dart.

[bwilkerson]

As far as I can tell, you're right that /pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart is where these comment references are being parsed.

I'm ambivalent on the question about whether to continue supporting parens. It sounds like Dartdoc's need for then has probably been resolved, but on the other hand there might be legacy docs that are using them that might not get cleaned up quickly.

[jcollins-g]

@bwilkerson Yes, it has been mostly resolved with constructor tearoffs and .new references. There might be a strange case where it is the only way to disambiguate comment references that include package/library names, but I think all those cases can be better served with a more specific reference declaration. e.g. instead of [packageName.foo()], [packageName.libraryName.foo].

[jcollins-g]

I would support removing the parentheses recommendation from Effective Dart once constructor-tearoffs are in stable, possibly adding a section for how to refer to the unnamed constructor via .new.

[bwilkerson]

@munificent For the discussion around Effective Dart.

[DanTup]

I've no opinion here (beyond thinking Effective Dart and the server should be consistent), so feel free to close this if the plan is just to remove the recommendation (and/or move it to track that) :-)

[munificent]

I agree "Effective Dart" should only advocate something that actually works. :)

For what it's worth, I personally do like using parentheses in doc comments to refer to functions because I think it's clearer to the reader what's being referred to.

[ghost]

Hello! Just leaving my opinion here as a Dart user.
As someone who heavily documents my code for my team to be able to better understand how everything works, I would vote in favor of adding proper support for using parentheses in the dartdocs. I already use parentheses in my documentation because it makes it obvious that I'm referencing either to a method or a constructor, and not to a value/getter/setter, and proper support would make it even easier to jump to the actual reference when someone wants to see how it works.

[lrhn]

As a different opinion, I don't like the parentheses. A link of [int.parse()] suggests that int.parse takes zero arguments.
(Or can you also write [int.parse(Strring)?)

I'd expect the content of the [...] to be something denoting the declaration, not an arbitrary expression.

If you want to control what's inside the link text, I'd expect to use ``[int.parse(String)][int.parse]` instead. (That works, right?)

[DanTup]

If you want to control what's inside the link text, I'd expect to use [int.parse(String)][int.parse] instead. (That works, right?)

I don't know about for generated Dartdoc pages, but at least some of the discussion above relates to reading the raw source code in the editor (and being able to Ctrl+Click to jump to the related function). Using the example above does result in a clickable link on the second part - although I'm not sure if you meant [int.parse(String)](int.parse) (since markdown links usually use parens for the url?) which wouldn't work:

[lrhn]

Ah, I was thinking it should be a "reference style link", probably mixin Wiki-syntax with plain markdown. I guess a normal (...) based on makes more sense.

[DanTup]

Ah, I was thinking it should be a "reference style link",

Ah, ofc, that probably does make more sense. We could potentially insert the signature automatically (as fake text, like the closing labels) although I'm not sure if the benefit would justify the effort in doing that (and the inserted text part wouldn't be clickable).

[srawlins]

To elaborate on what Janice wrote above, Dartdoc uses the trailing parens (()) to disambiguate between a constructor and an instance field with the same name, which is legal. In the presence of an instance field name 'b' and a constructor named 'b', Dartdoc points[A.b] to the field, and [A.b()] to the constructor.

However, one is also allowed to declare an instance method with the same name as a constructor, in which case the parentheses would not, semantically, really disambiguate. We could just stick to a rule that "parens hint toward a constructor; no parens hint toward an instance member." 🤷

[DanTup]

a constructor and an instance field with the same name, which is legal
...
one is also allowed to declare an instance method with the same name as a constructor

TIL! It always bugged me that I can't have static and instance members with the same name - that feels a little more odd given constructor names seem exempt :-)

I think using the parens as only a hint and not a rule would be better though. It seems odd to require parens when there's no ambiguity. It also seems a little odd that I can't use parens on a function.

[srawlins]

I think we're suggesting the same thing 😄 . I wouldn't suggest requiring parens broadly. Just that, in the handful of cases in the universe where a class has a constructor and instance member with the same name, and you really want to refer to them in doc comments, [A.b] will refer to the instance member, and [A.b()] will refer to the constructor. Without a conflict, [A.b] will still refer to a constructor if it exists, or an instance member if it exists, or a static member, for that matter, if it exists.

[FMorschel]

About the colouring, here in this comment you can see that the . also is not painted. Not a problem but I feel like they should behave the same. And to the comment above, constructors with () for differentiation are also not painted, not simply functions.

---

From #59910 (comment), I feel like this is a good place to point out about the missing docs on this:

Dartdoc prefers the A.foo instance member. To specifically refer to the constructor, you can use A.foo(). We should document this behavior here: https://dart.dev/tools/doc-comments/references

[DanTup]

Dots are actually uncoloured in the editor too:

We only add semantic tokens to the class name and member name so they're just the default surrounding colour (which in the editor is nothing, so it's the default color from the theme).

We could perhaps use the custom "reset" token we use in interpolation to reset the colours, which would make them the same default colour as the editor - although it may rely on knowing where they are (I'm not sure if we currently get a range for "int.parse" here, or just two isolated ranges for "int" and "parse").

[FMorschel]

One more thing I'm not sure it should work but things like [List< int>] also have the same behaviour. I was expecting that to work but I'm unsure if it is only the same bug or if it is not designed for this. Does anyone here know (maybe @srawlins)?

[srawlins]

It does not work; the request for that feature is #47444.