diff --git a/doc/analyzers/VSMEF003.md b/doc/analyzers/VSMEF003.md new file mode 100644 index 000000000..97535a2e5 --- /dev/null +++ b/doc/analyzers/VSMEF003.md @@ -0,0 +1,62 @@ +# VSMEF003: Exported type not implemented by exporting class + +A class that declares `[Export(typeof(T))]` should implement interface `T` or inherit from class `T`. + +## Cause + +A class is decorated with `[Export(typeof(T))]` but does not implement the interface or inherit from the class specified by `T`. + +## Rule description + +When using MEF Export attributes with an explicit type parameter, the exporting class should implement that type. If the class does not implement the specified interface or inherit from the specified base class, it will likely cause runtime composition failures or unexpected behavior. + +## How to fix violations + +Either: + +- Make the exporting class implement the specified interface, or +- Make the exporting class inherit from the specified base class, or +- Change the Export attribute to export the correct type, or +- Remove the type parameter to export the class's own type + +## When to suppress warnings + +This warning can be suppressed if you intentionally want to export a type that is not implemented by the exporting class, though this is rarely a good practice and may cause composition issues at runtime. + +## Example + +### Violates + +```csharp +interface ICalculator +{ + int Add(int a, int b); +} + +[Export(typeof(ICalculator))] // ❌ Violates VSMEF003 +public class TextProcessor +{ + public string ProcessText(string input) => input.ToUpper(); +} +``` + +### Does not violate + +```csharp +interface ICalculator +{ + int Add(int a, int b); +} + +[Export(typeof(ICalculator))] // ✅ OK +public class Calculator : ICalculator +{ + public int Add(int a, int b) => a + b; +} + +[Export] // ✅ OK - exports Calculator type +public class Calculator +{ + public int Add(int a, int b) => a + b; +} +``` \ No newline at end of file diff --git a/doc/analyzers/index.md b/doc/analyzers/index.md index 42172e286..1dd70af88 100644 --- a/doc/analyzers/index.md +++ b/doc/analyzers/index.md @@ -7,3 +7,5 @@ to help you avoid common mistakes while authoring MEF parts. ID | Title --|-- VSMEF001 | Importing property must have setter +VSMEF002 | Avoid mixing MEF attribute libraries +VSMEF003 | Exported type not implemented by exporting class diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md b/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md index 6640189c3..0f676e15b 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md @@ -4,3 +4,4 @@ ### New Rules Rule ID | Category | Severity | Notes --------|----------|----------|------- +VSMEF003 | Usage | Warning | Exported type not implemented by exporting class diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/README.md b/src/Microsoft.VisualStudio.Composition.Analyzers/README.md index 8cc541048..69713b136 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/README.md +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/README.md @@ -5,3 +5,4 @@ Analyzers for MEF consumers to help identify common errors in MEF parts. Analyzer ID | Description --|-- VSMEF001 | Ensures that importing properties define a `set` accessor. +VSMEF003 | Ensures that exported types are implemented by the exporting class. diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.Designer.cs b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.Designer.cs index 45cbe4424..1ae2e710c 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.Designer.cs +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.Designer.cs @@ -95,5 +95,23 @@ internal static string VSMEF002_Title { return ResourceManager.GetString("VSMEF002_Title", resourceCulture); } } + + /// + /// Looks up a localized string similar to The type "{0}" does not implement the exported type "{1}". This may be an authoring mistake.. + /// + internal static string VSMEF003_MessageFormat { + get { + return ResourceManager.GetString("VSMEF003_MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Exported type not implemented by exporting class. + /// + internal static string VSMEF003_Title { + get { + return ResourceManager.GetString("VSMEF003_Title", resourceCulture); + } + } } } diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx index cc0d261bd..e338d192b 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx @@ -129,4 +129,10 @@ Avoid mixing MEF attribute varieties + + The type "{0}" does not implement the exported type "{1}". This may be an authoring mistake. + + + Exported type not implemented by exporting class + \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF003ExportTypeMismatchAnalyzer.cs b/src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF003ExportTypeMismatchAnalyzer.cs new file mode 100644 index 000000000..dfa6039c9 --- /dev/null +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF003ExportTypeMismatchAnalyzer.cs @@ -0,0 +1,180 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.VisualStudio.Composition.Analyzers; + +/// +/// Creates a diagnostic when `[Export(typeof(T))]` is applied to a class that does not implement T, +/// or to a property whose type is not compatible with T. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] +public class VSMEF003ExportTypeMismatchAnalyzer : DiagnosticAnalyzer +{ + /// + /// The ID for diagnostics reported by this analyzer. + /// + public const string Id = "VSMEF003"; + + /// + /// The descriptor used for diagnostics created by this rule. + /// + internal static readonly DiagnosticDescriptor Descriptor = new( + id: Id, + title: Strings.VSMEF003_Title, + messageFormat: Strings.VSMEF003_MessageFormat, + helpLinkUri: Utils.GetHelpLink(Id), + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Descriptor); + + /// + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); + + context.RegisterCompilationStartAction(context => + { + // Only scan further if the compilation references the assemblies that define the attributes we'll be looking for. + if (context.Compilation.ReferencedAssemblyNames.Any(i => + string.Equals(i.Name, "System.ComponentModel.Composition", StringComparison.OrdinalIgnoreCase) || + string.Equals(i.Name, "System.Composition.AttributedModel", StringComparison.OrdinalIgnoreCase))) + { + INamedTypeSymbol? mefV1ExportAttribute = context.Compilation.GetTypeByMetadataName("System.ComponentModel.Composition.ExportAttribute"); + INamedTypeSymbol? mefV2ExportAttribute = context.Compilation.GetTypeByMetadataName("System.Composition.ExportAttribute"); + context.RegisterSymbolAction( + context => AnalyzeTypeDeclaration(context, mefV1ExportAttribute, mefV2ExportAttribute), + SymbolKind.NamedType); + context.RegisterSymbolAction( + context => AnalyzePropertyDeclaration(context, mefV1ExportAttribute, mefV2ExportAttribute), + SymbolKind.Property); + } + }); + } + + private static void AnalyzeTypeDeclaration(SymbolAnalysisContext context, INamedTypeSymbol? mefV1ExportAttribute, INamedTypeSymbol? mefV2ExportAttribute) + { + var namedType = (INamedTypeSymbol)context.Symbol; + + // Skip interfaces, enums, delegates - only analyze classes + if (namedType.TypeKind != TypeKind.Class) + { + return; + } + + Location? location = namedType.Locations.FirstOrDefault(); + if (location is null) + { + // We won't have anywhere to publish a diagnostic anyway. + return; + } + + foreach (var attributeData in namedType.GetAttributes()) + { + // Check if this is an Export attribute + if (SymbolEqualityComparer.Default.Equals(attributeData.AttributeClass, mefV1ExportAttribute) || + SymbolEqualityComparer.Default.Equals(attributeData.AttributeClass, mefV2ExportAttribute)) + { + // Check if the export attribute has a type argument + if (attributeData.ConstructorArguments is [{ Kind: TypedConstantKind.Type, Value: INamedTypeSymbol exportedType }, ..]) + { + // Check if the exporting type implements or inherits from the exported type + if (!IsTypeCompatible(namedType, exportedType)) + { + context.ReportDiagnostic(Diagnostic.Create( + Descriptor, + location, + namedType.Name, + exportedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + } + } + } + } + + private static void AnalyzePropertyDeclaration(SymbolAnalysisContext context, INamedTypeSymbol? mefV1ExportAttribute, INamedTypeSymbol? mefV2ExportAttribute) + { + var property = (IPropertySymbol)context.Symbol; + + Location? location = property.Locations.FirstOrDefault(); + if (location is null) + { + // We won't have anywhere to publish a diagnostic anyway. + return; + } + + foreach (var attributeData in property.GetAttributes()) + { + // Check if this is an Export attribute + if (SymbolEqualityComparer.Default.Equals(attributeData.AttributeClass, mefV1ExportAttribute) || + SymbolEqualityComparer.Default.Equals(attributeData.AttributeClass, mefV2ExportAttribute)) + { + // Check if the export attribute has a type argument + if (attributeData.ConstructorArguments is [{ Kind: TypedConstantKind.Type, Value: INamedTypeSymbol exportedType }, ..]) + { + // Check if the property type is compatible with the exported type + if (!IsPropertyTypeCompatible(property.Type, exportedType)) + { + context.ReportDiagnostic(Diagnostic.Create( + Descriptor, + location, + property.Name, + exportedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + } + } + } + } + + private static bool IsTypeCompatible(INamedTypeSymbol implementingType, INamedTypeSymbol exportedType) + { + // If they're the same type, it's compatible + if (SymbolEqualityComparer.Default.Equals(implementingType, exportedType)) + { + return true; + } + + // Check if implementing type inherits from exported type (for classes) + if (exportedType.TypeKind == TypeKind.Class) + { + var currentType = implementingType.BaseType; + while (currentType != null) + { + if (SymbolEqualityComparer.Default.Equals(currentType, exportedType)) + { + return true; + } + + currentType = currentType.BaseType; + } + } + + // Check if implementing type implements exported interface + if (exportedType.TypeKind == TypeKind.Interface) + { + return implementingType.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, exportedType)); + } + + return false; + } + + private static bool IsPropertyTypeCompatible(ITypeSymbol propertyType, INamedTypeSymbol exportedType) + { + // If they're the same type, it's compatible + if (SymbolEqualityComparer.Default.Equals(propertyType, exportedType)) + { + return true; + } + + // If property type is a named type, use the same logic as for classes + if (propertyType is INamedTypeSymbol namedPropertyType) + { + return IsTypeCompatible(namedPropertyType, exportedType); + } + + return false; + } +} diff --git a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/VSMEF003ExportTypeMismatchAnalyzerTests.cs b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/VSMEF003ExportTypeMismatchAnalyzerTests.cs new file mode 100644 index 000000000..2ce7177e4 --- /dev/null +++ b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/VSMEF003ExportTypeMismatchAnalyzerTests.cs @@ -0,0 +1,359 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using VerifyCS = CSharpCodeFixVerifier; +using VerifyVB = VisualBasicCodeFixVerifier; + +public class VSMEF003ExportTypeMismatchAnalyzerTests +{ + [Fact] + public async Task ExportingClassItself_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + [Export(typeof(Test))] + class Test + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportingImplementedInterface_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + [Export(typeof(ITest))] + class Test : ITest + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportingBaseClass_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + class BaseTest + { + } + + [Export(typeof(BaseTest))] + class Test : BaseTest + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportingUnimplementedInterface_ProducesDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + [Export(typeof(ITest))] + class [|Test|] + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportingUnrelatedClass_ProducesDiagnostic() + { + string test = """ + using System.Composition; + + class OtherClass + { + } + + [Export(typeof(OtherClass))] + class [|Test|] + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportingUnimplementedInterface_ProducesDiagnostic_VB() + { + string test = """ + Imports System.Composition + + Interface ITest + End Interface + + + Class [|Test|] + End Class + """; + + await VerifyVB.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportWithoutTypeArgument_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + [Export] + class Test + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExportWithContractName_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + [Export("SomeContract")] + class Test + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task MefV1ExportingUnimplementedInterface_ProducesDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + interface ITest + { + } + + [Export(typeof(ITest))] + class [|Test|] + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task MefV1ExportingImplementedInterface_ProducesNoDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + interface ITest + { + } + + [Export(typeof(ITest))] + class Test : ITest + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task MultipleExportsWithMismatch_ProducesDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + interface IOther + { + } + + [Export(typeof(ITest))] + [Export(typeof(IOther))] + class [|Test|] : ITest + { + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task NonClassTypes_ProduceNoDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + class ITest + { + [Export(typeof(ITest))] + public ITest TestProperty => throw new System.NotImplementedException(); + + [Export("Method")] + public void TestMethod() => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task PropertyExportingMatchingType_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + class TestClass + { + [Export(typeof(ITest))] + public ITest TestProperty => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task PropertyExportingCompatibleType_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + class ConcreteTest : ITest + { + } + + class TestClass + { + [Export(typeof(ITest))] + public ConcreteTest TestProperty => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task PropertyExportingIncompatibleType_ProducesDiagnostic() + { + string test = """ + using System.Composition; + + interface ITest + { + } + + interface IOther + { + } + + class TestClass + { + [Export(typeof(ITest))] + public IOther [|TestProperty|] => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task PropertyExportingUnrelatedClass_ProducesDiagnostic() + { + string test = """ + using System.Composition; + + class TestType + { + } + + class OtherType + { + } + + class TestClass + { + [Export(typeof(TestType))] + public OtherType [|TestProperty|] => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task MefV1PropertyExportingIncompatibleType_ProducesDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + interface ITest + { + } + + interface IOther + { + } + + class TestClass + { + [Export(typeof(ITest))] + public IOther [|TestProperty|] => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task PropertyExportWithoutTypeArgument_ProducesNoDiagnostic() + { + string test = """ + using System.Composition; + + class TestClass + { + [Export] + public string TestProperty => throw new System.NotImplementedException(); + } + """; + + await VerifyCS.VerifyAnalyzerAsync(test); + } +}