dotnet / csharplang

The official repo for the design of the C# programming language
11.43k stars 1.02k forks source link

CS1584+CS1658/CS0081 when using constructed generic types in XML comments #401

Open paulomorgado opened 7 years ago

paulomorgado commented 7 years ago

This code:

/// <summary>
/// A <see cref="TaskCompletionSource{IReadOnlyDictionary{string, object}}" /> object that handles the completion of the state machine execution.
/// </summary>
private readonly TaskCompletionSource<IReadOnlyDictionary<string, object>> completion;

Generates 4 CS1658 warning caused by a CS0081 error in the comment line warnings both in VS2015 and VS2017 and an extra CS1584 warning in VS2017.

By the way, where is the documentation for these errors/warnings?

jnm2 commented 7 years ago

Constructed generic types are not supported in XML documentation, to my irritation. Though in your example copying the entire type into the documentation does seem like a waste since people will be viewing the type twice in a row now, no matter where they are reading the documentation.

paulomorgado commented 7 years ago

to my irritation Our irritation! 😄

I'm trying to work out phrasing that inform unexperienced developers that asynchronous methods return a Task and everyone that the important part is the Task<T>.Result.

gafter commented 7 years ago

I'm not sure if this is intended as a language proposal, a language question, or if it belongs in the Roslyn repo as a comment about the behavior of the compiler.

paulomorgado commented 7 years ago

Neither do I!

Should that be wrong? What does the spec say about that?

aluanhaddad commented 7 years ago

This is definitely a pain point. I would love to see support for this.

sharwell commented 7 years ago

Currently the compiler does not allow a documentation comment to directly reference an instantiated generic type. For the purpose of hyperlinking in the resulting documentation, this seems to make sense. However, documentation includes more than just a simple hyperlink. I can certainly see value in the ability to reference an instantiated generic type which displays using the type as-referenced, but links to the original type definition.

From what I can tell, the spec neither requires nor forbids this ability. I believe it would make sense to formalize the desired behavior as a language proposal prior to implementing it within one specific compiler.

jnm2 commented 7 years ago

As far as hyperlinking goes, I'd assume it would hyperlink to the .GetGenericTypeDefinition() of the type.

sharwell commented 7 years ago

@jnm2 Yes, that's what I meant by linking to the original type definition.

paulomorgado commented 7 years ago

@sharwell,

I does allows IReadOnlyDictionary{string, object}. Just doesn't allow TaskCompletionSource{IReadOnlyDictionary{string, object}}.

jnm2 commented 7 years ago

Thought a documentation generator could be free to, if it wanted, insert links inside links exactly as what happens when you'd go to definition in the IDE: IReadOnlyDictionary<string, MyType>

@paulomorgado It actually doesn't allow IReadOnlyDictionary{string, object} the way you're thinking; it just aliases TKey as string and TValue as object. It doesn't show String for VB.NET and string for C#, for example. Importantly, it doesn't allow a . in the type parameter alias, as in, Task<System.String>, because no type parameter can be named System.String.

paulomorgado commented 7 years ago

⚡️ 💡

Now it makes sense why it's doing what's doing.

But it should be fixed.

kwaclaw commented 7 years ago

The current SandCastle help compiler already does the desired thing for constructed generic types when they are read from the assembly and not from the doc comment. E.g. a return type of IDictionary<string, object> is shown as the same, but the link points to the documentation for the generic type IDictionary<TKey, TValue>. Extending this behaviour to doc comments would be consistent.

sharwell commented 7 years ago

@EWSoftware I'm interested in implementing a prototype of this feature. Given a reference like the following, do you have any thoughts regarding an output syntax which would work well with SHFB? Modification to SHFB will probably be necessary eventually, but I'm curious about what would be easiest to accommodate.

<see cref="Dictionary{string, List{object}}"/>

:memo: This would likely render in documentation as:

Dictionary<string, List<object>>

HaloFour commented 7 years ago

@sharwell

Wouldn't you have to escape at least the < characters within the attribute?

sharwell commented 7 years ago

@HaloFour Yes that was an oversight. Corrected in my example.

Joe4evr commented 7 years ago

I've really wanted this for a while, but now that I think about it: 💭 Is there also something that could be done for partially constructed generic types? So given for example

<see cref="Dictionary{string, TValue}"/>

would TValue render to anything? What about when TValue is a type parameter on the containing type? This is probably a whole other can of worms, but I thought I'd throw it in for consideration.

EWSoftware commented 7 years ago

@sharwell cref can only target one ID. Unless your proposing changing the structure of the see element or replacing it with something else, I think the simplest approach is that the compiler rewrites such targets in the XML comments file as a series of see elements for each of the referenced types. Of course, if inner text is specified, then that wouldn't happen and it would use the given inner text instead. For your example above:

<see cref="Dictionary{string, List{object}}"/>

It would get rewritten to the XML comments file as:

<see cref="System.Collections.Generic.Dictionary{TKey, TValue}">Dictionary</see>&lt;
<see cref="System.String" />, <see cref="System.Collections.Generic.List{T}">List</see>&lt;
<see cref="System.Object" />&gt;&gt;

If a type parameter is given instead of a type (i.e. TValue) then it would just be rendered as literal text.

That's effectively what you have to do now by hand if you want it to read that way in the topic. Since it's just a series of see elements interspersed with literal text, nothing would need changing as far as SHFB or the presentation styles.

paulomorgado commented 7 years ago

If it's not possible to change cref, for the short term, I would be happy with a convention that we could agree upon for constructed or partially constructed types.

sharwell commented 7 years ago

@paulomorgado I believe @EWSoftware was talking about the form of a cref attribute as it appears in the output XML file. The form that appears in the original source code can vary as long as the compiler transforms it to an output that tools down the line will understand.

paulomorgado commented 7 years ago

@sharwell, yes, I understood it. But changing that requires that the tool that you use changes to account for that. If that's not possible, we should come up with a convention to document constructed or partially constructed types.

This is good enough for C#, but not for F#, VB or any other language:

<see cref="System.Collections.Generic.Dictionary{TKey, TValue}">Dictionary</see>&lt;
<see cref="System.String" />, <see cref="System.Collections.Generic.List{T}">List</see>&lt;
<see cref="System.Object" />&gt;&gt;
sharwell commented 7 years ago

Currently, Dictionary<TKey, TValue> would appear as:

T:System.Collections.Generic.Dictionary`2

The generic parameters are actually omitted.

It would be interesting to emit the following for a constructed generic type:

T:System.Collections.Generic.Dictionary`2[[System.Int32, System.String]]
workgroupengineering commented 1 year ago

Similar Issue

    /// <summary>
    ///     Executes the specified <see cref="Func{Task{TResult}}"/> asynchronously on the
    ///     thread that the Dispatcher was created on
    /// </summary>
    /// <typeparam name="TResult">The type of retur value of <paramref name="action"/></typeparam>
    /// <param name="action">
    ///     A <see cref="Func{Task{TResult}}"/> delegate to invoke through the dispatcher.
    /// </param>
    /// <param name="priority">
    ///     The priority that determines in what order the specified
    ///     callback is invoked relative to the other pending operations
    ///     in the Dispatcher.
    /// </param>
    /// <returns>
    ///     An task that completes after the task returned from callback finishes
    /// </returns>
    public Task<TResult> InvokeAsync<TResult>(Func<Task<TResult>> action, DispatcherPriority priority = default)
    {
     ...
    }