api-doc-tools icon indicating copy to clipboard operation
api-doc-tools copied to clipboard

<param name=""> Empty param names in <Docs> node

Open TianqiZhang opened this issue 8 years ago • 19 comments

When using xml files from this folder , I found 9 files has empty param names in their <Docs> node, like this one from Microsoft.VisualC.StlClr.Generic\ReverseRandomAccessIterator`1.xml

<Docs>
    <typeparam name="TArg1">To be added.</typeparam>
    <typeparam name="TArg2">To be added.</typeparam>
    <typeparam name="TResult">To be added.</typeparam>
    <param name="">To be added.</param>
    <param name="">To be added.</param>
    <summary>To be added.</summary>
    <returns>To be added.</returns>
    <remarks>To be added.</remarks>
  </Docs>

First, there should not be any <param> node under Type/Docs, secondly, <param>'s name should not be empty.

TianqiZhang avatar Feb 27 '17 01:02 TianqiZhang

There are 9 xml files with this issue, there's the list:

Microsoft.VisualC.StlClr\\BinaryDelegate`3.xml
Microsoft.VisualC.StlClr.Generic\\ReverseRandomAccessIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ReverseBidirectionalIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ContainerRandomAccessIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ContainerBidirectionalIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ConstReverseRandomAccessIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ConstReverseBidirectionalIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ConstContainerRandomAccessIterator`1.xml
Microsoft.VisualC.StlClr.Generic\\ConstContainerBidirectionalIterator`1.xml

TianqiZhang avatar Feb 27 '17 01:02 TianqiZhang

First, there should not be any <param> node under Type/Docs

That is odd.

secondly, <param>'s name should not be empty.

That is also odd.

Would it be possible to get a copy of the assembly that produced this XML?

jonpryor avatar Feb 27 '17 14:02 jonpryor

I've sent a copy via email, @jonpryor

joelmartinez avatar Feb 27 '17 20:02 joelmartinez

I'm confused. :-)

What's odd is that mdoc from Mono 4.8 produces different behavior. When I run it locally:

$ mdoc update -o en Microsoft.VisualC.STLCLR.dll

There are only two mentions of <typeparam name="TResult">:

$ grep -rl '<typeparam name="TResult"' _en2
en/Microsoft.VisualC.StlClr/BinaryDelegate`3.xml
en/Microsoft.VisualC.StlClr/UnaryDelegate`2.xml

Only BinaryDelegate?3.xml contains <typeparam name="TArg1"> and <typeparam name="TArg2"> as well, so the only document I am able to find which resembles the original request is BinaryDelegate?3.xml.

Note, in particular, that ReverseRandomAccessIterator?1.xml does not contain the original specified fragment.

(Note: Using ? instead of backticks because GitHub Markdown doesn't support escaping backticks.)

This allows me to answer the original question:

First, there should not be any <param> node under Type/Docs

This is incorrect. There should be <param/> nodes under /Type/Docs, for delegate types. This is also why there are <typeparam/> nodes as well, and the <returns/> node. The /Type/Docs element on delegates contains documentation for the Invoke() method.

Other delegate methods cannot be documented.

This is in keeping with ECMA-335 XML, and with how delegates are documented at MSDN (e.g. System.Action, which doesn't show delegate members as distinct constructs, e.g. there is no BeginInvoke() documented).

secondly, <param>'s name should not be empty

I am not able to repro this with mdoc 4.8. BinaryDelegate?3.xml contains:

<Docs>
  <typeparam name="TArg1">To be added.</typeparam>
  <typeparam name="TArg2">To be added.</typeparam>
  <typeparam name="TResult">To be added.</typeparam>
  <param name="A_0">To be added.</param>
  <param name="A_1">To be added.</param>
  <summary>To be added.</summary>
  <returns>To be added.</returns>
  <remarks>To be added.</remarks>
</Docs>

This matches what the IL contains for BinaryDelegate?3.Invoke():

  .class public auto ansi sealed beforefieldinit BinaryDelegate`3<TArg1,TArg2,TResult>
        extends [mscorlib]System.MulticastDelegate
  {
...
    .method public final virtual newslot 
           instance default !TResult Invoke (!TArg1 A_0, !TArg2 A_1)  runtime managed fwdref 
...

Perhaps this is a macOS vs. Windows behavioral difference, and that's why parameter names aren't being extracted?

jonpryor avatar Feb 27 '17 20:02 jonpryor

I was unable to reproduce this issue, and got the same results as @jonpryor above.

@TianqiZhang/@dend ... this XML was generated using an earlier preview. Can you use the latest release (v5.0.0.7) and let me know if this issue is still present?

joelmartinez avatar Mar 07 '17 22:03 joelmartinez

Seems like a non-repro in my case. @TianqiZhang please confirm once I send you the updated instructions.

dend avatar Mar 13 '17 07:03 dend

gonna close this unless we hear back that it's still present :)

joelmartinez avatar Mar 13 '17 18:03 joelmartinez

It's still present. See here for an example: https://github.com/dotnet/dotnet-api-docs/blob/master/xml/Microsoft.VisualC.StlClr.Generic/ConstContainerRandomAccessIterator%601.xml#L586

This will cause issues for IntelliSense in VS for our next release coming up.

Can we reopen this @dend @joelmartinez? In CAPS, these empty params were called __unnamed0.

mairaw avatar Nov 22 '18 04:11 mairaw

@mairaw Yeah … we probably do need to handle this more gracefully. I've sent an email to @terrajobst with some questions to help us improve our knowledge of this situation. We are either going to have to adopt one of these naming conventions, (A_0, param0, __unnamed0), or extend the data structure so that the param node includes the parameter index, to help consumers figure out what docs it's for.

joelmartinez avatar Nov 26 '18 15:11 joelmartinez

Cool. I pinged @terrajobst and a couple of the C++ docs folks to see if they can help us decide.

mairaw avatar Dec 05 '18 01:12 mairaw

My current inclination is to add the parameter's Index to the param node to indicate which parameter the documentation is for. There will have to be additional tasks for ecma2yaml and the templates to correctly identify/link the param to the Parameter in these cases.

Additionally, we'll have to adjust the intellisense export subcommand so that it generates the correct docxml in these instances.

/fyi @wh-alice

joelmartinez avatar Feb 22 '19 19:02 joelmartinez

Right now, there seems to be a bug in the ecma2yaml as well, since it doesn't identify the empty params anyway and it will block the MSDN migration. Even when we add description for the empty params, they don't render on docs.

See https://github.com/dotnet/dotnet-api-docs/pull/1927 for an example.

mairaw avatar Feb 22 '19 20:02 mairaw

@mairaw the reason why ecma2yaml is filtering out empty params is that, param name is the only key we can associate the description with the param itself. Without the name, we don't know which description belong to which param.

So my suggestion is that we give the empty param a name in ECMAXML so it looks like a normal param. //cc @joelmartinez

TianqiZhang avatar Feb 25 '19 02:02 TianqiZhang

I think we're all in agreement here that we should give these a name. Since this will block migration because the content won't be able to show up causing a compliance issue, I'd prioritize this soon.

mairaw avatar Feb 25 '19 20:02 mairaw

well, not exactly … my proposal is to explicitly avoid a name, because any fake name we give it would not be usable to call that member (in reflection code, for example). My proposal is to use the Index, so that in cases where it doesn't have a name, it would just be <param Index="1">the docs</param>. To me this is the more proper approach since it's not inventing a name that can't actually be used.

I agree that we should prioritize this soon, but we should bring it up on the PM side

joelmartinez avatar Feb 25 '19 20:02 joelmartinez

@joelmartinez using Index make sense to me. Once every param is assigned a Index, I can update ECMA2Yaml to use it.

TianqiZhang avatar Feb 27 '19 03:02 TianqiZhang

As I said offline, I don't care too much about the implementation detail.

But how are we going to show the param names on the param list? See an example

image

mairaw avatar Mar 05 '19 23:03 mairaw

I say we simply use the type name … so if it's a String, we show string, if it's a generic type parameter, we show T, or whatever. Or perhaps something like T (no name) ?

Personally speaking, I'd be against making up a fake parameter name (param 0, etc), because that "name" isn't usable, and could lead a developer to make incorrect assumptions.

joelmartinez avatar Mar 06 '19 03:03 joelmartinez

Internal tracking issue logged

joelmartinez avatar Mar 29 '19 00:03 joelmartinez