Consider adding 'dotnet format --verify-no-changes' to the build.cmd

[JanKrivanek]

(originaly reported by @rokonec)

Issue Description

In case we want to keep IDE0055

The formating analyzer warning can be pretty generic and hardly actionable - e.g.:

C:\src\msbuild\src\Build.UnitTests\BackEnd\MSBuild_Tests.cs(839,35): error IDE0055: Fix formatting [C:\src\msbuild\src\
Build.UnitTests\Microsoft.Build.Engine.UnitTests.csproj::TargetFramework=net472]

Wherease dotnet format gives clue quickly:

>dotnet format whitespace MSBuild.sln --verify-no-changes
C:\src\msbuild\src\Build.UnitTests\BackEnd\MSBuild_Tests.cs(839,35): error WHITESPACE: Fix whitespace formatting. Delete 1 characters. [C:\src\msbuild\src\Build.UnitTests\Microsoft.Build.Engine.UnitTests.csproj]

We can have extra step in build.cmd running the dotnet format whitespace MSBuild.sln --verify-no-changes failing the dev build for nonzero exit code. Plus it should suggest to simply run dotnet format whitespace MSBuild.sln to resolve the warnings without the need for manual complicated investigation of the problem.

Steps to Reproduce

Insert double space anywhere into the code and run build.cmd

Priority

Nice-to-have, only relates to our infra (no customer impact)
Though - it should be super quick to add

Alternative

add following to .editorconfig:

# Fix formatting
dotnet_diagnostic.IDE0055.severity = suggestion

[jrdodds]

There should be parity or equivalence maintained with build.sh.

[rainersigwald]

I would prefer to disable the rule. Failing builds for formatting changes sounds very unpleasant.

Plus it should suggest to simply run dotnet format whitespace MSBuild.sln to resolve the warnings without the need for manual complicated investigation of the problem.

A nice dream, but alas:

❯ dotnet format whitespace MSBuild.sln
Warnings were encountered while loading the workspace. Set the verbosity option to the 'diagnostic' level to log warnings.

[danmoseley]

Fwiw Roslyn uses the "@jaredpar system" ... formatting issues are only warnings on dev machines; in PR validation they are errors but the validation continues to ensure you get build and test results.

[rainersigwald]

That's my preferred approach for most things--but the low actionability of this one makes me prefer to just nix the rule.

[jaredpar]

I would prefer to disable the rule. Failing builds for formatting changes sounds very unpleasant.

Failing builds for formatting changes on developer machines is the fastest way to anger your developer team. No one wants to be in the middle of debugging a tricky issue, think they found the fix, build and get an error because they included a white space after a }.

Developers who truly want the "error every developer build on formatting differences" are in the minority. It is completely reasonable to have this as an option, but it shoudl be off by default.

[JanKrivanek]

To make the intention clear here - the dotnet format step is suggested to get more actionable message (plus offering a command for a quicker resolution) for a case where the analyzer error is already set to fail the build. That's our current situation.

By no means is this meant to cause build error that wouldn't otherwise occur.

Should we decide to demote or disable the formatting rules - then this work doesn't apply (or should only produce information output - that would anyways be lost in the other build log spew, so useless).

I'll update the description accordingly

[AR-May]

Team triage: we inclined to disable this non-specific error.