dotnet format 7.0.304 fails when resolving types from COM references

[molesmoke]

Describe the bug

dotnet format cannot resolve types from COM references

To Reproduce

A trivial test project is available here:

https://github.com/molesmoke/WinFormsAppDotNetFormat

Exceptions (if any)

error CS0246: The type or namespace name 'SpVoice' could not be found (are you missing a using directive or an assembly reference?)

Further technical details

• Include the output of dotnet --info

.NET SDK:
Version: 7.0.304
Commit: 7e794e2

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22621
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\7.0.304\

Host:
Version: 7.0.7
Architecture: x64
Commit: 5b20af47d9

.NET SDKs installed:
6.0.401 [C:\Program Files\dotnet\sdk]
6.0.402 [C:\Program Files\dotnet\sdk]
6.0.410 [C:\Program Files\dotnet\sdk]
7.0.304 [C:\Program Files\dotnet\sdk]
7.0.400-preview.23274.1 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
Not set

global.json file:
C:\Users\jerem\source\repos\tests\WinFormsAppDotNetFormat\global.json

Learn more:
https://aka.ms/dotnet/info

Download .NET:
https://aka.ms/dotnet/download

• The IDE (VS / VS Code/ VS4Mac) you're running on, and its version

Build Tools & VS Pro 17.6.3

[jeremy-visionaid]

Looks like 7.0.107 works OK with global.json:

{
  "sdk": {
    "version": "7.0.107"
  }
}

[marcpopMSFT]

FYI @arkalyanms @sharwell @jaredpar

[sharwell]

I'm not sure how/why 7.0.107 worked for this. COMReference isn't supported for the dotnet tool, which includes dotnet format.
dotnet/msbuild#3986

[jeremy-visionaid]

@sharwell FWIW, this also applied to types from C++/CLI and assemblies referenced from the filesystem. Only types in the dotnet SDK and NuGets are resolved succesfully. COM references were just easiest to create a trivial repro for. I imagine a lot of projects would be unable to leverage dotnet format if it threw an error for that.

[sharwell]

C++/CLI is also not supported by dotnet, but references to managed assemblies in a <Reference> element should work. Can you provide sample code for this which is currently failing?

[jeremy-visionaid]

Looks like the regular <Reference> actually is working OK. I was guarding some PropertyGroups with explicit conditions per platform for platform specific assemblies. Setting an unguarded default (e.g. assume x64) allows dotnet format to work for those cases. So, maybe that was a local problem that didn't manifest as a failure prior to this change in behavior.

I appreciate it sounds like this situation was perhaps not intended to work, but it would be great if the old behavior could be restored. Or for EnforceCodeStyleInBuild to cause a build failure when invoked through msbuild on the command line (I'd have to go back and check, but IIRC the reason I'm even calling dotnet format in the first place is because that path doesn't work as I'd expect).

[sharwell]

but it would be great if the old behavior could be restored

I have no idea how/what changed here. I'm not aware of any change in behavior with respect to these.

[jeremy-visionaid]

Still broken in 7.0.306, but OK in 7.0.109

[sharwell]

@jeremy-visionaid Absent someone investigating and identifying the specific reason why the behavior changed along with an explanation of how to restore and preserve the older behavior, I would not expect any further changes to occur in this space. The new behavior is certainly aligned with our understanding of the expected behavior.

[jeremy-visionaid]

@sharwell Thanks for setting the expectations. At least there is a reasonable workaround by simply excluding the troublesome error code:

dotnet format --no-restore --verify-no-changes --exclude-diagnostics CS0246

I'm happy enough for this to be closed given it does, unfortunately, seem to be the expected behaviour.

Really I think the bigger issue here is that EnforceCodeStyleInBuild doesn't do what it says on the tin:
dotnet/roslyn#49439

i.e. it should be possible to have msbuild fail and not have to verify the formatting separately.

[sharwell]

i.e. it should be possible to have msbuild fail and not have to verify the formatting separately.

@jeremy-visionaid I believe this is possible today. All of the projects I work on regularly use the build (and not dotnet-format) for code style enforcement. There is no single setting that covers all diagnostics, so enabling specific diagnostics in the build may require different settings depending on which diagnostic is involved. Is there a specific diagnostic you expected to see in the build but only saw when running dotnet-format?

[jeremy-visionaid]

@sharwell I would have hoped so, but as dotnet/roslyn#49439 says:

TreatWarningsAsErrors does not affect EnforceCodeStyleInBuild

Having a play around I think I was able to get more like the behaviour I expected there by adding:
<CodeAnalysisTreatWarningsAsErrors>$(TreatWarningsAsErrors)</CodeAnalysisTreatWarningsAsErrors>
I don't think I'd be alone in thinking that CodeAnalysisTreatWarningsAsErrors should default to TreatWarningsAsErrors out of the box.

This looks quite useful, but was pretty hard to find:
https://github.com/dotnet/roslyn-analyzers/blob/f356758767f3cb3b30a151788b834955bd03ecd7/src/Tools/GenerateDocumentationAndConfigFiles/README.md

I'll do some more tinkering and see if I can achieve my desired outcome with msbuild. Thanks for your nudge to take another look at it.

[jeremy-visionaid]

@sharwell I think I'm getting closer to something viable, but there's some dodgy behaviour with msbuild -warnaserror. Following the documentation here: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview ...

If I enable IDE055 (Formatting rule) in .editorconfig:

[*.{cs,vb}]
dotnet_diagnostic.IDE0055.severity = warning

Then run

msbuild -warnaserror

The build correctly fails (returning 1 to the shell) with error IDE0055: Fix formatting

Running msbuild -warnaserror again, the build succeeds, so looks like the incremental build behaviour is broken :(
Though it might still be usable for CI which would obviously be doing clean builds.

[jeremy-visionaid]

@sharwell I've spent a bit of time today trying to tighten up the CI and transition to enforcing style as part of the build, and while I've managed to get it a bit stricter I've still not been able to get EnforceCodeStyleInBuild to function as a suitable replacement for dotnet format. One example is import ordering e.g. error IMPORTS: Fix imports ordering. Maybe there's some directives I'm missing to get equivalent behaviour, or perhaps not?

[rainersigwald]

The build correctly fails (returning 1 to the shell) with error IDE0055: Fix formatting

Running msbuild -warnaserror again, the build succeeds, so looks like the incremental build behaviour is broken :( Though it might still be usable for CI which would obviously be doing clean builds.

This is a weakness of MSBuild's warning model, tracked as dotnet/msbuild#3046.

[sharwell]

@sharwell I would have hoped so, but as dotnet/roslyn#49439 says:

TreatWarningsAsErrors does not affect EnforceCodeStyleInBuild

I don't believe this is correct. TreatWarningsAsErrors is heavily relied on for exactly this purpose.

Then run

msbuild -warnaserror

I wouldn't expect -warnaserror to work here prior to dotnet/msbuild#3046. Using /p:TreatWarningsAsErrors=true instead should work.

[sharwell]

I don't think I'd be alone in thinking that CodeAnalysisTreatWarningsAsErrors should default to TreatWarningsAsErrors out of the box.

That should already be the default value.

I've spent a bit of time today trying to tighten up the CI and transition to enforcing style as part of the build, and while I've managed to get it a bit stricter I've still not been able to get EnforceCodeStyleInBuild to function as a suitable replacement for dotnet format.

It really sounds like you're doing a bunch of unnecessary work.

1. Enable diagnostics you want to enforce by setting their severity in .globalconfig to warning (not error)
2. If (1) includes any diagnostics with the form IDE####, set <EnableCodeStyleInBuild>true</EnableCodeStyleInBuild>
3. If you previously built the project in the current directory without setting code style warnings to errors, clean the directory before proceeding to build in the next step (unnecessary for CI builds since they always start clean)
4. Pass /p:TreatWarningsAsErrors=true when building if you want to fail on code style warnings

Make sure you are not setting <CodeAnalysisTreatWarningsAsErrors> anywhere, since it would interfere with default behavior.

error IMPORTS: Fix imports ordering

This isn't supported by an IDE rule today, but is available through third-party analyzer packages. One example is SA1200, SA1208, SA1209, SA1210, SA1211, SA1216, and SA1217 from the Ordering Rules in StyleCop Analyzers.

[jeremy-visionaid]

@sharwell Thanks for sticking with me on this, I'm grateful for the advice despite my frustration. I know we're way off topic from the original issue now...

Maybe I'm missing something, but these statements seem contradictory?

TreatWarningsAsErrors does not affect EnforceCodeStyleInBuild

I don't believe this is correct. TreatWarningsAsErrors is heavily relied on for exactly this purpose.
and
If (1) includes any diagnostics with the form IDE####, set true

If EnforceCodeStyleInBuild is supposed to default to TreatWarningsAsErrors then it seems I'm not the only one finding that it doesn't... I don't know why it matters for my project, maybe I can repro in something smaller if this is a bug.

It really sounds like you're doing a bunch of unnecessary work.

I agree, it really feels like this is being more work than it should be. I expect there's some subtly in the way the different warning properties might be set that affect the build (e.g. -warnaserror vs TreatWarningsAsErrors, or AnalysisLevel vs editorconfig/globalconfig settings). Perhaps some unexpected/undocumented behaviour that's bringing the whole thing down. Nonetheless, I'm really struggling to get an acceptable workflow bringing the analyzers an established project that's been around since pre-roslyn. To summarize the problems so far:

1. msbuild + analyzer incremental build behaviour is broken (Warnings are not reported for repeated builds msbuild#3046)
2. The feature set of dotnet format and the default anaylzers don't match
3. More rules are being enforced by CI than in the IDE even though AFAIK the config appears to be the same
4. There are issues with various analyzers that are preventing me from turning on certain categories/levels. For example, I can't seem to silence CA5393 for generated files

If I set:
dotnet_diagnostic.CA5393.severity = silent
I still get...
Microsoft.Interop.LibraryImportGenerator\Microsoft.Interop.LibraryImportGenerator\LibraryImports.g.cs(84,51): error CA5393: Use of unsafe DllImportSearchPath value AssemblyDirectory

Pass /p:TreatWarningsAsErrors=true when building if you want to fail on code style warnings

I don't mean to be a stickler/pedant, but that doesn't seem to match the documentation... Without getting in to the guts of it, I'd prob just expect the behaviour to match -warnaserror (https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview)

Make sure you are not setting anywhere, since it would interfere with default behavior.

As above, I need to set CodeAnalysisTreatWarningsAsErrors otherwise the warnings are not errors... It's the only occurence of CodeAnalysisTreatWarningsAsErrors in the project/repo (In Directory.csproj.props) To my knowledge I haven't made any global/system wide-config changes anywhere and there don't appear to be any
<CodeAnalysisTreatWarningsAsErrors>$(TreatWarningsAsErrors)</CodeAnalysisTreatWarningsAsErrors>

I am already running some additional third-party analyzers too, but IDE builds only. Perhaps I can add those to CI if I can fix the other problems.

[jeremy-visionaid]

@sharwell Ha, just as I typed all that, I realize the crux of the problem... I'd tied TreatWarningsAsErrors to ContinuousIntegrationBuild. This gives different behaviour when you're building in the IDE vs msbuild command line and affects CodeAnalysisTreatWarningsAsErrors.

When TreatWarningsAsErrors isn't overriden, there's no need for me to set CodeAnalysisTreatWarningsAsErrors explicitly, and that makes sense.

However, effectively setting TreatWarningsAsErrors to false actually ends up disabling a bunch of analysis entirely in the IDE. That seems like a bug to me (since I still want the analyzer warnings in the IDE, I just don't want them to be errors).

I still can't seem to turn off CA5393, and that's a separate issue anyway, but I can at least work around it with
<WarningsNotAsErrors>CA5393</WarningsNotAsErrors>

[sharwell]

For example, I can't seem to silence CA5393 for generated files

CA5393 is a security rule. It intentionally includes all generated code:
https://github.com/dotnet/roslyn-analyzers/blob/20690b42777d89aade8323b103f61893091c1dca/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/UseDefaultDllImportSearchPathsAttribute.cs#L58-L59

I'd tied TreatWarningsAsErrors to ContinuousIntegrationBuild

This is a fairly common practice, e.g.:

<PropertyGroup>
  <TreatWarningsAsErrors Condition="'$(TreatWarningsAsErrors)' == '' AND '$(ContinuousIntegrationBuild)' == 'true'">true</TreatWarningsAsErrors>
</PropertyGroup>

I would never recommend setting TreatWarningsAsErrors to default to true for IDE development or command line builds. It slows development down too much.

[jeremy-visionaid]

CA5393 is a security rule. It intentionally includes all generated code:

Right, I don't have an issue with it running on generated code, I have an issue with it not respecting the configuration where I want to disable that particular warning entirely. I could change the code to use DllImport as an alternative, but what's the reason for the warning triggering on the generated code for LibraryImport but not for an equivalent DllImport with the following in editorconfig?

dotnet_diagnostic.CA5393.severity = none

I would never recommend setting TreatWarningsAsErrors to default to true for IDE development or command line builds. It slows development down too much.

I wanted warnings to be OK for local dev but not for CI, but some mysterious combination of factors was causing some breakage in one situation or another. My props file has something quite similar to your suggestion. Just as an example though (I'm not actually using this), I find the following config causes there to be no code analysis warnings whatsoever:

msbuild -property:ContinuousIntegrationBuild=true

  <PropertyGroup>
    <AnalysisLevel>latest-All</AnalysisLevel>
    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
    <ResolveComReferenceSilent>true</ResolveComReferenceSilent>
    <WarningsNotAsErrors>CA5393,CS8002,SYSLIB0011,SYSLIB0014</WarningsNotAsErrors>
    <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
  </PropertyGroup>

  <PropertyGroup Condition=" '$(ContinuousIntegrationBuild)' == 'true' ">
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <CodeAnalysisTreatWarningsAsErrors>false</CodeAnalysisTreatWarningsAsErrors>
  </PropertyGroup>

But just removing <CodeAnalysisTreatWarningsAsErrors>false</CodeAnalysisTreatWarningsAsErrors> prints the errors and fails the build as expected. Maybe there's some good reason for the behaviour that I'm missing, but that feels quite fragile/sensitive. Now that I have something that's working it should be good to keep it that way. I'm not sure if it is sufficient for a smaller test project, but if I get the chance I'll see if I can post a project with a minimal repro for it.

Thanks once more for your help :)

[sharwell]

with the following in editorconfig?

.editorconfig is a file path-based configuration mechanism. It only applies to paths on disk that match the filters. .globalconfig is the true replacement for .ruleset, and applies to the whole project including the output from source generators.

In general, you should not have any dotnet_diagnostic.xxxx.severity lines in an .editorconfig file. The .editorconfig file should specify style preferences, and the rule severity configuration should be placed in .globalconfig.

[jeremy-visionaid]

@sharwell That's great, thanks. I find that moving the analyzer config to .globalconfig does silence the warnings in the generated files. Though I have some counterpoints again to highlight why I still think the current behaviour is confusing...

.editorconfig is a file path-based configuration mechanism

The analyzer reports the generated file is at a deeper path than the .editorconfig:

i.e. given an editorconfig at:
C:\src\project\.editorconfig

I get:

C:\src\project\Library\Microsoft.Interop.LibraryImportGenerator\Microsoft.Interop.LibraryImportGenerator\LibraryImports.g.cs(7,44,7,66): warning CA5393: Use of unsafe DllImportSearchPath value AssemblyDirectory

Perhaps the generated file isn't really at that location, but given the analyzer treats/reports it like it is, then I expect that the editorconfig should actually apply.

In general, you should not have any dotnet_diagnostic.xxxx.severity lines in an .editorconfig file

But that's where VS Pro puts them. I know that's likely beyond scope here since it's a UI thing, but I can't see an option to create them in .globalconfig, or a cue that a .globalconfig should be created. There's no template for a .globalconfig, and trying to create one with an .editorconfig template throws an error for me. Even once a .globalconfig file exists, VS continues to prompt to add the analyzer suppressions to .editorconfig. I wonder if there's a roadmap ticket to improve that somewhere? From a user perspective, I want to be able suppress warnings with a few mouse clicks not be copying/pasting things between different config files. Something like a hint/warning label in the editorconfig UI Analyzers tab might be nice to guide users to putting them in a more appropriate place. I'll have a search on the developer community later for any existing feedback there and/or add a new item as required.

[sharwell]

Perhaps the generated file isn't really at that location, but given the analyzer treats/reports it like it is, then I expect that the editorconfig should actually apply.

The tracking issue for this is dotnet/roslyn#47384.

But that's where VS Pro puts them.

This is also covered in the issue above, e.g. comment dotnet/roslyn#47384 (comment).

[jeremy-visionaid]

I've been busy working on other issues, so I still haven't had a chance to come up with a standalone repro for the flakiness in the config as above, but this one was another issue responsible for the difficulties in making the CI fail as expected:
dotnet/roslyn#41640

But then IDE0005 triggers as a false positive with dotnet format, so have to add an exclusion for that there too. I'll try and include these issues in a repro repo at some point.

[jeremy-visionaid]

@sharwell The situation seems improved in dotnet 8. Please go ahead and close the issue. Thanks again for your help.