dotnet / docfx

Static site generator for .NET API documentation.
https://dotnet.github.io/docfx/
MIT License
4.06k stars 863 forks source link

Cross-reference silently ignored #5670

Open djee-ms opened 4 years ago

djee-ms commented 4 years ago

Operation System: Windows

DocFX Version Used: 2.51.0 (NuGet)

Template used: default

Steps to Reproduce:

  1. Unzip the attached minimal repro : docfx-bug.zip
  2. Run docfx with docfx.exe --intermediateFolder .\obj -o .\build --serve --debug
  3. Observe the warning Invalid cref value "!:A.B.BaseClass.Field" found in triple-slash-comments for OtherClass, ignored
  4. Open http://localhost:8080/api/A.B.C.html and observe the "See: ." missing the reference to A.B.BaseClass.Field.

So far so good, even though I am not sure why cref doesn't work here. Anyway, on to the actual bug:

  1. Change other.cs and replace cref with xref
  2. Run docfx again (same args)
  3. Observe no warning
  4. Open http://localhost:8080/api/A.B.C.html and observe again the "See: ." missing the reference to A.B.BaseClass.Field.

Expected Behavior:

Actual Behavior:

Cross-reference XML tag is copied as-is to the output, and not rendered by the browser which cannot understand e.g. <see> tags.

djee-ms commented 4 years ago

Ping -- this is breaking a large amount of the documentation of our project since we have some Unity library in a sub-namespace of a lower-level C# library, with the former doing frequent references to types in the latter. We are planning a release of this project soon, and this is a blocker with documentation broken that way. Can someone please triage this issue and validate whether we are doing something stupid or there is an actual bug?

superyyrrzz commented 4 years ago

This seems to be mixture use of 2 syntaxes:

  1. XML comments <see>: <see cref="xxx"/>. We can replace cref with href to reference an URL here, but xref is not supported.

  2. DFM (DocFX flavored Markdown) xref: <xref:xxx> is one of the supported form.

So DocFX recognizes <see xref="xxx"/> neither a valid <see> nor a valid xref, then silently passes it to output. Useing cref instead should fix this.

But I agree DocFX can improve validation: if an unknown attribute other than cref/href is found within <see>, warn it.

djee-ms commented 4 years ago

Hi @superyyrrzz, thanks for the quick reply! I was under the impression <see> was supporting xref too, I guess I was wrong. I will try to replace the references with the DFM <xref:> syntax, that should work.

One question though, why is <see cref="A.B.BaseClass.MyType"/> not working inside the A.B.C namespace? Is it because A.B.BaseClass is parsed in the context of a .csproj but A.B.C is parsed in the context of a standalone file not part of this .csproj? Shouldn't those be merged under a same "cref umbrella" when parsing them all as a single docfx invocation? I would think xref is only for types from external third-party assemblies, like the System types etc. And even for that, looking at the docs you linked for XML comments <see> their example uses <see cref="System.Console.WriteLine(System.String)"/> which is clearly not meant to be part of the same assembly than the TestClass of the example.

superyyrrzz commented 4 years ago

I think standalone file can be a cause. Roslyn may not be able to find the referenced type in cref when parsing it. Resolving from csproj seems to work.

However, xref is resolved in a later phase. All UIDs within one docfx.json are taken under the same scope, so it can resolve properly.

djee-ms commented 4 years ago

Yes I looked around in SO & co. and it seems people have the same issue and are working around with either of:

I prefer the former but we have only 2-3 assemblies; other projects like microsoft/MixedRealityToolkit-Unity have 20+, and manually maintaining fake .csproj doesn't seem like a good option for them.

KalleOlaviNiemitalo commented 4 years ago

<xref:id_of_another_file> in Markdown is a URI autolink with xref as the URI scheme. Does <see href="xref:id_of_another_file">content override</see> likewise work in XML documentation comments?

manually maintaining fake .csproj doesn't seem like a good option

If you use MSBuild wildcards in the fake .csproj, you won't have to edit it when you add or remove source files.

<ItemGroup>
  <Compile Include="**\*.cs" />
</ItemGroup>

Perhaps this will need to exclude AssemblyInfo.cs, to avoid compiler errors when AssemblyInfo.cs files intended for different assemblies add the same attributes.

djee-ms commented 4 years ago

Good catch, might give it a try, thanks @KalleOlaviNiemitalo!

djee-ms commented 4 years ago

I actually made a few discoveries:

djee-ms commented 4 years ago

Also note : I checked, and currently microsoft/MixedRealityToolkit-Unity is using the syntax [some link](xref:System.String) in their XML comments inside .cs files (I am not interested about markdown files, everything works as expected with them). They seem to have just a few links like that.

KalleOlaviNiemitalo commented 4 years ago

Re docfx-bug-invalid-cref.zip, if I replace <TargetFramework>netstandard2.0</TargetFramework> with <TargetFramework>net4.8</TargetFramework>, then DocFX 2.45.1.0 with Visual Studio 15.9.21 and .NET Core SDK 2.1.513 can extract the metadata just fine.

Does DocFX support getting a MSBuild diagnostic log from the metadata extraction, to see which targets it calls and which items it defines?

Perhaps you can work around the bug by running the MSBuild and C# compiler first, and then only giving the DLL and XML files to DocFX.