search

dotnet / docfx

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

[Bug] System.Data warnings for .NET 8 WPF assemblies #9459

Closed rohahn closed 2 days ago

rohahn commented 11 months ago

Describe the bug Running docfx metadata on a .NET 8 WPF DLL generates a bunch of wrong warnings. Since we use --warningsAsErrors, this is a blocker for us. :(

The transitive dependency DirectWriteForwarder.dll references assemblies from .NET 8 and .NET Framework. UniversalAssemblyResolver cannot handle this.

To Reproduce

  1. Create a simple WPF assembly with 
    <TargetFramework>net8.0-windows</TargetFramework>
<UseWPF>true</UseWPF>
  2. Configure docfx.json to process assemblies (the problem does not occur when processing project files)
{
  "metadata": [
    {
      "src": [
        {
          "files": ["artifacts/assemblies/*.dll"],
          "src": "./"
        }
      ],
      "dest": "api"
    }
  ]
}
  1. Execute dotnet docfx metadata .\docfx.json

This results in the following warnings:

dotnet docfx metadata .\docfx.json
                                                                           Using .NET Core SDK 8.0.100
Loading assembly C:/Users/hahn/Projects/DocFxTest/artifacts/assemblies/DocFxWarningTest.dll
warning: Unable to resolve assembly reference System.Data.SqlClient, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.Data.Odbc, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Data.OleDb, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.IO.Ports, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Runtime.Serialization.Schema, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.ServiceModel.Syndication, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.ServiceProcess.ServiceController, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
Processing DocFxWarningTest
Creating output...

Build succeeded with warning.

    7 warning(s)
    0 error(s)

Expected behavior There should be no warnings.

Context (please complete the following information):

Additional context I ran docfx in the debugger and was able to see where the dependency is coming from:

PresentationCore -> DirectWriteForwarder -> System.Data -> System.Data.SqlClient

DirectWriteForwarder is referencing System.Data from .NET Framework (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\System.Data.dll).

image

UniversalAssemblyResolver.FindAssemblyFile in CompilationHelper is resolving C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.0\System.Data.dll instead, which is a reference assembly that only contains forwarded types.

image

Since these references with 0.0.0.0 version cannot be resolved, the warnings are generated.

yufeih commented 11 months ago

Same error from ILSpy:

image
rohahn commented 11 months ago

I also checked ILSpy and it seems the problem is not new with .NET 8.

image

ILSpy is using the wrong path.

We were previously using .NET 6 and Docfx 2.59.4.0 without the warnings. When upgrading to .NET 8, we also tried to upgrade to the latest Docfx.

So it seems recent changes in CompilationHelper must have led to the problem being uncovered.

bitbonk commented 11 months ago

We have the same problem since we upgraded docfx to .NET 8.

We generally want to build the docs warning-free to validate that we don't have any missing links or other errors in the docs, that's also why we turned on WarningsAsErrors. Since in docfx, there doesn't seem to be a way to ignore warnings individually, we currently can only ignore all warnings which defeats the purpose of validation.

rohahn commented 11 months ago

I also opened an issue for ILSpy.

Herrmel commented 7 months ago

I have the same problem with net6 assemblies but in my case I sometimes even get the following error that prevents my build:

error: (1,7): error CS0518: Predefined type 'System.Object' is not defined or imported
error: (3,29): error CS0518: Predefined type 'System.String' is not defined or imported
error: (3,19): error CS0518: Predefined type 'System.Void' is not defined or imported

Since ILSpy does not seem to do anything about it I tested some stuff and noticed ILSpy is able to resolve the previously unresolvable assembly after I opended it in ILSpy before I (re)load my assembly. Maybe it is possible to work around this issue in docfx aswell if we could add the framework-dlls via references in docfx.json and load them in the ILSpy-reflection beforehand? Does docfx currently use the references configuration in assembly reflection at all?

siegfriedpammer commented 3 days ago

ILSpy is using the wrong path.

No, it's not. The System.Data.dll located in the .NET 6.0 SDK directory has the correct version number (which is 4.0.0.0). Why should ILSpy fall back to a possibly outdated/incompatible version from classic .NET?

See also my comment in the ILSpy repository: https://github.com/icsharpcode/ILSpy/issues/3128#issuecomment-2423745209

siegfriedpammer commented 3 days ago

UniversalAssemblyResolver.FindAssemblyFile in CompilationHelper is resolving C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.0\System.Data.dll instead, which is a reference assembly that only contains forwarded types.

It is NOT a reference assembly. It just exists for backwards compatibility (like mscorlib.dll and netstandard.dll). Reference assemblies are assemblies that contain all externally visible type definitions, but no IL (just a dummy throw null; statement in the body).

Since ILSpy does not seem to do anything about it

@Herrmel I tested this and this stuff works in ILSpy. Since we didn't even know that docfx is using our assembly resolver and we don't know what their expectations are it's hard for us to provide a solution. I would be glad to provide support, if the maintainers of this project got in touch with us and shared some more info.

One thing I noticed:

warning: Unable to resolve assembly reference System.Data.SqlClient, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.Data.Odbc, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Data.OleDb, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.IO.Ports, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Runtime.Serialization.Schema, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.ServiceModel.Syndication, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.ServiceProcess.ServiceController, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a

These assemblies are not part of the default .NET 8.0 Windows Desktop framework. If you want to use them, you need to add references to nuget packages. For ILSpy it is impossible to detect whether a reference originated from a nuget package, because there is just a normal reference in the binary. However, there should be a .deps.json file next to the DLL, which is understood by UniversalAssemblyResolver (or rather DotNetCorePathFinder) if it is configured correctly - then it should be able to find references from nuget packages.

I can help you debug this, if somebody provides setup and reproduction steps with docfx. Thank you very much!

siegfriedpammer commented 3 days ago

I had a quick look at the code in CompilationHelper and I noticed, that in CreateCompilationFromAssembly you create a CSharpCompilation without any syntax trees but just references and even the "main assembly" added as a reference. The problem here is that from my experience with Roslyn (of course, someone from the C# compiler/Roslyn team would have to confirm this), it doesn't go well, if you add not only the directly used references, but also transitive references to a compilation, especially with the MetadataImportOptions.All option. Last time I tried this at our company, I got lots of strange compiler errors for some projects.

Also, can someone explain what the DotnetApiCatalog.Compile implementation is used for? What results are expected from UniversalAssemblyResolver? Thanks!

Also keep in mind that ILSpy and the decompiler engine, for which UniversalAssemblyResolver was created, are designed to work even if some references cannot be resolved. It will add extra casts in the decompiled code and may not be able to detect all patterns, but still do its best. So, if you are using UniversalAssemblyResolver, you should not expect it to deliver flawless results in all cases. Roslyn on the other hand expects you to provide all necessary information and produces an error if you fail to do so. I believe that ILSpy is not exactly the right tool for this job, but instead the dotnet/docfx people should talk to the dotnet/runtime people and ask them, to provide an assembly resolver that "does what the runtime does".

filzrev commented 2 days ago

I've confirmed reported issue can be reproduced with following steps.

  1. Install docfx as .NET Global Tools with following command

    dotnet tool install docfx -g

  2. Create work directory.
  3. Initialize docfx project with following command.

    docfx init --yes

  4. Edit docfx.json and replace metadata section with following content. 
    "metadata": [
  {
    "src": [
      {
        "src": "./",
        "files": [
          "dlls/*.dll"
        ]
      }
    ],
    "dest": "api"
  }
],
  5. Copy WPF application dll to dlls directory.
  6. Run docfx metadata command
yufeih commented 2 days ago

Despite disguising themselves as 4.0 version assemblies, these assemblies usually contain type forwarders and sometimes new types which are not available in older frameworks. The version number is actually wrong/misleading, but that's the mess we're living in for the sake of eternal backwards compatibility. The assembly resolving logic in ILSpy and the decompiler engine has a lot of special cases and hacks because of this mess.

True. This is the mess to live with and I'll exclude the warning from docfx side. Thank you @siegfriedpammer for looking into it.

Herrmel commented 1 day ago

@Herrmel I tested this and this stuff works in ILSpy. Since we didn't even know that docfx is using our assembly resolver and we don't know what their expectations are it's hard for us to provide a solution. I would be glad to provide support, if the maintainers of this project got in touch with us and shared some more info.

I actually had a different Problem wich is now fixed in ILSpy. In my case for whatever reason .net left behind empty Version folders. IlSpy tried to use these older and empty version folders to find the referenced assemblies and its problematic to use a compilation without the core library.