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

Add support for <inheritdoc> cref #7656

Open
kuglitsd opened this issue Oct 29, 2024 · 4 comments
Open

Add support for <inheritdoc> cref #7656

kuglitsd opened this issue Oct 29, 2024 · 4 comments

Comments

@kuglitsd
Copy link

Product

Hot Chocolate

Version

14.0.0

Link to minimal reproduction

https://github.com/kuglitsd/HotChocolateInheritdoc

Steps to reproduce

Run the project.

Inheritdoc only seems to be working for Implementations of Interface Methods. Everything else does NOT seem to work

Image

What is expected?

The tag should correctly inherit and display the documentation comments from the specified members, classes, or interfaces in the schema

What is actually happening?

Only the comment inherited from the IQuery interface implementation seems to work.

Relevant log output

No response

Additional context

No response

@michaelstaib michaelstaib added this to the HC-14.2.0 milestone Nov 3, 2024
@glen-84 glen-84 modified the milestones: HC-14.2.0, Backlog Nov 5, 2024
@glen-84 glen-84 changed the title Inconsistent Inheritance of XML Documentation Comments with <inheritdoc> Tag Add support for <inheritdoc> cref Nov 5, 2024
@glen-84
Copy link
Collaborator

glen-84 commented Nov 5, 2024

It's not inconsistent (it's working for the TestInterface method because that method exists on the interface that Query implements), Hot Chocolate simply does not yet support cref.

I've changed the title and type of this issue accordingly.

@JSanchezIO
Copy link

Hey y'all, I was investigating this a bit and wanted to share my findings. It seems to me that this issue stems from this section of code:

private void ReplaceInheritdocElements(
MemberInfo member,
XElement? element)
{
if (element is null)
{
return;
}
var children = element.Nodes().ToList();
foreach (var child in children.OfType<XElement>())
{
if (string.Equals(child.Name.LocalName, _inheritdoc,
StringComparison.OrdinalIgnoreCase))
{
var baseType =
member.DeclaringType?.GetTypeInfo().BaseType;
var baseMember =
baseType?.GetTypeInfo().DeclaredMembers
.SingleOrDefault(m => m.Name == member.Name);
if (baseMember != null)
{
var baseDoc = GetMemberElement(baseMember);
if (baseDoc != null)
{
var nodes =
baseDoc.Nodes().OfType<object>().ToArray();
child.ReplaceWith(nodes);
}
else
{
ProcessInheritdocInterfaceElements(member, child);
}
}
else
{
ProcessInheritdocInterfaceElements(member, child);
}
}
}
}

By Line 310, we've validated that the child element represents an inheritdoc tag. However, the child element isn't checked for a cref attribute. Regardless, verifying the presence of a cref attribute and using its value to determine the documentation to copy presents another challenge. Specifically, GetMemberElement(MemberInfo member) uses its arguments, member.Module.Assembly, to determine which XML file to parse. This is problematic because the cref attribute only provides a string which could represent any kind of member, e.g., constructor, event, field, method, property. The value's variance makes it difficult to determine its assembly.

Here are some prospective solutions I thought of. Though, I imagine there are far better ones:

  • Introduce a property that allows consumers to specify additional XML file names
    • Within a new method called GetCrefElement(string cref) we could check each file sequentially
    • Alternatively, we could reference the XML file names and generate a merged XML file that is referenced for each lookup
  • To continue relying on assembly lookup we could, instead, require an additional property called assembly on <inheritdoc /> tags

Lastly, another bit of value could come from adding support for the optional path attribute that is available on <inheritodc /> tags.

@michaelstaib
Copy link
Member

If you want you can raise a PR.

@huysentruitw
Copy link
Contributor

huysentruitw commented Feb 7, 2025

We currently have this code in our custom doc-provider, but it is ugly and only supports Type-crefs. We have to scan all assemblies in the current app domain to find the type 😭

private MemberInfo? GetInheritdocBaseMember(XElement inheritdoc, MemberInfo member)
{
    var cref = inheritdoc.Attribute(Cref);
    if (cref is not null && !string.IsNullOrWhiteSpace(cref.Value))
    {
        if (!cref.Value.StartsWith("T:", StringComparison.Ordinal))
            return null; // We don't support other cref items like properties or methods yet.
        
        var typeName = cref.Value[2..];
               
        var type = _knownInheritdocAssemblies.ToArray()
            .Select(assembly => assembly.GetType(typeName, false))
            .FirstOrDefault(x => x != null);

        if (type is null)
        {
            type = AppDomain.CurrentDomain.GetAssemblies()
                .Select(assembly => assembly.GetType(typeName, false))
                .FirstOrDefault(x => x != null);
            
            if (type != null)
                _knownInheritdocAssemblies.Add(type.Assembly);
        }

        return type;
    }
    
    var baseType =
        member.DeclaringType?.GetTypeInfo().BaseType;

    var baseMember =
        baseType?.GetTypeInfo().DeclaredMembers
            .SingleOrDefault(m => m.Name == member.Name);
    
    return baseMember;
}

The reason we use this is so we don't have to duplicate our documentation on both commandhandlers and mutations, like this:

/// <inheritdoc cref="MyCommandHandler "/>
[Mutation]
public static async Task<MyCommandResult> MyMutation(
    MyCommandHandler commandHandler, MyCommand input, CancellationToken cancellationToken)
{
    return await commandHandler.Handle(input, cancellationToken);
}

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

No branches or pull requests

5 participants