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

This updates docfx to have basic support C# 9 support. #6805

Merged
merged 14 commits into from
Jun 1, 2021

Conversation

tannergooding
Copy link
Member

@tannergooding tannergooding commented Nov 24, 2020

Microsoft Reviewers: Open in CodeFlow

@tannergooding
Copy link
Member Author

There are also other features of C# 9, like records, that weren't validated.

@tannergooding
Copy link
Member Author

Will look into the failure. Seems at least one of the tests is failing with:

Failed Microsoft.DocAsCode.Metadata.ManagedReference.Tests.GenerateMetadataFromAssemblyTest.TestGenerateMetadataFromAssemblyWithReferences [136 ms]
  Error Message:
   Microsoft.DocAsCode.Exceptions.DocfxException : Unable to generate spec reference for T:System.ValueTuple{System.String,System.String}
---- System.NullReferenceException : Object reference not set to an instance of an object.

@superyyrrzz
Copy link
Contributor

superyyrrzz commented Nov 26, 2020

@tannergooding Thank you for adding the new features!

Could you help add some unit tests in this PR for the new feature supported? I think you are supporting native sized integers and function pointers here, but some tests will help me understand it better.

I am not sure of the answer to your questions, as these codes are rarely updated. But before they are answered, I think it is fine to make the current change working for happy path 😄

@tannergooding
Copy link
Member Author

Could you help add some unit tests in this PR for the new feature supported?

Can do. I should be able to get to this in the next few days now that I'm back off vacation.

@tannergooding
Copy link
Member Author

Rebased and rerunning tests before adding the new tests.

@buehler
Copy link

buehler commented Mar 25, 2021

This would be very nice! I really like to document stuff with docfx, but newer code uses records which are not supported :-(

@yufeih
Copy link
Contributor

yufeih commented Apr 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@superyyrrzz superyyrrzz requested a review from v-pegao April 9, 2021 02:42
@roji
Copy link
Member

roji commented May 29, 2021

Any news on this PR? Lack of support for C# 8/9 features is really a blocker for using docfx with modern projects...

@tannergooding
Copy link
Member Author

I've updated this to include tests and to cover:

  • function pointers
  • nint/nuint
  • readonly members
  • ref and ref readonly returns

This does not cover:

  • records
  • init only setters

There may be other syntax or languages features that also need improvements.

@tannergooding
Copy link
Member Author

@superyyrrzz, sorry for the delay in getting this up, but I believe there is now sufficient tests covering the added functionality. Please let me know if anything further is needed.

@tannergooding
Copy link
Member Author

Also updated to cover ref struct and readonly struct which are somewhat common in the BCL and were missing and an explicit test covering value tuple names

@tannergooding
Copy link
Member Author

Copy link
Contributor

@yufeih yufeih left a comment

Choose a reason for hiding this comment

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

@tannergooding
Thanks for you contribution! This is HOT 🔥 🔥 🔥 .
The overall logic LGTM and we'll try to merge and ship it soon.

@tannergooding
Copy link
Member Author

The overall logic LGTM and we'll try to merge and ship it soon.

Just wondering, will this automatically flow to v3 and to whatever we power docs.microsoft.com with (if its not one of the mainline branches)?

@yufeih
Copy link
Contributor

yufeih commented Jun 1, 2021

Just wondering, will this automatically flow to v3 and to whatever we power docs.microsoft.com with (if its not one of the mainline branches)?

Unfortunately no. docs.microsoft.com uses monodoc for .NET API which v3 will likely also use to unify on a single pipeline. monodoc internally uses Mono.Cecil to analyze dlls instead of roslyn.

@tannergooding
Copy link
Member Author

tannergooding commented Jun 1, 2021

monodoc internally uses Mono.Cecil to analyze dlls instead of roslyn.

That seems unfortunate, especially given how many language features can involve special attributes and other annotations that I don't believe Cecil directly supports (they'll be exposed, but the "understanding" of whether IntPtr is an IntPtr or a nint is no longer as simple as checking symbol.IsNativeIntegerType).

In either case, it would be good to get most of these features working there as well, to ensure that things like System.Span or System.Numerics.Vector4 correctly display the relevant semantic information in the docs. Maybe I'll see how hard it is to add the same support to mono-doc...

@yufeih
Copy link
Contributor

yufeih commented Jun 1, 2021

The input to generate .NET APIs on docs is a bunch of DLLs rather than the source code. We've tried to stay away from the source code to decouple from the complex build system of each component, with the downside you mentioned.

@joelmartinez What's our status on supporting these latest C# language features on docs? @tannergooding might be able to help with his expertise in this area.

@yufeih yufeih merged commit 0bd7c8e into dotnet:dev Jun 1, 2021
@joelmartinez
Copy link

@yufeih we've recently been working to improve mdoc's support for nullable. I would recommend we work with our PM, @mimisasouvanh, on any feature that needs improvement in mdoc's language formatters.

@tannergooding you're right, cecil only takes us so far, and there's additional logic we often need to implement in mdoc as new language features come out, especially the ones that depend on attributions. Please feel free to ping myself or @TianqiZhang if you have any questions or thoughts about contributing to mdoc :)

@sirgru
Copy link

sirgru commented Sep 8, 2022

Are there any changes considering C# 9 and 10 support? In particular, newer code uses records which are not supported in V2. V3 has been coming for a long time, but I couldn't even initialize it properly to check if it supports newer features.

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.

7 participants