Incorrect error codes reported

[RussKie]

Version Used:

I'm updating Windows Forms SDK/toolset from P6 to RC1, and our builds fail with errors which we have already suppressed. E.g.:

It appears instead of CS0618 we must now be suppressing SYSLIB0011 but I had to spend time to figure it out on my own.
Likewise we have other failures, e.g.

...and only VS is telling me that I now need to suppress a different one:

Few builds:

• https://dev.azure.com/dnceng/public/_build/results?buildId=817132&view=results
• https://dev.azure.com/dnceng/public/_build/results?buildId=817156&view=results

Steps to Reproduce:

1. Clone this commit: dotnet/winforms@2427ee0
2. Run .\build.cmd
3. Observe multiple failures

Expected Behavior:

Errors must either correctly report codes or use the existing suppression rules.

[RikkiGibson]

This is an intended result of the "better obsoletion" feature (#42119). Per the proposal:

When the user suppresses the existing generic diagnostic ID for obsoletion (e.g. CS0618 in C#), the compiler should not suppress any obsoletions with an explicitly set diagnostic ID (unless that explicit ID happens to match the generic diagnostic, but that would be bad practice from the API author). Instead, the developer has to add a suppression for the specified diagnostic ID. As such, adding a diagnostic is the same as changing a diagnostic ID and thus a source breaking change.

Tagging @terrajobst

[Youssef1313]

@RikkiGibson I think the complaint here is because the explicit id is shown in Visual Studio but not Visual Studio Code.

[RussKie]

@RikkiGibson the problem is that build fails without clear indication of what is wrong - see the failed builds I've linked. VS doesn't fail the build, it only shows warnings (this is another big issue), but those warnings are too totally misleading. And only by hovering with a mouse I found that in fact I need to suppress different warnings.

This is a very-very bad user experience. I've lost 3 hours yesterday because of this.

[jaredpar]

@RussKie

the problem is that build fails without clear indication of what is wrong

The screen shots you indicate of the compilation process have clear error messages associated with them.

Is your complaint about how the IDE displays them? If so then can you show us what the error list looks like?

VS doesn't fail the build, it only shows warnings (this is another big issue

Not easy to tell from your issue description what the VS behavior is. The screen shots you provided demonstrate the command line compiler issuing an error and a quick info screen shot.

[RussKie]

The screen shots you indicate of the compilation process have clear error messages associated with them.

No, it tells me that it failed due to CS0618, which are suppressed in the code. In fact it failed due to SYSLIB0011, which I needed to suppress.

[RikkiGibson]

Sorry, I misunderstood the issue to mean "I updated my .NET SDK version and now my CS0618 suppressions no longer work".

VS doesn't fail the build, it only shows warnings (this is another big issue)

Is this a bug? e.g. you have a property in your project like <TreatWarningsAsErrors>true</TreatWarningsAsErrors> or <WarningsAsErrors>SYSLIB0011</WarningsAsErrors> but the build reported a warning instead of an error?

only by hovering with a mouse I found that in fact I need to suppress different warnings.

My best guess for what is wrong is the compiler is pinned to an old version in command line builds (i.e. pre Roslyn 3.6/VS 16.6) but not in VS design time builds. I believe common methods of pinning the compiler version, such as adding a MicrosoftNetCompilersToolsetVersion in an arcade project, are not respected in the VS editor, and only affect command line builds.

I tried to create a minimal reproducer for the problem:

using System.IO;
using System.Runtime.Serialization.Formatters.Binary;

public class C
{
    static void M1(
        BinaryFormatter binaryFormatter,
        Stream stream)
    {
#pragma warning disable CS0618
        binaryFormatter.Serialize(stream, new object());
#pragma warning restore CS0618
    }
}

Using dotnet sdk:

> dotnet --version
5.0.100-rc.1.20452.10

> dotnet build /t:rebuild
Microsoft (R) Build Engine version 16.8.0-preview-20451-02+51a1071f8 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
C:\Users\rikki\src\net5scratch\Program.cs(11,9): warning SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\Users\rikki\src\net5scratch\net5scratch.csproj]
  net5scratch -> C:\Users\rikki\src\net5scratch\bin\Debug\net5.0\net5scratch.dll

Build succeeded.

C:\Users\rikki\src\net5scratch\Program.cs(11,9): warning SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.' [C:\Users\rikki\src\net5scratch\net5scratch.csproj]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.77

Building in VS 16.8-preview3:

1>------ Rebuild All started: Project: net5scratch, Configuration: Debug Any CPU ------
1>You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
1>C:\Users\rikki\src\net5scratch\Program.cs(11,9,11,56): warning SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.'
1>net5scratch -> C:\Users\rikki\src\net5scratch\bin\Debug\net5.0\net5scratch.dll
1>Done building project "net5scratch.csproj".
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========

Also found equivalent behavior when hovering on the squiggle or checking the Error List in VS or VS Code--the text mentions SYSLIB0011, not CS0618.

[RussKie]

My best guess for what is wrong is the compiler is pinned to an old version in command line builds (i.e. pre Roslyn 3.6/VS 16.6)

Would you be able to point me where is this set?
Here's the branch that was failing: https://github.com/RussKie/winforms/tree/Roslyn-47736 (dotnet/winforms@2427ee0)

I'm not quite sure what's going on.
I just had a clean checkout of the branch and built it form the cli - it failed with CS0618

I also built it in VS 16.8.0 Preview 4.0 [30515.18.main] and it failed for some other unrelated reason (cmake).
This VS version how shows me SYSLIB0011 (instead of CS0618), but it doesn't fail the build:

So there are two issues here.

1. One is that the build is failing with incorrect IDs
2. VS is not failing (this is separate, we can track it separately)

What am I missing?

[RussKie]

I apologies if I come across rude, I don't mean it. This is just stressing me out.

[RikkiGibson]

Please try adding #error version to a source file and let us know the message you get in the VS error list and in the command line build-- if they are different, that will make us pretty confident that different compiler versions are in use.

I apologies if I come across rude, I don't mean it. This is just stressing me out.

Thanks, I appreciate you saying that.

[RussKie]

From VS I get this:

3> error CS1029: #error: 'version'
3> error CS8304: Compiler version: '3.8.0-4.20461.1 (d885b798)'. Language version: preview.

[RussKie]

From cli I get this:

 error CS1029: #error: 'version' [C:\Development\winforms.temp\src\System.Windows.Forms.Design\src\System.Windows.Forms.Design.csproj]
 error CS8304: Compiler version: '3.8.0-3.20452.3 (90e570d7)'. Language version: preview. 

[RikkiGibson]

Hmm. The versions are different but only by about a week: a Sept 11th build in VS and a Sept 2nd build in CLI. 90e570d...d885b79. I do not think changes that would affect handling custom obsolete diagnostics went in during that time.

My next guess is that the SDKs in use are different between the different build environments, and the Obsolete attributes on standard library types are different between the two SDKs. It seems like the Obsolete attribute changes went in preview8: dotnet/runtime#39269

[jaredpar]

I tried reproducing this on my machine using VS 30514.8 and I'm getting what I believe is the correct behavior:

1. The build has warnings but succeeds
2. The error list display only warnings and uses the SYSLIB00 diagnostic code
3. The compiler output is a warning and uses the SYSLIB00 diagnostic code

To reproduce I just cloned the winforms repo, opened in VS and hit build.

This matches my intuition about how the feature is expected to work. Essentially the diagnostic produced by the compiler now prefers the one in the attribute over our generic diagnostic code. It appears we are consistently doing this in the repo and it's flowing through the IDE correctly. Maybe I'm missing a step in the repro but this appears correct.

I agree this diagnostic code change can be a bit jarring but it was part of the explicit design: @GrabYourPitchforks @terrajobst can comment on their desire for having this experience and what the motivations were.

@mavasani just for visibility that there may be some experience inconsistency here in the various VS builds.

Compiler Output

Error List

[RussKie]

One thing I totally forgot to mention (sorry), to start Windows Forms solution in VS you need to run:

restore.cmd
start-vs.cmd

This will restore the necessary dependencies and set env vars for VS.

[RikkiGibson]

@jmarolf and I are taking a look at this now

[RussKie]

Thank you heaps.

I had an offline chat with @jaredpar, few things may be worth adding here:

PS C:\Development\winforms> dotnet --list-sdks
3.1.402 [C:\Program Files\dotnet\sdk]
5.0.100-preview.7.20366.15 [C:\Program Files\dotnet\sdk]
5.0.100-rc.1.20452.10 [C:\Program Files\dotnet\sdk]
5.0.100-rc.2.20465.19 [C:\Program Files\dotnet\sdk]
6.0.100-alpha.1.20421.6 [C:\Program Files\dotnet\sdk]

But because we need to point VS to the SDK/tooling we want we prepend the local dotnet path to the process PATH (via start-vs.cmd). So VS should use of the following:

PS C:\Development\winforms\.dotnet\sdk> dir

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----         7/09/2020   8:46 PM                5.0.100-preview.6.20310.4
d-----        16/09/2020   3:10 PM                5.0.100-rc.1.20452.10
d-----        10/09/2020   3:35 PM                5.0.100-rc.1.20454.5
d-----        17/09/2020   9:18 AM                5.0.100-rc.2.20465.34

[GrabYourPitchforks]

I agree with the premise that the console output should clearly show the SYSLIB#### diagnostic code if it's going to require you to #pragma warning disable that specific warning code. I don't know why the console output would display CS0618 if it's expecting you to suppress SYSLIB0011, but looks like the necessary people are on this thread investigating it.

Related issue #47310 might also help here. In the case of SYSLIB0011, the error message explicitly includes the URL https://aka.ms/binaryformatter, which provides sample code showing how to suppress the warnings if needed. We're adding similar samples to the other warnings (see https://aka.ms/dotnet-warnings/SYSLIB0001), but these URLs currently don't show up on the command line because they're not part of the exception message itself. They're instead part of the UrlFormat property, which isn't used when generating the human-readable error message to the console.

[RikkiGibson]

I found that dotnet build and .\Build.cmd are producing different behaviors in the winforms repo. dotnet build produces the custom diagnostic ID that we want, while .\Build.cmd produces the built-in CS0618. We have taken a look at the binlogs and compared the arguments to csc for the two builds, as well as csc versions. The culprit is not yet obvious but I will continue to investigate later.

[RikkiGibson]

/+warnaserror seems to be the problem. The minimal repro project #47736 (comment) gives an unexpected error CS0618 when either <WarningsAsErrors>SYSLIB0011</WarningsAsErrors> is specified or <TreatWarningsAsErrors>true</TreatWarningsAsErrors> is specified.

Thanks to @jmarolf for helping me track this down. I suppose this bug didn't have the chance to negatively impact more people since corefx didn't add custom diagnostic IDs until recently.

[RussKie]

Do I need to raise a separate issue for VS not failing a build, or would this be related?

[RikkiGibson]

Well, it appears that .\Build.cmd somehow or another is passing /+warnaserror to the compiler while dotnet build and building in VS is not. First I would make sure whether there is an issue with your build configuration.

[jaredpar]

@RikkiGibson
Still not 100% following. Are you saying the compiler behavior changes when /+warnaserror is used?

[RikkiGibson]

With /-warnaserror, the compiler gives a diagnostic with the custom ID e.g. SYSLIB00XX like it should. With /+warnaserror, the compiler loses the custom ID and just gives CS0618.

[jaredpar]

Ah okay. That would definitely explain the results I saw when I tried to reproduce this. I was using it without warnings as errors hence everything looked just fine. Good catch!

[RikkiGibson]

@RussKie for a short-term workaround you might be able to change your pragmas to suppress the custom diagnostic ID as well as the old non-custom ID, e.g. #pragma warning disable CS0618,SYSLIB0011. Thanks for catching/reporting this.

edit: Actually the workaround I suggested is pointless 😉 disabling the explicit diagnostic ID works, even if the diagnostic ID is reported incorrectly.