Skip to content
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

CA1812 should not warn for top-level Program classes #5628

Closed
martincostello opened this issue Oct 13, 2021 · 25 comments
Closed

CA1812 should not warn for top-level Program classes #5628

martincostello opened this issue Oct 13, 2021 · 25 comments
Assignees
Labels
False_Positive A diagnostic is reported for non-problematic case Resolution-Fixed

Comments

@martincostello
Copy link
Member

Analyzer

Diagnostic ID: CA1812

Describe the improvement

An application that uses top-level programs and uses an analysis rule set that includes CA1812 will generate a warning from the compiler-generated Program class in even a trivial application.

Console.WriteLine("Hello, World!");
Program.cs(1,1): warning CA1812: Program is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it static (Shared in Visual Basic). [Project.csproj]

This then requires the developer to suppress it like so:

#pragma warning disable CA1812
Console.WriteLine("Hello, World!");

Describe suggestions on how to achieve the rule

Special-case compiler-generated Program classes for top-level programs and do not emit the violation.

Additional context

Application that reproduces the problem adapted from dotnet new console using the .NET 6 RC2 SDK.

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <AnalysisMode>AllEnabledByDefault</AnalysisMode>
    <EnableNETAnalyzers>true</EnableNETAnalyzers>
    <ImplicitUsings>enable</ImplicitUsings>
    <NoWarn>$(NoWarn);CA1014;CA1303</NoWarn>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
</Project>
Console.WriteLine("Hello, World!");
@mavasani
Copy link
Contributor

I think this was recently fixed by @Youssef1313

@Youssef1313
Copy link
Member

Yeah I believe it should have been fixed. Not aware which version contains the fix though.

@martincostello
Copy link
Member Author

Anything required beyond adding the package? Doesn't seem to have any effect if I add the relevant feed to nuget.config and add this to the project file:

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="7.0.0-preview1.21513.1">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

@Youssef1313
Copy link
Member

Likely fixed in #5351.

@mavasani
Copy link
Contributor

Thanks @Youssef1313

@adamralph
Copy link
Contributor

This still occurs in the latest RC (6.0.100-rc.2.21505.57), see https://github.com/adamralph/minver/runs/4132244037?check_suite_focus=true

Weirdly, it's only a problem on Windows and MacOS, but not Linux.

Is the intention to fix this before RTM?

@Youssef1313
Copy link
Member

It looks like the milestone for #5351 was set to .NET 7.

@mavasani @jmarolf Can this get in .NET 6?

@martincostello
Copy link
Member Author

Yeah I've seen this not fixed in 6.0.0/6.0.100 either, but that's already been built and ships today so would need backporting to 6.0.1.

@adamralph
Copy link
Contributor

@martincostello I realise RTM is tomorrow, but all I see at https://dotnet.microsoft.com/download/dotnet/6.0 is 6.0.100-rc.2.21505.57.

@martincostello
Copy link
Member Author

I believe it's shipping today ahead of .NET Conf tomorrow - the 6.0.0 packages are all in NuGet.org now (I woke up this morning to various dependabot updates), and the 6.0.100 SDK has been present at the URL it lives at for CI scripts for a week or so. They just haven't updated the releases.json file (which powers the .NET website's download links) yet.

@jmarolf
Copy link
Contributor

jmarolf commented Nov 8, 2021

It looks like the milestone for #5351 was set to .NET 7.

Manish Vasani Jonathon Marolf Can this get in .NET 6?

sadly, no by the time the fix was merged final builds of .NET 6 were already spinning for today. Considering CA1812 is off by default I don't think this meets the bar to port as a day 0 fix. but I will pursue it for the 6.0.200 SDK release

@michha
Copy link

michha commented Dec 9, 2021

@jmarolf
Will this be fixed in 6.0.200 SDK? Is there a rough timeline for its release?

@jmarolf jmarolf self-assigned this Dec 9, 2021
@jmarolf jmarolf added this to the .NET 6.x milestone Dec 9, 2021
@Evangelink Evangelink added False_Positive A diagnostic is reported for non-problematic case Resolution-Fixed labels Jan 24, 2022
@Grauenwolf
Copy link

It is fixed in 7.0.0-preview1.22168.1.

	<ItemGroup>
		<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="7.0.0-preview1.22168.1">
			<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
			<PrivateAssets>all</PrivateAssets>
		</PackageReference>
	</ItemGroup>

@Youssef1313
Copy link
Member

@jmarolf @mavasani This one is already fixed, but for 7.x.

The request here for this issue is backporting to 6.x. I don't know if it meets the bar for backporting. If not, can we close?

@MrDaedra
Copy link

MrDaedra commented Jul 6, 2022

Any updates on this fix?

@Youssef1313 as .NET 6 is in LTS it makes sense to backport it

@Youssef1313
Copy link
Member

@Youssef1313 as .NET 6 is in LTS it makes sense to backport it

@mavasani @jmarolf Can you comment on whether this can be backported?

@mavasani
Copy link
Contributor

mavasani commented Jul 7, 2022

Tagging @buyaa-n

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 9, 2022

As @jmarolf mentioned the analyzer is off by default and there is a workaround (suppress), therefore this does not meet the bar for servicing 6.0, I guess it could be considered if 1st party customer requesting it

@BryceBarbara
Copy link

BryceBarbara commented Jul 12, 2022

Just to share with others who might be encountering this, I was able to disable this for all projects in my solution by putting this in the .editorconfig

# Workaround for https://github.com/dotnet/roslyn-analyzers/issues/5628
[Program.cs]
dotnet_diagnostic.ca1812.severity = none

@Youssef1313
Copy link
Member

Closing as this is fixed in 7.x and doesn't meet the bar to backport for 6.x

@kimsey0
Copy link

kimsey0 commented Jan 10, 2023

Which version is this fixed from? I'm still getting a warning if I create a new console project and enable the Recommended analysis mode with the .NET SDK version 7.0.101.

@Youssef1313
Copy link
Member

@kimsey0 I think the fix didn't yet flow into the .NET SDK.

Can you try with 7.0.0 from NuGet.org?

@kimsey0
Copy link

kimsey0 commented Jan 10, 2023

That does indeed seem to fix it. I just assumed that since Microsoft.CodeAnalysis.NetAnalyzers 7.0.0 was released on December 8th and .NET SDK 7.0.1 was released on December 13th, it would include the newest analyzers, but there might be a bit of a delay for the flow. (7.0.1 is also marked as a security patch, so I don't know if that influences anything.)

I hope the analyzer changes will be out in the next SDK release. :-)

@dotnetshadow
Copy link

When I try with nuget 7.0.1 version I still get the issue?

image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False_Positive A diagnostic is reported for non-problematic case Resolution-Fixed
Projects
None yet
Development

No branches or pull requests