Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
paulomorgado opened this issue Apr 4, 2017 · 22 comments
Assignees
Labels
Milestone

Comments

@paulomorgado
Copy link

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
Copy link
Contributor

jnm2 commented Apr 4, 2017

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
Copy link
Author

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
Copy link
Member

gafter commented Apr 5, 2017

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
Copy link
Author

Neither do I!

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

@aluanhaddad
Copy link

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

@sharwell
Copy link
Member

sharwell commented Apr 6, 2017

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
Copy link
Contributor

jnm2 commented Apr 6, 2017

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

@sharwell
Copy link
Member

sharwell commented Apr 6, 2017

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

@paulomorgado
Copy link
Author

@sharwell,

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

@jnm2
Copy link
Contributor

jnm2 commented Apr 6, 2017

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
Copy link
Author

⚡️ 💡

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

But it should be fixed.

@kwaclaw
Copy link

kwaclaw commented May 31, 2017

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
Copy link
Member

sharwell commented Aug 31, 2017

@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}}"/>

📝 This would likely render in documentation as:

Dictionary<string, List<object>>

@HaloFour
Copy link
Contributor

@sharwell

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

@sharwell
Copy link
Member

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

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 31, 2017

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
Copy link

@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
Copy link
Author

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
Copy link
Member

sharwell commented Sep 4, 2017

@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
Copy link
Author

@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
Copy link
Member

sharwell commented Sep 4, 2017

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]]

@DustinCampbell DustinCampbell self-assigned this Sep 17, 2018
@MadsTorgersen MadsTorgersen added this to the X.0 candidate milestone Sep 19, 2018
@MadsTorgersen MadsTorgersen added the Blocked Waiting for a dependency label Sep 19, 2018
@workgroupengineering
Copy link

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)
    {
     ...
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests