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

Formatters are ignoring generic parameter attributes

Open mpMelnikov opened this issue 8 years ago • 7 comments

Based on #131 discussion CSharpFormatter ignores generic parameter attributes. Can be reproduced on the assembly compiled from mdoc/Test/DocTest-v1.cs file.

  1. csc DocTest-v1.cs /unsafe
  2. mdoc update -o en DocTest-v1.dll
// Input C# code
public U BaseMethod<[Doc ("S")] S> (S genericParameter) 
<!-- Output xml -->
<MemberSignature Language="C#" Value="public U BaseMethod&lt;S&gt; (S genericParameter);" />
<MemberSignature Language="ILAsm" Value=".method public hidebysig instance !U BaseMethod&lt;S&gt;(!!S genericParameter) cil managed" />

mpMelnikov avatar Oct 19 '17 10:10 mpMelnikov

@dend this was a pre-existing bug discovered during development. Do you want to make this a part of milestone#3, or address at a later date?

joelmartinez avatar Oct 19 '17 14:10 joelmartinez

@joelmartinez we should address this for M3

dend avatar Oct 19 '17 15:10 dend

@joelmartinez While investigating F# signatures, I've found there is no such a bug. When I reported it, I considered that attributes should appear in the signature, but really MDocUpdater class adds them to the <Attribute> node.

<TypeParameters>
        <TypeParameter Name="S">
          <Attributes>
            <Attribute>
              <AttributeName>Mono.DocTest.Doc("S")</AttributeName>
            </Attribute>
          </Attributes>
        </TypeParameter>
</TypeParameters>

So, I suppose it should be closed. Sorry for the misinformation

mpMelnikov avatar Nov 12 '17 14:11 mpMelnikov

@mpMelnikov Actually ... this is a really good question. For attributes applied to members, we maintain them separately because they are just appended on the front-end. However, for parameters it would be a lot more difficult to do so, because it would require parsing the signature and doing string manipulation to insert them.

I think there might be value in adding them to the actual string signature for that reason ... however, we have to be sure that we don't inadvertently add attributes that are only added "behind the scenes" by the compiler (ie. they don't appear in the source code).

@dend ... would love to hear your thoughts on this. I'm inclined to maintain this feature request open and have it implemented, as I think it will offer a more proper reconstruction of the source code.

joelmartinez avatar Nov 16 '17 15:11 joelmartinez

@joelmartinez I suppose, in this case, the ticket should be extended to method parameters' and return types' attributes in addition to generic type parameters as all of them are handled in the same way.

And it looks like quite a task because we will have to copy some attributes logic from MDocUpdater to MemberFormatter and modify it for each MemberFormatter implementation.

Now, none of these attributes is presented in signatures:

[return:Doc ("return:DocAttribute", Property=typeof(Widget))]
[Doc("normal DocAttribute", Field=true)]
public void M1 ([Doc ("c", FlagsEnum=ConsoleModifiers.Alt | ConsoleModifiers.Control)] char c, 
	[Doc ("f", NonFlagsEnum=Color.Red)] out float f, 
	[Doc ("v")] ref DocValueType v) {f=0;}
<Member MemberName="M1">
      <MemberSignature Language="C#" Value="public void M1 (char c, out float f, ref Mono.DocTest.DocValueType v);" />
      <MemberSignature Language="ILAsm" Value=".method public hidebysig instance void M1(char c, [out] float32&amp; f, valuetype Mono.DocTest.DocValueType&amp; v) cil managed" />
      <MemberSignature Language="F#" Value="member this.M1 : char *  *  -&gt; unit" Usage="widget.M1 (c, f, v)" />
      <MemberType>Method</MemberType>
      <AssemblyInfo>
        <AssemblyVersion>0.0.0.0</AssemblyVersion>
      </AssemblyInfo>
      <Attributes>
        <Attribute>
          <AttributeName>Mono.DocTest.Doc("normal DocAttribute", Field=true)</AttributeName>
        </Attribute>
      </Attributes>
      <ReturnValue>
        <ReturnType>System.Void</ReturnType>
        <Attributes>
          <Attribute>
            <AttributeName>Mono.DocTest.Doc("return:DocAttribute", Property=typeof(Mono.DocTest.Widget))</AttributeName>
          </Attribute>
        </Attributes>
      </ReturnValue>
      <Parameters>
        <Parameter Name="c" Type="System.Char">
          <Attributes>
            <Attribute>
              <AttributeName>Mono.DocTest.Doc("c", FlagsEnum=System.ConsoleModifiers.Alt | System.ConsoleModifiers.Control)</AttributeName>
            </Attribute>
          </Attributes>
        </Parameter>
        <Parameter Name="f" Type="System.Single&amp;" RefType="out">
          <Attributes>
            <Attribute>
              <AttributeName>Mono.DocTest.Doc("f", NonFlagsEnum=Mono.DocTest.Color.Red)</AttributeName>
            </Attribute>
          </Attributes>
        </Parameter>
        <Parameter Name="v" Type="Mono.DocTest.DocValueType&amp;" RefType="ref">
          <Attributes>
            <Attribute>
              <AttributeName>Mono.DocTest.Doc("v")</AttributeName>
            </Attribute>
          </Attributes>
        </Parameter>
      </Parameters>
      <Docs>
        <param name="c">To be added.</param>
        <param name="f">To be added.</param>
        <param name="v">To be added.</param>
        <summary>To be added.</summary>
        <remarks>To be added.</remarks>
      </Docs>
    </Member>

mpMelnikov avatar Nov 16 '17 16:11 mpMelnikov

That's a really good point ... it's definitely not a small/simple task

joelmartinez avatar Nov 16 '17 16:11 joelmartinez

I'd say we should keep this feature request open, but at this time (and per our conversation with @BillWagner), this does not seem like a blocker.

dend avatar Nov 27 '17 16:11 dend