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.

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?)