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

Enable Analyzers disabled as a part of globalconfig merge from runtime #7887

Open
elachlan opened this issue Oct 5, 2022 · 8 comments
Open
Assignees
Labels
enhancement Product code improvement that does NOT require public API changes/additions untriaged The team needs to look at this issue in the next triage
Milestone

Comments

@elachlan
Copy link
Contributor

elachlan commented Oct 5, 2022

Is your feature request related to a problem? Please describe

List of diagnostics that need to be investigated, fixed, and enabled.

Line  297: dotnet_diagnostic.CA1821.severity = none # TODO: warning
Line  466: dotnet_diagnostic.CA2208.severity = none # TODO: warning
Line  533: dotnet_diagnostic.CA2245.severity = none # TODO: warning
Line  851: dotnet_diagnostic.IL3000.severity = none # TODO: warning
Line 1137: dotnet_diagnostic.SA1302.severity = none # TODO: warning
Line 1179: dotnet_diagnostic.SA1400.severity = none # TODO: warning
Line 1191: dotnet_diagnostic.SA1404.severity = none # TODO: warning
Line 1608: dotnet_diagnostic.IDE0059.severity = none # TODO: warning

Describe the solution you'd like and alternatives you've considered

N/A

Will this feature affect UI controls?

N/A

@elachlan elachlan added api-suggestion (1) Early API idea and discussion, it is NOT ready for implementation untriaged The team needs to look at this issue in the next triage labels Oct 5, 2022
@elachlan
Copy link
Contributor Author

elachlan commented Oct 5, 2022

I started investigating CA1416(https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1416). Winforms would have to swap out its OsVersion static class with OperatingSystem.IsWindowsVersionAtLeast().

@JeremyKuhne Coincidently this would remove the need for us to convert the NtDll PInvoke.

Edit: runtime does cache the OSVersion.

@elachlan elachlan changed the title Enable Analyzer disabled as a part of globalconfig merge from runtime Enable Analyzers disabled as a part of globalconfig merge from runtime Oct 5, 2022
@RussKie RussKie removed the untriaged The team needs to look at this issue in the next triage label Oct 5, 2022
@RussKie RussKie added enhancement Product code improvement that does NOT require public API changes/additions untriaged The team needs to look at this issue in the next triage and removed api-suggestion (1) Early API idea and discussion, it is NOT ready for implementation untriaged The team needs to look at this issue in the next triage labels Oct 5, 2022
@RussKie RussKie added this to the .NET 8.0 milestone Oct 5, 2022
@elachlan
Copy link
Contributor Author

@RussKie CA2007 has single violation:

Task.Run(Async Function() As Task
Await _splashScreenCompletionSource.Task
_formLoadWaiter.Set()
End Function)

I am unsure if ConfigureAwait(false) is correct. I am pretty sure it is, since it should be resolved inside the Task.Run().

@elachlan
Copy link
Contributor Author

@dreddy-work IDE0059 is complimented by CA1804. Can I enable CA1804 (Remove unused locals)?

@elachlan
Copy link
Contributor Author

Turns out CA1804 is replaced by IDE0059? But IDE0059 is replacing values with underscores.

@dreddy-work

This comment was marked as resolved.

@elachlan

This comment was marked as resolved.

@JeremyKuhne JeremyKuhne modified the milestones: .NET 8.0, .NET 9.0 Aug 16, 2023
@JeremyKuhne
Copy link
Member

@elachlan I think we're good here now, aren't we?

@JeremyKuhne JeremyKuhne added the 📭 waiting-author-feedback The team requires more information from the author label Jul 24, 2024
@JeremyKuhne JeremyKuhne modified the milestones: .NET 9.0, Future Jul 24, 2024
@elachlan
Copy link
Contributor Author

The ones that need to be addressed are marked as TODO in CodeAnalysis.src.globalconfig and CodeAnalysis.test.globalconfig

	\winforms\eng\CodeAnalysis.src.globalconfig (8 hits)
	Line  297: dotnet_diagnostic.CA1821.severity = none # TODO: warning
	Line  466: dotnet_diagnostic.CA2208.severity = none # TODO: warning
	Line  533: dotnet_diagnostic.CA2245.severity = none # TODO: warning
	Line  851: dotnet_diagnostic.IL3000.severity = none # TODO: warning
	Line 1097: # SA1206: Keyword ordering - TODO Re-enable as warning after https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3527
	Line 1137: dotnet_diagnostic.SA1302.severity = none # TODO: warning
	Line 1191: dotnet_diagnostic.SA1404.severity = none # TODO: warning
	Line 1608: dotnet_diagnostic.IDE0059.severity = none # TODO: warning
  \winforms\eng\CodeAnalysis.test.globalconfig (7 hits)
	Line   7: dotnet_diagnostic.CA1052.severity = none # TODO: warning
	Line  14: dotnet_diagnostic.CA1416.severity = none # TODO: warning
	Line  26: dotnet_diagnostic.CA1810.severity = none # TODO: warning
	Line  35: dotnet_diagnostic.CA1822.severity = none # TODO: warning
	Line  48: dotnet_diagnostic.CA2007.severity = none # TODO: warning
	Line  51: dotnet_diagnostic.CA2014.severity = none # TODO: warning
	Line 327: dotnet_diagnostic.xUnit1004.severity = silent # TODO: warning

@dotnet-policy-service dotnet-policy-service bot added untriaged The team needs to look at this issue in the next triage and removed 📭 waiting-author-feedback The team requires more information from the author labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Product code improvement that does NOT require public API changes/additions untriaged The team needs to look at this issue in the next triage
Projects
None yet
Development

No branches or pull requests

4 participants