-
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
Implement RS1041 (Compiler extensions should be implemented in assemblies targeting netstandard2.0) #7116
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7116 +/- ##
==========================================
- Coverage 96.42% 96.42% -0.01%
==========================================
Files 1412 1414 +2
Lines 337058 337480 +422
Branches 11145 11166 +21
==========================================
+ Hits 325020 325421 +401
- Misses 9231 9240 +9
- Partials 2807 2819 +12 |
src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx
Show resolved
Hide resolved
...eAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompilerExtensionTargetFrameworkAnalyzerTests.cs
Show resolved
Hide resolved
…lies targeting netstandard2.0)
0308079
to
48d4ebe
Compare
string displayName; | ||
switch (appliedTargetFrameworkAttribute.ConstructorArguments[0].Value as string) | ||
{ | ||
case ".NETStandard,Version=v1.0": |
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.
The compiler already has the ability to pass MSBuild properties through as analyzer config options. By default we pass none. I'm starting to wonder if we should automatically pass a few ones like <TargetFramework>
here. There are a number of analyzers that are interested in TFM. Looking at it that way doesn't require an type of semantic model access.
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.
Is this done by project-system?
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 repository has the ability to (and does in some cases) add such properties to the list of analyzer-exposed values. I can convert the analyzer to use this if desired. It could also be left as a follow-up.
var namedType = (INamedTypeSymbol)context.Symbol; | ||
if (!IsRegisteredExtension(namedType, diagnosticAnalyzer, diagnosticAnalyzerAttribute, out var applicationSyntaxReference) | ||
&& !IsRegisteredExtension(namedType, sourceGeneratorInterface, generatorAttribute, out applicationSyntaxReference) | ||
&& !IsRegisteredExtension(namedType, incrementalGeneratorInterface, generatorAttribute, out applicationSyntaxReference)) | ||
{ | ||
// This is not a compiler extension | ||
return; | ||
} |
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.
Is it intentional to not check CodeFixProvider
?
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.
➡️ Yes. CodeFixProvider is an IDE feature, not a compiler feature.
No description provided.