-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Warnings CS1701 and CS1702 should be removed #19640
Comments
Just found something funny: The core compiler targets actually try to disable it for anything other than ancient .NETFramework v1.0 or v1.1: roslyn/src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets Lines 40 to 45 in aa683ac
This doesn't check TFM so it started showing up in .NETStandard and .NETCoreApp 1.0 and 1.1! We should just remove this from the targets and delete the warnings. |
Suppress warnings in test projects caused by a bug in the Roslyn compiler when targeting .NET Core 1.0 (see dotnet/roslyn#19640)
Suppress warnings in test projects caused by a bug in the Roslyn compiler when targeting .NET Core 1.0 (see dotnet/roslyn#19640)
Suppress warnings in test projects caused by a bug in the Roslyn compiler when targeting .NET Core 1.0 (see dotnet/roslyn#19640)
These two warnings have been deprecated for years in project templates and by the SDK. dotnet/roslyn#19640 In .NET Framework 4.8 these warnings cause complaints about System.IO.Compression.FileSystem, Version=4.0.0.0 not matching with System.IO.Compression, Version=4.2.0.0. However, the default runtime policy of using the newer version is fine.
Still hanging around in VS 2022 17.2 |
I confirm this is still the case in Visual Studio 2022 17.4.2 |
It looks like if a project doesn't define So it looks like a typical simple project will have these suppressed automatically by the SDK. @jcouv Any concerns removing these from the compiler? |
@Youssef1313 I conferred with Jared and he'd be fine to remove them. |
The only issue is I'm not exactly sure if there is any particular process we need to follow when removing a warning from the compiler. Items that need to be considered are:
These may have simple answers, I haven't looked, but they need to be validated. Another option may be to just leave the diagnostics in place but set them to disabled by default. |
@jaredpar I think they will (or at least should) be ignored |
These warnings assume runtime behavior that only holds on full .NET framework: that you need to supply "policy" to bind a ref to a lower assembly to a higher version. This means that the warnings have to be disabled when targeting other .NET platforms
Furthermore, even when targeting .NET Framework, msbuild will also warn and so removing these warnings doesn't prevent our greater build ecosystem from diagnosing the issue where it actually applies. Plus, msbuild actually understands app.config binding redirects in making the call on whether to warn, and it can even generate a correct one for you (via AutoGenerateBindingRedirects=true), which it recommends in its warning.
These warnings have had to be disabled in project templates for a long time. Searching for "NoWarn" 1701 on GitHub yields 250K results: https://github.com/search?q=NoWarn+1701&type=Code&utf8=%E2%9C%93
dotnet/sdk disables them for you to avoid cluttering your csproj, but this opens up issues around the ordering of NoWarn between user and SDK. I'd like to get the SDK out of the business of disabling warnings. Over the past several years, the only times I see 1701 is when a build misconfiguration has undone a vital NoWarn, and nobody can decipher the message. e.g. dotnet/sdk#1205
The text was updated successfully, but these errors were encountered: