Doc comment references cannot resolve an extension member when prefixed by the extended type's name

[srawlins]

Take this example:

extension E on String {
  void m() {}
}

Today, you can refer to m in a doc comment reference by the extension name (/// [E.m]), but not by the extended type's name (/// [String.m]).

I think it should be perfectly logical to refer to an extension member by the extended type's name. But there may be some quirks. E.g. given two extensions, E on List<int> and E on List<bool>, each with a member named m. What does /// [List.m] refer to? There are rules about resolving extension members and "specificity," which can probably apply without modification to comment references. We just have to start with a rule about instantiating the type to bounds. And probably make it non-nullable, so that it can refer to an extension E on String?.

[bwilkerson]

I have a question.: do the references in doc comments refer to a type or to a declaration?

I had always thought that they refer to a declaration, and that it didn't make sense. For example, we don't appear to support a reference such as /// [List<int>.add]. In a similar fashion, we don't support typedefs, so if I have typedef L = List;, a reference such as /// [L.add] isn't supported.

References to inherited members are supported, but I view that as showing that we support references to all of the members of a declaration, not just the locally defined ones, rather than showing that we support types.

If they do refer to a declaration rather than a type, then I think it's consistent that we don't support references to members on the extended type.

[srawlins]

Can you explain the difference? I'm not sure what "referring to a type" means in the context of /// [List<int>.add] or /// [L.add].

Can they refer to whichever one allows this feature? Do we have to remove support for something else if we allow this feature? In other words, what is the downside choosing the one that allows this feature?

[bwilkerson]

Can you explain the difference? I'm not sure what "referring to a type" means in the context of /// [List<int>.add] or /// [L.add].

Another way to ask the same question is: do the references in doc comments refer to a DartType or to an Element?

There is a class named List. There's an element for List, but List isn't a type. We allow users to write the "type" List in code, but that's just syntactic sugar for List<dynamic>.

I've always thought of [List.add] as being a reference to the member (element) named add on the class (element) List. And I've always thought of dartdoc as documenting elements, not types.

The reason I think it's important is because the answer to that question implies a lot (to me, at least) about what we should and shouldn't support.

If we're supporting elements, then the UX should be consistent with other references in code and it seems a lot better defined. If we're supporting types, then we have to answer more questions.

If we're supporting types, then not only should we support [L.add], but it seems like we should support other types, such as [List<int>.add]. And it raises the question of whether we should only support nominative types or whether we should try to support structural types for consistency (though I have no idea what that would look like).

But if we're only supporting elements, then I wouldn't support [<extendedType>.<extensionMember>] because those members (as you noted above) are defined on a type rather than on an element.

Can they refer to whichever one allows this feature? Do we have to remove support for something else if we allow this feature? In other words, what is the downside choosing the one that allows this feature?

I think I mostly answered those in the text above, but I think this feature is asking to support types in references rather than elements, I don't think we'd have to remove any other support, but I do think we'd need to add more support in order to be consistent with the change (or at least what I perceive to be a change). And I think that opens a can of worms that I'd rather leave closed, so that's one downside.

The other downside I see is that [List.m] can be ambiguous. If m is defined on extensions of List<int> and List<String>, the if we follow the language's rules, that ought to be flagged as being ambiguous and no navigation should be supported. But I think the ambiguity might be a bigger negative than not being able to use the extended type to refer to the member.

[srawlins]

I think I see. I think we should support types then, so that we can add enhancements like resolving [L.add] and [List<int>.add], and this feature here.

The other downside I see is that [List.m] can be ambiguous. If m is defined on extensions of List<int> and List<String>, the if we follow the language's rules, that ought to be flagged as being ambiguous and no navigation should be supported. But I think the ambiguity might be a bigger negative than not being able to use the extended type to refer to the member.

I think that supporting [List.m] where m is an extension member that is accessible on an expression of type List<dynamic> (that being the instantiated-to-bounds type), such as Iterable<int> or List<bool> is a good feature. And flagging an ambiguous [List.m] is a good thing. And that the ambiguity is only a good thing (since it's reported).

[srawlins]

See also https://github.com/dart-lang/sdk/issues/47444 where I mention some other expansions I think we should do on comment references.

[bwilkerson]

... so that we can add enhancements like resolving [L.add] and [List<int>.add], and this feature here.

I don't think the value proposition here is very high. For example, what does it add to support [List<int>.add]? That is, why is this better than the already supported [List.add]? Do we have user requests for this support? (Same question for using the extended type rather than the extension type.)

I can see more value for typedefs, because you might want to use a more domain-specific name, but I don't know how often users typedef one type to another like that.

[srawlins]

For example, what does it add to support [List.add]?

Consistency? If there is no problem with consistency then we don't need it. I thought you wanted it for consistency if we go the "refer to types" route.

(Same question for using the extended type rather than the extension type.)

Yes this is used internally. IMHO it is less helpful to document something like /// Use [TheNameOfAnExtensionThatYouOtherwiseNeverNeed.extensionMember]. It is more helpful to be able to write /// Use [String.extensionMember].

[bwilkerson]

For example, what does it add to support [List<int>.add]?

Consistency? If there is no problem with consistency then we don't need it.

In #47444 you wrote that you thought we ought to support "type literal with type arguments (foo<...>)", so I assumed you still wanted that. I just didn't understand why. Sounds like you might have changed your mind.

I thought you wanted it for consistency if we go the "refer to types" route.

Yes, I like consistency. I think that it's generally better from a UX perspective to be consistent. But I was actually using consistency as a reason to not go the "refer to types" route because I dislike the implications of that definition.

In particular, no, I don't want to support [List<int>.add] (or any other form that includes type arguments). I don't think it adds any significant value for us to support it. (And it's possibly a negative value if we aren't consistent in terms of navigation with how we handle type arguments outside of documentation.)

I do, however, see your point about extension types.

Perhaps if I squint just right I can convince myself that we're not opening the Pandora's box that allowing types would open and can live with the extension.

[srawlins]

In https://github.com/dart-lang/sdk/issues/47444 you wrote that you thought we ought to support "type literal with type arguments ([foo<...>])"

I still do want that, type literals. I've seen lots of people write [List<String>], like "returns a [List<String>] of the results" or something like that. I don't know that I've seen a request for [List<int>.add], but if it makes the text more readable, I'm in favor of supporting that.

[bwilkerson]

I've seen lots of people write [List<String>] ...

For the record, I don't think "sometimes people write x" to be a good argument for supporting that syntax, even when it's lots of people.

I'd rather we didn't go there, but I could imagine supporting it as being equivalent to [List]<[String]> (because that's how we navigate everywhere else that a type annotation with type arguments is found). But I really don't think supporting that syntactic form has enough value to make it worthwhile.

... but if it makes the text more readable, I'm in favor of supporting that.

I don't think that [List<int>.add] is more readable than [List.add]. I think it's harder to understand.

It's obvious that we have different ideas about this, and I doubt that I'll change your mind if I haven't already done so, so it's probably time for me to bow out gracefully.

[srawlins]

I was able to change some internal comments to reference the extension's name, so I don't see further request for this.

[srawlins]

CC @goderbauer

[goderbauer]

In a similar fashion, we don't support typedefs, so if I have typedef L = List;, a reference such as /// [L.add] isn't supported.

We are running into a similar problem with typedefs in Flutter. We had to rename a type to NewName. To not break a bunch of things we created a typedef for the OldName. Now, lots of doc comments still reference to properties via the OldName (e.g. [OldName.value]). Dartdoc is completely fine with this and resolves everything correctly (see the link to MaterialState.hovered in https://main-api.flutter.dev/flutter/material/MaterialStateUnderlineInputBorder-class.html, since MaterialState is a typedef for WidgetState it links there). However, the comment_references lint now complains that these references cannot be resolved. I think it should not complain about this und properly resolve them since oldName.value is a legal way of accessing that property.

cc @LongCatIsLooong, who ran into this issue.