Skip to content

chore: Add support for derivedClasses metadata when implements interfaces #10632

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

filzrev
Copy link
Contributor

@filzrev filzrev commented Apr 14, 2025

This PR intended to fix a part of issue that is reported at #2531

Currently docfx generate derivedClasses metadata when inherited from class.
But derivedClasses metadata is not outputted for interface implements.

This PR add derivedClasses support for interfaces.

Additionally, Replace dictionary lookup logics to use CollectionsMarshal.GetValueRefOrAddDefault
to reduce dictionary double-lookup costs.

Test
I've manually confirmed derivedClasses metadata generation with following sample code.
And confirmed metadata is generated as expected.

namespace BuildFromProject;

public interface ISample{}

public abstract class SampleClassBase :ISample {}

public class Sample01 :ISample
{
    public int AAA{get;set;}
}

public class Sample02: ISample
{
    public int BBB{get;set;}
}

public class Sample03: SampleClassBase
{
    public int BBB{get;set;}
}

TODO
Currently derivedClasses metadata information is not rendered for interface HTML page.
It need to modify docfx templates also (by another PR)

@yufeih yufeih requested a review from Copilot May 21, 2025 09:28
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for derivedClasses metadata when types implement interfaces and optimizes dictionary lookups using CollectionsMarshal.GetValueRefOrAddDefault.

  • Introduces derivedClasses entries in interface snapshot JSON files
  • Extends UpdateDerivedClassMapping to register classes under implemented interfaces
  • Refactors dictionary additions to use GetValueRefOrAddDefault and updates AppendDerivedClass to include interfaces

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/docfx.Snapshot.Tests/SamplesTest.Seed/api/CatLibrary.IAnimal.html.view.verified.json Added derivedClasses array for IAnimal
test/docfx.Snapshot.Tests/SamplesTest.Seed/api/BuildFromProject.Inheritdoc.Issue9736.IJsonApiOptions.html.view.verified.json Added derivedClasses array for IJsonApiOptions
test/docfx.Snapshot.Tests/SamplesTest.Seed/api/BuildFromProject.Class1.IIssue8948.html.view.verified.json Added derivedClasses array for IIssue8948
test/docfx.Snapshot.Tests/SamplesTest.CSharp/api/CSharp11.StaticAbstractMembersInInterfaces.IGetNext-1.html.view.verified.json Added derivedClasses array for IGetNext
src/Docfx.Dotnet/ManagedReference/Resolvers/SetDerivedClass.cs Added interface handling in UpdateDerivedClassMapping, refactored dictionary logic, and updated AppendDerivedClass to include interfaces
Comments suppressed due to low confidence (2)

src/Docfx.Dotnet/ManagedReference/Resolvers/SetDerivedClass.cs:52

  • [nitpick] The variable name superClass is ambiguous in the context of interfaces. Rename it to something like interfaceName to clarify its purpose.
var superClass = implements[implements.Count - 1];

src/Docfx.Dotnet/ManagedReference/Resolvers/SetDerivedClass.cs:23

  • No automated tests were added to verify derivedClasses generation for interfaces. Consider adding unit tests covering classes that implement one or multiple interfaces to ensure correct metadata output.
private void UpdateDerivedClassMapping(List<MetadataItem> items, Dictionary<string, List<string>> _derivedClassMapping)

@filzrev filzrev force-pushed the chore-add-drivedclasses-for-interface branch from f98e7b2 to 7184add Compare May 21, 2025 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant