Parameters of Array Type Report 'No XML doc comment found'

[lordmilko]

I have found what appears to be a bug, wherein XmlDoc2CmdletDoc will report "No XML doc comment found" for a parameter when the parameter's type is an array.

The following program generates the error

/// <summary>
/// <para type="description">Something!</para> 
/// </summary>
public class Random
{
}

[Cmdlet(VerbsCommon.Get, "Fail")]
public class Class1 : PSCmdlet
{
    /// <summary>
    /// <para type="description">Hello?</para> 
    /// </summary>
    [Parameter]
    public Random[] Param { get; set; }
}

I have found the issue appears to be caused by the Redgate Jolt library, specifically method AppendXDCFullTypeNameTo in Convert.cs on line 401.

private static StringBuilder AppendXDCFullTypeNameTo(StringBuilder builder, Type type)
{
    if (type.IsGenericType) { type = type.GetGenericTypeDefinition(); }
    return AppendNormalizedXDCTypeNameTo(builder, type);
}

I believe this method should be modified to also check whether or not the type is an array, as follows

private static StringBuilder AppendXDCFullTypeNameTo(StringBuilder builder, Type type)
{
    if (type.IsGenericType)
    {
        type = type.GetGenericTypeDefinition();
    }
    else if (type.IsArray)
    {
        type = type.GetElementType();
    }
    return AppendNormalizedXDCTypeNameTo(builder, type);
}

A potential complexity with this however is that I can see on line 533 of Convert.cs there is a method ReduceToElementType which also appears to be capable of determining the underlying type of an array, however this method does not appear to be invoked by the method chain XmlDoc2CmdletDoc uses.

For reference, the following call stack shows the path to AppendXDCFullTypeNameTo when we need to parse an array.

Jolt.dll!Jolt.Convert.AppendXDCFullTypeNameTo(System.Text.StringBuilder builder, System.Type type) Line 409
Jolt.dll!Jolt.Convert.ToXmlDocCommentMember(System.Type type) Line 62
Jolt.dll!Jolt.XmlDocCommentReader.GetComments(System.Type type) Line 214
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Comments.JoltCommentReader.GetComments(System.Type type) Line 26
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Comments.LoggingCommentReader.GetComments(System.Type type) Line 31
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Comments.RewritingCommentReader.GetComments(System.Type type) Line 28
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Comments.CachingCommentReader.GetComments(System.Type type) Line 31
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Extensions.CommentReaderExtensions.GetTypeDescriptionElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, System.Type type, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 338
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateTypeElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, System.Type type, bool includeMamlDescription, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 560
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateParameterElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, XmlDoc2CmdletDoc.Core.Domain.Parameter parameter, string parameterSetName, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 387
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateSyntaxItemElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, XmlDoc2CmdletDoc.Core.Domain.Command command, string parameterSetName, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 332
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateSyntaxElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, XmlDoc2CmdletDoc.Core.Domain.Command command, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 310
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateCommandElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, XmlDoc2CmdletDoc.Core.Domain.Command command, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 249
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateHelpItemsElement(XmlDoc2CmdletDoc.Core.Comments.ICommentReader commentReader, System.Collections.Generic.IEnumerable<XmlDoc2CmdletDoc.Core.Domain.Command> commands, XmlDoc2CmdletDoc.Core.ReportWarning reportWarning) Line 235
XmlDoc2CmdletDoc.Core.dll!XmlDoc2CmdletDoc.Core.Engine.GenerateHelp(XmlDoc2CmdletDoc.Core.Options options) Line 57
XmlDoc2CmdletDoc.exe!XmlDoc2CmdletDoc.Program.Main(string[] args) Line 31

While this appears to be an issue with the Jolt module used by XmlDoc2CmdletDoc, as Redgate's repo for it does not have an issues page, I am not familiar with what Jolt is typically used for, and @ChrisLambrou appears to be a major contributor to both projects, I am opening an issue here.

Arrays of builtin types do not generate this warning, however this is because XmlDoc2CmdletDoc filters out errors for types outside of the assembly being processed.

Any assistance on this matter would be greatly appreciated.

Regards,
lordmilko

[ChrisLambrou]

Hi,

Thanks for reporting this in such detail. I'm afraid I'm actually not all that familiar with the Jolt library. The Redgate version of it is simply a copy of the original core library with a small bug fix, since the original author was unresponsive. I'll try and make time to investigate this in the next couple of days, but unfortunately you've caught me at a rather busy time (moving house), so I can't promise a rapid turnaround.

Regards,
Chris

[ChrisLambrou]

I had a play with this over the weekend, but I wan't able to repeat the problem. I duplicated the following code, and the tool works fine.

/// <summary>
/// <para type="description">Something!</para> 
/// </summary>
public class Random
{
}

[Cmdlet(VerbsCommon.Get, "Fail")]
public class Class1 : PSCmdlet
{
    /// <summary>
    /// <para type="description">Hello?</para> 
    /// </summary>
    [Parameter]
    public Random[] Param { get; set; }
}

The help text, "Hello?", is correctly obtained from the Param property's XML comment, and the tool doesn't go as far as trying to retrieve the comment from the Random class.

However... if you remove the comment from the Param property, the tool will attempt to fall back to any comment present for the type of the parameter (in this case, Random[]). This is where it's probably failing to infer the Random type from Random[].

I'll investigate this further. In the meantime, I suggest that you ensure all of your properties have their own explicit <para type="description">...</para> comments. I'm actually rather regretting having provided the fallback mechanism that takes help text from a type if it's unavailable from a parameter or output type. In fact, I'm thinking of removing this "feature". In hindsight, I can't think of a single good case for using a general type description rather than some crafted text to explain the context in which the type is used.

In other words, from your example, it's far better to provide text that explains why and how your Param parameter is used, rather than relying on some general text to describe was a Random is.

[lordmilko]

Hi Chris,

Thanks for your time looking into this. I can see in the -Help.xml file that is output the "Hello?" text is correctly inserted into the XML, however it appears the build log still generates an warning stating there was an error

1>------ Rebuild All started: Project: XmlDocTest, Configuration: Debug Any CPU ------
1>  XmlDocTest -> Z:\XmlDocTest\XmlDocTest\bin\Debug\XmlDocTest.dll
1>  AssemblyPath: Z:\XmlDocTest\XmlDocTest\bin\Debug\XmlDocTest.dll, OutputHelpFilePath: Z:\XmlDocTest\XmlDocTest\bin\Debug\XmlDocTest.dll-Help.xml, DocCommentsPath: Z:\XmlDocTest\XmlDocTest\bin\Debug\XmlDocTest.xml, TreatWarningsAsErrors False
1>  Warnings:
1>      XmlDocTest.Class1:
1>          No description comment found.
1>          No examples found.
1>      XmlDocTest.Random[]:
1>          No XML doc comment found.
1>          No description comment found.
1>  GenerateHelp completed with exit code 'Success'
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========

        <command:parameter required="false" globbing="false" pipelineInput="false" position="named">
          <maml:name>Param</maml:name>
          <maml:description>
            <maml:para>Hello?</maml:para>
          </maml:description>
          <command:parameterValue required="true">Random[]</command:parameterValue>
          <dev:type>
            <maml:name>XmlDocTest.Random[]</maml:name>
            <maml:uri />
          </dev:type>
        </command:parameter>

I have uploaded my solution here

From my perspective, I always put descriptions on both the types and the parameters, as XmlDoc2CmdletDoc throws warnings without them. My goal is to have my build throw no warnings at all

Regards,
lordmilko

--

Just to add to this, looking at method GenerateParameterElement on line 385 of XmlDoc2CmdletDoc.Core/Engine.cs, a cursory glance indicates it attempts to generate both the description element and the type element, without performing a check if the latter is required

private XElement GenerateParameterElement(ICommentReader commentReader, Parameter parameter, string parameterSetName, ReportWarning reportWarning)
{
    var element = new XElement(CommandNs + "parameter",
        ...
        GenerateDescriptionElement(commentReader, parameter, reportWarning),
        ...
        GenerateTypeElement(commentReader, parameter.ParameterType, true, reportWarning),
        ...
    );
...

[ChrisLambrou]

Ah, sorry. I'm afraid I didn't look in quite so much detail. I'm trying to move house at the moment, so am rather busy. I think I'm probably going to go ahead and remove support for documenting types, as it encourages sloppy documentation. I need to examine it in further detail, first, but it should eliminate this class of warning for you.

[lordmilko]

When it comes to parameters, documenting types should be unnecessary, however I believe documenting types is till required for Input/Output documentation sections found in get-help -full. As such support for that should not be removed completely.

Regards,
lordmilko

[ChrisLambrou]

Yeah, I think you're right. Unless and until I add support for explicitly documenting cmdlet output types, this feature will have to remain.

[ChrisLambrou]

Okay, I've done some more playing around with the Jolt code and tests, and I think I've come to the conclusion that it's behaving correctly, and our problem lies elsewhere. In particular, Jolt's only concern is trying to convert generic types into the kind of sequences that appear in XML doc comment output files. (e.g. the type System.Action<int, bool, char, string> would be turned into T:System.Action`4). This allows it to locate the XML doc comment for a given type by working out which <member/> element to match. From your example, typeof(Random) would be converted to T:XmlDocTest.Random, which would then match the <member name="T:XmlDocTest.Random">...</member> in the XML documentation file. The special treatment of generic types is only relevant to being able to resolve actual generic types, not properties.

In short, I don't think your proposed fix will work, and it's behaviour is incorrect anyway:

private static StringBuilder AppendXDCFullTypeNameTo(StringBuilder builder, Type type)
{
    if (type.IsGenericType)
    {
        // For example, this would convert IList<int> to IList<>.
        type = type.GetGenericTypeDefinition();
    }
    else if (type.IsArray)
    {
        // For example, this would convert Random[] to Random, but it's wrong to do so
        // given the purpose of this method, which is to remove generic specificity
        // rather than dereference container types.
        type = type.GetElementType(); // WRONG!
    }
    return AppendNormalizedXDCTypeNameTo(builder, type);
}

I think the real fix should lie in XmlDoc2CmdletDoc. Essentially, we want to dereference container types whenever we perform a doc comment lookup. For example:

1. Given Random, we want the XML doc for Random, obviously.
2. Given IList<Random>, we really want the XML doc for Random.
3. Given Random[], we really want the XML doc for Random.

Currently, only case 1 actually works. Cases 2 and 3 do not (where case 3 exactly describes your situation). I think this can be addressed by introducing another implementation of ICommentReader alongside one of the existing implementations. The new implementation would basically unwrap container types (i.e. cases 2 and 3) and use the resulting contained type. I'll try to find time to have a crack at this tonight.

I think it's right to fix this problem here in XmlDoc2CmdletDoc. This unwrapping behaviour is really specific to XmlDoc2CmdletDoc, not the Jolt library.

[ChrisLambrou]

Issue #37 and PR #38 also relate to this issue. I've committed a partial fix, but this issue remains open.