-
Notifications
You must be signed in to change notification settings - Fork 468
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
Error when users use a file
type for things we need to load
#7501
Conversation
Inspired by dotnet/roslyn#76355. `file` types can cause errors on .NET Framework hosts, and we should just prevent these from being used. The purpose of `file` types is to not be visible outside the current file, and the purpose of analyzers is to be loaded by some other plugin system. These goals are mutually exclusive.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7501 +/- ##
========================================
Coverage 96.50% 96.50%
========================================
Files 1450 1452 +2
Lines 347435 347566 +131
Branches 11413 11416 +3
========================================
+ Hits 335297 335426 +129
- Misses 9245 9246 +1
- Partials 2893 2894 +1 |
...osoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/DoNotUseFileTypesForAnalyzersOrGenerators.cs
Show resolved
Hide resolved
...osoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/DoNotUseFileTypesForAnalyzersOrGenerators.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/DoNotUseFileTypesForAnalyzersOrGenerators.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.CodeAnalysis.Analyzers/Microsoft.CodeAnalysis.Analyzers.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems more like a compiler bug than an analyzer issue, but can maybe be mitigated here. Would recommend the compiler report a better error for the scenario, as users who encounter the problem will not be able to make the connection between the current error and this change.
CreateLocalizableResourceString(nameof(DoNotUseFileTypesForAnalyzersOrGeneratorsTitle)), | ||
CreateLocalizableResourceString(nameof(DoNotUseFileTypesForAnalyzersOrGeneratorsMessage)), | ||
DiagnosticCategory.MicrosoftCodeAnalysisCorrectness, | ||
DiagnosticSeverity.Error, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Severity should be warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is an error, then users do not need to make a connection between a compiler load failure and a warning.
Inspired by dotnet/roslyn#76355.
file
types can cause errors on .NET Framework hosts, and we should just prevent these from being used. The purpose offile
types is to not be visible outside the current file, and the purpose of analyzers is to be loaded by some other plugin system. These goals are mutually exclusive.