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

Implement RS1041 (Compiler extensions should be implemented in assemblies targeting netstandard2.0) #7116

Merged
merged 1 commit into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/rules/RS1041.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## RS1041: Compiler extensions should be implemented in assemblies targeting netstandard2.0

Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.

|Item|Value|
|-|-|
|Category|MicrosoftCodeAnalysisCorrectness|
|Enabled|True|
|Severity|Warning|
|CodeFix|False|
---

This rule helps ensure compiler extensions (e.g. analyzers and source generators) will load correctly in all compilation
scenarios. Depending on the manner in which the compiler is invoked, the compiler may execute under .NET Framework or
.NET, and compiler extensions are expected to work consistently in both cases. By targeting netstandard2.0, compiler
extensions are known to be compatible with both execution environments.

### Compiler extension points

The following compiler extension points are examined by this analyzer:

* `DiagnosticAnalyzer`
* `DiagnosticSuppressor`
* `ISourceGenerator`
* `IIncrementalGenerator`
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RS1038 | MicrosoftCodeAnalysisCorrectness | Warning | CompilerExtensionStrictApiAnalyzer
RS1041 | MicrosoftCodeAnalysisCorrectness | Warning | CompilerExtensionTargetFrameworkAnalyzer, [Documentation](https://github.com/dotnet/roslyn-analyzers/blob/main/docs/rules/RS1041.md)
Original file line number Diff line number Diff line change
Expand Up @@ -586,4 +586,13 @@
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullForFieldMessage" xml:space="preserve">
<value>A call to 'SemanticModel.GetDeclaredSymbol({0})' will always return 'null'</value>
</data>
<data name="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkMessage" xml:space="preserve">
sharwell marked this conversation as resolved.
Show resolved Hide resolved
<value>This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</value>
</data>
<data name="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleDescription" xml:space="preserve">
<value>Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</value>
</data>
<data name="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleTitle" xml:space="preserve">
<value>Compiler extensions should be implemented in assemblies targeting netstandard2.0</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ internal static class DiagnosticIds
public const string DoNotRegisterCompilerTypesWithBadAssemblyReferenceRuleId = "RS1038";
public const string SemanticModelGetDeclaredSymbolAlwaysReturnsNull = "RS1039";
public const string SemanticModelGetDeclaredSymbolAlwaysReturnsNullForField = "RS1040";
public const string DoNotRegisterCompilerTypesWithBadTargetFrameworkRuleId = "RS1041";

// Release tracking analyzer IDs
public const string DeclareDiagnosticIdInAnalyzerReleaseRuleId = "RS2000";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Runtime.Versioning;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers
{
using static CodeAnalysisDiagnosticsResources;

[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
internal sealed class CompilerExtensionTargetFrameworkAnalyzer : DiagnosticAnalyzer
{
private static readonly LocalizableString s_localizableTitle = CreateLocalizableResourceString(nameof(DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleTitle));
private static readonly LocalizableString s_localizableDescription = CreateLocalizableResourceString(nameof(DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleDescription));
private const string HelpLinkUri = "https://github.com/dotnet/roslyn-analyzers/blob/main/docs/rules/RS1041.md";

public static readonly DiagnosticDescriptor DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkStrictRule = new(
DiagnosticIds.DoNotRegisterCompilerTypesWithBadTargetFrameworkRuleId,
s_localizableTitle,
CreateLocalizableResourceString(nameof(DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkMessage)),
DiagnosticCategory.MicrosoftCodeAnalysisCorrectness,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: s_localizableDescription,
helpLinkUri: HelpLinkUri,
customTags: WellKnownDiagnosticTagsExtensions.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkStrictRule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);

context.RegisterCompilationStartAction(context =>
{
var typeProvider = WellKnownTypeProvider.GetOrCreate(context.Compilation);
var diagnosticAnalyzer = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisDiagnosticsDiagnosticAnalyzer);
if (diagnosticAnalyzer is null)
return;

var targetFrameworkAttribute = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemRuntimeVersioningTargetFrameworkAttribute);
if (targetFrameworkAttribute is null)
return;

AttributeData? appliedTargetFrameworkAttribute = context.Compilation.Assembly.GetAttribute(targetFrameworkAttribute);
if (appliedTargetFrameworkAttribute is null)
return;

if (appliedTargetFrameworkAttribute.ConstructorArguments.IsEmpty)
{
return;
}

string displayName;
switch (appliedTargetFrameworkAttribute.ConstructorArguments[0].Value as string)
{
case ".NETStandard,Version=v1.0":
Copy link
Member

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.

Copy link
Member

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?

Copy link
Member Author

@sharwell sharwell Jan 2, 2024

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.

case ".NETStandard,Version=v1.1":
case ".NETStandard,Version=v1.2":
case ".NETStandard,Version=v1.3":
case ".NETStandard,Version=v1.4":
case ".NETStandard,Version=v1.5":
case ".NETStandard,Version=v1.6":
case ".NETStandard,Version=v2.0":
// The compiler supports this target framework
return;

default:
displayName = appliedTargetFrameworkAttribute.NamedArguments.FirstOrDefault(arg => arg.Key == nameof(TargetFrameworkAttribute.FrameworkDisplayName)).Value.Value as string
?? appliedTargetFrameworkAttribute.ConstructorArguments[0].Value as string
?? "<unknown>";
break;
}

var diagnosticAnalyzerAttribute = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisDiagnosticsDiagnosticAnalyzerAttribute);
var sourceGeneratorInterface = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisISourceGenerator);
var incrementalGeneratorInterface = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisIIncrementalGenerator);
var generatorAttribute = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisGeneratorAttribute);

context.RegisterSymbolAction(
context =>
{
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;
}
Comment on lines +91 to +98
Copy link
Member

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?

Copy link
Member Author

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.


context.ReportDiagnostic(Diagnostic.Create(
DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkStrictRule,
Location.Create(applicationSyntaxReference.SyntaxTree, applicationSyntaxReference.Span),
displayName));
},
SymbolKind.NamedType);
});
}

private static bool IsRegisteredExtension(INamedTypeSymbol extension, [NotNullWhen(true)] INamedTypeSymbol? extensionClassOrInterface, [NotNullWhen(true)] INamedTypeSymbol? registrationAttributeType, [NotNullWhen(true)] out SyntaxReference? node)
{
if (!extension.Inherits(extensionClassOrInterface))
{
node = null;
return false;
}

foreach (var attribute in extension.GetAttributes())
{
// Only examine extension registrations in source
Debug.Assert(attribute.ApplicationSyntaxReference is not null,
$"Expected attributes returned by {nameof(ISymbol.GetAttributes)} (as opposed to {nameof(ITypeSymbolExtensions.GetApplicableAttributes)}) to have a non-null application.");
if (attribute.ApplicationSyntaxReference is null)
continue;

if (!attribute.AttributeClass.Inherits(registrationAttributeType))
continue;

node = attribute.ApplicationSyntaxReference;
return true;
}

node = null;
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@
<target state="translated">Toto rozšíření kompilátoru jazyka C# by nemělo být implementováno v sestavení obsahujícím odkaz na Microsoft.CodeAnalysis.VisualBasic. Sestavení Microsoft.CodeAnalysis.VisualBasic není vždy poskytováno během kompilačních scénářů jazyka C#, takže odkazy na něj by mohly způsobit, že se rozšíření kompilátoru bude chovat nepředvídatelně.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkMessage">
<source>This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</source>
<target state="new">This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleDescription">
<source>Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</source>
<target state="new">Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleTitle">
<source>Compiler extensions should be implemented in assemblies targeting netstandard2.0</source>
<target state="new">Compiler extensions should be implemented in assemblies targeting netstandard2.0</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithWorkspacesReferenceMessage">
<source>This compiler extension should not be implemented in an assembly containing a reference to Microsoft.CodeAnalysis.Workspaces. The Microsoft.CodeAnalysis.Workspaces assembly is not provided during command line compilation scenarios, so references to it could cause the compiler extension to behave unpredictably.</source>
<target state="translated">Toto rozšíření kompilátoru by nemělo být implementováno v sestavení obsahujícím odkaz na Microsoft.CodeAnalysis.Workspaces. Sestavení Microsoft.CodeAnalysis.Workspaces se během scénářů kompilace příkazového řádku neposkytuje, takže odkazy na něj by mohly způsobit, že se rozšíření kompilátoru bude chovat nepředvídatelně.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@
<target state="translated">Diese C#-Compilererweiterung sollte nicht in einer Assembly implementiert werden, die einen Verweis auf "Microsoft.CodeAnalysis.VisualBasic" enthält. Die Assembly "Microsoft.CodeAnalysis.VisualBasic" wird nicht immer in C#-Kompilierungsszenarien bereitgestellt. Verweise darauf können daher ein unvorhersehbares Verhalten der Compilererweiterung verursachen.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkMessage">
<source>This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</source>
<target state="new">This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleDescription">
<source>Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</source>
<target state="new">Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleTitle">
<source>Compiler extensions should be implemented in assemblies targeting netstandard2.0</source>
<target state="new">Compiler extensions should be implemented in assemblies targeting netstandard2.0</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithWorkspacesReferenceMessage">
<source>This compiler extension should not be implemented in an assembly containing a reference to Microsoft.CodeAnalysis.Workspaces. The Microsoft.CodeAnalysis.Workspaces assembly is not provided during command line compilation scenarios, so references to it could cause the compiler extension to behave unpredictably.</source>
<target state="translated">Diese Compilererweiterung sollte nicht in einer Assembly implementiert werden, die einen Verweis auf "Microsoft.CodeAnalysis.Workspaces" enthält. Die Assembly "Microsoft.CodeAnalysis.Workspaces" wird in Befehlszeilen-Kompilierungsszenarien nicht bereitgestellt. Verweise darauf können daher ein unvorhersehbares Verhalten der Compilererweiterung verursachen.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@
<target state="translated">Esta extensión del compilador de C# no debe implementarse en un ensamblado que contenga una referencia a Microsoft.CodeAnalysis.VisualBasic. El ensamblado Microsoft.CodeAnalysis.VisualBasic no siempre se proporciona durante los escenarios de compilación de C#, por lo que las referencias a él podrían hacer que la extensión del compilador se comporte de forma impredecible.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkMessage">
<source>This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</source>
<target state="new">This compiler extension should not be implemented in an assembly with target framework '{0}'. References to other target frameworks will cause the compiler to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleDescription">
<source>Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</source>
<target state="new">Types which implement compiler extension points should only be declared in assemblies targeting netstandard2.0. More specific target frameworks are only available in a subset of supported compilation scenarios, so targeting them may cause the feature to behave unpredictably.</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithUnsupportedTargetFrameworkRuleTitle">
<source>Compiler extensions should be implemented in assemblies targeting netstandard2.0</source>
<target state="new">Compiler extensions should be implemented in assemblies targeting netstandard2.0</target>
<note />
</trans-unit>
<trans-unit id="DoNotDeclareCompilerFeatureInAssemblyWithWorkspacesReferenceMessage">
<source>This compiler extension should not be implemented in an assembly containing a reference to Microsoft.CodeAnalysis.Workspaces. The Microsoft.CodeAnalysis.Workspaces assembly is not provided during command line compilation scenarios, so references to it could cause the compiler extension to behave unpredictably.</source>
<target state="translated">Esta extensión del compilador no debe implementarse en un ensamblado que contenga una referencia a Microsoft.CodeAnalysis.Workspaces. El ensamblado Microsoft.CodeAnalysis.Workspaces no se proporciona durante los escenarios de compilación de la línea de comandos, por lo que las referencias a él podrían hacer que la extensión del compilador se comporte de forma impredecible.</target>
Expand Down
Loading