docfx icon indicating copy to clipboard operation
docfx copied to clipboard

`inheritdoc` fails to inherit `param`s with mismatched types

Open rhys-vdw opened this issue 2 years ago • 16 comments

Operating System: Windows

DocFX Version Used: 2.59.3.0

Template used: default

Steps to Reproduce:

Inherit parameters from a function with the same parameter names, but different types:

    /// <summary>
    /// Create a new tween.
    /// </summary>
    /// <param name="from">The starting value.</param>
    /// <param name="to">The end value.</param>
    /// <param name="duration">Total tween duration in seconds.</param>
    /// <param name="onChange">A callback that will be invoked every time the tween value changes.</param>
    /// <returns>The newly created tween instance.</returns>
    public static Tween Tween(float from, float to, float duration, Action<float> onChange) =>
      CreateTween(from, to, duration, onChange);

    /// <inheritdoc cref="Tween" />
    public static Tween Tween(Vector2 from, Vector2 to, float duration, Action<Vector2> onChange) =>
      CreateTween(from, to, duration, onChange);

Expected Behavior:

Parameter descriptions are reused.

Actual Behavior:

Only parameters with matching types are inherited image

rhys-vdw avatar Jul 21 '22 19:07 rhys-vdw

@rhys-vdw That is the expected behavior.

paulushub avatar Jul 22 '22 01:07 paulushub

Ah, is there a spec?

rhys-vdw avatar Jul 22 '22 06:07 rhys-vdw

Ah, is there a spec?

The original purpose is about inherited methods (base classes and interfaces), https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/recommended-tags#inheritdoc

Tools including Sandcastle have made extensions (DocFX will issue warning on some of them) https://tunnelvisionlabs.github.io/SHFB/docs-master/SandcastleBuilder/html/79897974-ffc9-4b84-91a5-e50c66a0221d.htm

paulushub avatar Jul 22 '22 06:07 paulushub

I see nothing in there that says that the type information of the referenced member should be considered to filter the param tags. Since the tags themselves don't specify a type it seems reasonable that I could use this in this case.

You can specify elements in an external xml document that would have no type information by which to filter the params, so why should it apply this extra level of filtering here?

rhys-vdw avatar Jul 22 '22 09:07 rhys-vdw

I see nothing in there that says that the type information of the referenced member should be considered to filter the param tags.

See the C# specs of June 2022 from Page 591: https://www.ecma-international.org/wp-content/uploads/ECMA-334_6th_edition_june_2022.pdf

It does not contains any mention of inheritdoc, it is not one of the official stuff - just like some of the tags you will see on the Microsoft site. Most of these came from NDoc and Microsoft own documentations.

You will have to rely on the tools for the implementations. DocFX is a Doc-As-Code tool and will expectedly ensure the types. Sandcastle is an XPath tool and will pick anything bearing the matching name.

Here is the summary;

  • DocFX will check the type, name and position of the parameter.
  • Sandcastle will only check the name of the parameter.

If you need the Sandcastle style, you will most likely have to write a plugin.

paulushub avatar Jul 22 '22 11:07 paulushub

Okay, I think I understand what you mean. I feel that it should be possible regardless, but I'm not familiar with the internals. I'll investigate the plugin process.

rhys-vdw avatar Jul 22 '22 14:07 rhys-vdw

I'll investigate the plugin process.

Great, share your progress - I am investigating many stuff too. A search will reveal where to look https://github.com/dotnet/docfx/search?q=inheritdoc

paulushub avatar Jul 22 '22 14:07 paulushub

What if the C# types don't match, but the CLR types do?

public class C1 {
    /// <param name="o">Doc1</param>
    /// <param name="c">Doc2</param>
    public virtual void M(object o, params char[] c) {
    }
}
public class C2 : C1 {
    /// <inheritdoc/>
    public override void M(dynamic o, char[] c) {
    }
}

KalleOlaviNiemitalo avatar Jul 22 '22 14:07 KalleOlaviNiemitalo

What if the C# types don't match, but the CLR types do?

Depends on how ApiParameter.Type is created, test it.

paulushub avatar Jul 22 '22 15:07 paulushub

The current behaviour seems undesirable to me. Changing it won't cause issues because if you do have a param with a different meaning then you'll need to override it with a <param of the same name on the inheriting doc.

rhys-vdw avatar Jul 22 '22 15:07 rhys-vdw

The current behaviour seems undesirable to me.

As I pointed out in my understanding, the original purpose is for inherited methods (base classes and interfaces), not overloaded methods. If the actual field is byte for color and I provide overloaded methods with int and float to set it, it does not mean they have the same meaning even if they represent the same color component. I might need to point out that in the case for the float, the values must be between 0 ~ 1, and for int between 0 ~ 255.

inheritdoc is convenient but can easily be abused.

Microsoft (and other Big-Tech) have a whole documentation team, that anything goes example case is only desired in open source and small software companies.

paulushub avatar Jul 22 '22 16:07 paulushub

If the actual field is byte for color and I provide overloaded methods with int and float to set it, it does not mean they have the same meaning even if they represent the same color component. I might need to point out that in the case for the float, the values must be between 0 ~ 1, and for int between 0 ~ 255.

You can do this (and would have to in either case):

/// <summary>Accepts a color</summary>
/// <param name="color">A color value in range 0 ~ 1.</param>
void AcceptColor(float color) {

/// <param name="color">A color value in range 0 ~ 255.</param>
/// <inheritdoc cref="AcceptColor"/>
void AcceptColor(int color) {

There is no change in behaviour because the parameter would be overridden.

rhys-vdw avatar Jul 22 '22 16:07 rhys-vdw

There is no change in behaviour because the parameter would be overridden.

Voila! DocFX simply makes sure you always override if the types are not the same - best practice. There are even VS tools out there to automatically insert inheritdoc tags. So, a plugin will satisfy both worlds, except where it is not possible (like the IFilterVisitor) then you fork it.

Well, I have worked on Sandcastle for several years, was the first to document the sandcastle conceptual support with tools to get started, before it was supported in tools like SHFB and DocProject.

Sandcastle Conceptual Help: Quick Start

Together we can get something done here for DocFX beyond the original intents.

paulushub avatar Jul 22 '22 17:07 paulushub

It does not contains any mention of inheritdoc, it is not one of the official stuff

@paulushub, the ECMA-334 spec is many versions out of date at this point and while there is an attempt to bring it up to date (driven by https://github.com/dotnet/csharpstandard), it is missing many features that have been officially adopted in.

inheritdoc is one that is now officially recognized by the Roslyn compiler implementation and the various IDE toolings for VS and VS Code (I believe Rider as well). It may also get official inclusion by the language spec in the future. However, given that the "de facto" compiler implementation and tooling supports it, it is reasonable that docfx should as well and should mirror the relevant behavior and handling that already exists.

See https://github.com/dotnet/docfx/issues/7629 and related issues for how it should be handled. @sharwell is likely a good resource to get answers or ambiguities covered.

tannergooding avatar Jul 26 '22 18:07 tannergooding

Members that override a member defined in a base class

This is the one discussed here, and https://github.com/dotnet/docfx/issues/7629 does not provide any information on how it should be handled.

Members that implicitly implement an interface member Members that explicitly implement an interface member

These are not currently supported by DocFX and are clearly documented as such. Also, DocFX does not support automatic documentation. Finally, the support of roslyn in this case is limited, since this tool is only dealing with source codes.

NOTE: This is my view on why the last two are not supported and may not be or at least not completely in DocFX, the main limitation here is due to the CI feature. Sandcastle builds a large database to handle this, somebody needs to store this! For CI what do you do? store it with the project or as part of the docfx.console?

paulushub avatar Jul 26 '22 18:07 paulushub