api-doc-tools

api-doc-tools copied to clipboard

This time it's when type parameter names are different in generic methods, like:

    <Member MemberName="System.Linq.IQueryProvider.Execute&lt;S&gt;">
      <MemberSignature Language="C#" Value="S IQueryProvider.Execute&lt;S&gt; (System.Linq.Expressions.Expression expression);" />
      <MemberSignature Language="ILAsm" Value=".method hidebysig newslot virtual instance !!S System.Linq.IQueryProvider.Execute&lt;S&gt;(class System.Linq.Expressions.Expression expression) cil managed" />
      <MemberSignature Language="DocId" Value="M:System.Data.Linq.Table`1.System#Linq#IQueryProvider#Execute``1(System.Linq.Expressions.Expression)" />
      <MemberType>Method</MemberType>
      <AssemblyInfo>
        <AssemblyName>System.Data.Linq</AssemblyName>
        <AssemblyVersion>4.0.0.0</AssemblyVersion>
      </AssemblyInfo>
      <ReturnValue>
        <ReturnType>S</ReturnType>
      </ReturnValue>
      <TypeParameters>
        <TypeParameter Name="S" />
      </TypeParameters>
      <Parameters>
        <Parameter Name="expression" Type="System.Linq.Expressions.Expression" />
      </Parameters>
      <Docs>
        <typeparam name="S">To be added.</typeparam>
        <param name="expression">To be added.</param>
        <summary>To be added.</summary>
        <returns>To be added.</returns>
        <remarks>To be added.</remarks>
      </Docs>
    </Member>
    <Member MemberName="System.Linq.IQueryProvider.Execute&lt;TResult&gt;">
      <MemberSignature Language="C#" Value="TResult IQueryProvider.Execute&lt;TResult&gt; (System.Linq.Expressions.Expression expression);" />
      <MemberSignature Language="ILAsm" Value=".method hidebysig newslot virtual instance !!TResult System.Linq.IQueryProvider.Execute&lt;TResult&gt;(class System.Linq.Expressions.Expression expression) cil managed" />
      <MemberSignature Language="DocId" Value="M:System.Data.Linq.Table`1.System#Linq#IQueryProvider#Execute``1(System.Linq.Expressions.Expression)" />
      <MemberType>Method</MemberType>
      <AssemblyInfo>
        <AssemblyName>System.Data.Linq</AssemblyName>
        <AssemblyVersion>4.0.0.0</AssemblyVersion>
      </AssemblyInfo>
      <ReturnValue>
        <ReturnType>TResult</ReturnType>
      </ReturnValue>
      <TypeParameters>
        <TypeParameter Name="TResult" />
      </TypeParameters>
      <Parameters>
        <Parameter Name="expression" Type="System.Linq.Expressions.Expression" />
      </Parameters>
      <Docs>
        <typeparam name="TResult">To be added.</typeparam>
        <param name="expression">To be added.</param>
        <summary>To be added.</summary>
        <returns>To be added.</returns>
        <remarks>To be added.</remarks>
      </Docs>
    </Member>

How to repro: Clone this repo and run mdoc against dotnet folder. Above example will be generated in System.Data.Linq/Table`1.xml

They are not dupes. While they have the same signature, their type parameters are different:

      <ReturnValue>
        <ReturnType>S</ReturnType>
      </ReturnValue>
      <TypeParameters>
        <TypeParameter Name="S" />
      </TypeParameters>

@TianqiZhang @joelmartinez

Good eye @dend ... just used a different name for the typeparams. So the question is, do we leave it as is, since the C# and ILASM type signatures are different (even if the docid is the same, since it uses the tick notation for type params)?

Or should we do the same thing for ReturnValue and TypeParameters that we did for BaseTypeName in PR#34? where we add FrameworkAlternate when we encounter something different? It might work for the return type, but it might not be as easy for TypeParameter since there can be multiple of them ... so that makes the ordering kind of difficult.

@joelmartinez we use DocIDs to detect things, instead of the C#/IL signatures. Since we have two identical DocIDs, and we have the same DocIDs in the FrameworksIndex. I think it would be good to have something like FrameworkAlternate that can help us determine what to show for each moniker.

right ... so then we just have to figure out the semantics of how this will work

      <ReturnValue>
        <ReturnType>S</ReturnType>
        <ReturnType FrameworkAlternate="netcore">TResult</ReturnType>
      </ReturnValue>
      <TypeParameters>
        <TypeParameter Name="S" />
        <TypeParameter Name="TResult" FrameworkAlternate="netcore" />
      </TypeParameters>

That seems like it might be fine, but when there are multiple type parameters, it gets a bit more muddy

      <TypeParameters>
        <TypeParameter Name="S" />
        <TypeParameter Name="TResult" FrameworkAlternate="netcore" />
        <TypeParameter Name="K" />
      </TypeParameters>

which parameter above is the FrameworkAlternate for?

What if instead of putting FrameworkAlternate on the TypeParameter node, we put it on TypeParameters ? that way there's no confusion over what the type parameters are on a given framework?

     <TypeParameters>
        <TypeParameter Name="S" />
        <TypeParameter Name="K" />
      </TypeParameters>
     <TypeParameters FrameworkAlternate="netcore">
        <TypeParameter Name="TResult"  />
        <TypeParameter Name="K" />
      </TypeParameters>

And then we can keep it on ReturnType, since we only expect to have one of those xml nodes in ReturnValue. Thoughts on this proposal?

@joelmartinez sounds good - second proposal is reasonable.

@joelmartinez @dend just so that you know, I ran twice mdoc to verify if it's because the different type parameter name, and it seems NOT. Every time the mdoc ran, it inserts dup, even with the type parameter "S" already there.

I believe every member in a type xml should have unique DocId, it does not matter what type parameter name the member uses.

As an example, see the history of Table`1.xml: https://github.com/dotnet/docs/commits/CITest-standard20/xml/System.Data.Linq/Table%601.xml . You can see every "mdoc CI update" commit inserts the same dup.

I'd really appreciate it if you can get it fixed as soon as possible, it's so painful to manually fix the xml after CI, the automation is useless.

@joelmartinez @dend here's the commit of the manual fix, you can see I deleted two copies of dup: https://github.com/dotnet/docs/commit/a6ba057835b0025f7b067cd5d84ab671fdb9741f

@TianqiZhang yes, fully understood ... the work is in progress, just that its a bit tricky to nail down the functionality. Will let you guys know as soon as it's ready.

Reviewing items ... so we're going to be "monikerizing" the return type soon, which partially resolves this issue. However we still have to contend with the issue that we're including the generic type in the MemberName. So in the case of differing generic type names, we've got a mismatch issue; will be something to contend with.

Internal work item link: https://ceapex.visualstudio.com/Engineering/_workitems/edit/129109