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

CA1852 warns on generated Program for top-level statements #6141

Closed
kronic opened this issue Sep 1, 2022 · 20 comments · Fixed by #6278
Closed

CA1852 warns on generated Program for top-level statements #6141

kronic opened this issue Sep 1, 2022 · 20 comments · Fixed by #6278

Comments

@kronic
Copy link

kronic commented Sep 1, 2022

Description

Can't get rid of error CA1852

Reproduction Steps

Create default console project.
add .editorconfig with
dotnet_diagnostic.CA1852.severity = error

Expected behavior

genrate program class sealed

Actual behavior

genrate program class no sealed

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged label Sep 1, 2022
@stephentoub
Copy link
Member

Thanks. This is a bug in CA1852; it shouldn't be warning on the generated Program for top-level statements.

@julealgon
Copy link

@stephentoub do you have any idea about when this patch will be released?

@stephentoub
Copy link
Member

I don't know. @mavasani?

@mavasani
Copy link
Contributor

Tagging @jmarolf

@mavasani mavasani changed the title CS1852 warns on generated Program for top-level statements CA1852 warns on generated Program for top-level statements Jan 6, 2023
@cremor
Copy link

cremor commented Jan 11, 2023

This is still not fixed in SDK 7.0.102. But installing the NuGet package "Microsoft.CodeAnalysis.NetAnalyzers" 7.0.0 manually fixes it. Does the SDK not contain the release version of the NuGet package?

amis92 added a commit to BSData/gallery-dashboard that referenced this issue Feb 2, 2023
This rule is currently being incorrectly applied to the generated
`Program` class [1] and therefore has to be suppressed manually.

A fix [2] has already been committed to the Roslyn Analyzers repo once
that fix has been included in a future SDK update.

[1] dotnet/roslyn-analyzers#6141
[2] dotnet/roslyn-analyzers#6278
@jhartmann123
Copy link
Contributor

@mavasani @jmarolf The issue still appears with SDK 7.0.200. It looks like the fix from #6278 is only on main, but not on the 7.x release branch.

Current main with check for the top level statements:

if (type.TypeKind is TypeKind.Class &&
!type.IsAbstract &&
!type.IsStatic &&
!type.IsSealed &&
!type.IsExternallyVisible() &&
!type.HasAttribute(comImportAttributeType) &&
!type.IsTopLevelStatementsEntryPointType())
{
candidateTypes.Add(type);
}

Current branch release/7.0.1xx without this check:

if (type.TypeKind is TypeKind.Class &&
!type.IsAbstract &&
!type.IsStatic &&
!type.IsSealed &&
!type.IsExternallyVisible() &&
!type.HasAttribute(comImportAttributeType))
{
candidateTypes.Add(type);
}

smfeest added a commit to smfeest/buttercup that referenced this issue Mar 5, 2023
As mentioned in the previous commit, .NET 7 includes a new analyzer [1]
that checks for internal types that can be sealed.

Unfortunately that analyzer is currently generating false positives for
generated `Program` classes [2] and therefore has to be suppressed
manually.

A fix [3] has already been committed to the Roslyn Analyzers repo so we
should be able to remove this suppression once that fix has made its way
into an SDK update.

[1] dotnet/roslyn-analyzers#5594
[2] dotnet/roslyn-analyzers#6141
[3] dotnet/roslyn-analyzers#6278
@dotnetshadow
Copy link

dotnetshadow commented Mar 16, 2023

I'm still getting this issue.
#5628 (comment)

I think the way this issue has been handled is a bit of a mess. It's fixed, then not fixed for some servicing. It's then not included in SDK then it is then it's not... what is the ultimate status of this issue?

@buyaa-n
Copy link
Contributor

buyaa-n commented Mar 16, 2023

FYI it is only fixed in .NET 8, never ported to 7.0 before except the #6504, we will try to add that fix in next 7.0 release.

@dotnetshadow
Copy link

Looks like this has been fixed

@JamieMagee
Copy link
Member

This appears to have regressed in 7.0.302 (May 2023)

@mavasani
Copy link
Contributor

@buyaa-n - did the code flow/branching into dotnet/sdk repo break?

@JamieMagee
Copy link
Member

Here are some examples highlighting the regression:

@buyaa-n
Copy link
Contributor

buyaa-n commented May 16, 2023

@buyaa-n - did the code flow/branching into dotnet/sdk repo break?

I don't really know how it is supposed to flow, I don't see the change added in 7.0.2 included in 7.0.3

@QuintinWillison
Copy link

I've also seen this (private repository, so I can't share a link). For what it's worth, I don't get this failing locally (Windows) but do get it failing in CI (Linux GitHub runner).

@menees
Copy link

menees commented May 17, 2023

One workaround is to add this #pragma at the top of the Program.cs file:

// CA1852 Type 'Program' can be sealed because it has no subtypes in its containing assembly and is not externally visible
#pragma warning disable CA1852

@buyaa-n
Copy link
Contributor

buyaa-n commented May 17, 2023

Update: we are working on adding this to the next 7.0.3 release

@buyaa-n
Copy link
Contributor

buyaa-n commented May 23, 2023

The fix merged into next 7.0.3 release, will be ported to 7.0.4 too

@Emeka-MSFT
Copy link

Update: we are working on adding this to the next 7.0.3 release

Can you also consider adding a test, so that it remains "fixed".

@mavasani
Copy link
Contributor

Update: we are working on adding this to the next 7.0.3 release

Can you also consider adding a test, so that it remains "fixed".

The test was already added with original fix. The regression happened due to branching/code flow issue between this repo and the dotnet/sdk repo

smfeest added a commit to smfeest/buttercup that referenced this issue Nov 19, 2023
The bug [1] that made adding this suppression [2] necessary has been
fixed [3] in .NET 8.

[1] dotnet/roslyn-analyzers#6141
[2] 5a4dc3d
[3] dotnet/roslyn-analyzers#6278
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.