Fix VSMEF007

src/Microsoft.VisualStudio.Composition.Analyzers/Utils.cs

@@ -311,6 +311,12 @@ internal static bool GetAllowDefaultValue(AttributeData importAttribute)
 
         return null;
     }
+
+    internal static void Deconstruct<TKey, TValue>(this IGrouping<TKey, TValue> grouping, out TKey key, out IEnumerable<TValue> values)
+    {
+        key = grouping.Key;
+        values = grouping;
+    }
 }
 
 /// <summary>

src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF007DuplicateImportAnalyzer.cs

@@ -66,7 +66,7 @@ private static void AnalyzeType(SymbolAnalysisContext context)
             return;
         }
 
-        var importContracts = new List<ImportContract>();
+        var imports = new List<Import>();
 
         // Collect all imports from properties
         foreach (ISymbol member in namedType.GetMembers())
@@ -75,11 +75,11 @@ private static void AnalyzeType(SymbolAnalysisContext context)
             {
                 AttributeData? importAttribute = Utils.GetImportAttribute(property.GetAttributes());
 
-                string? contract = GetImportContract(importAttribute, property.Type);
-
-                if (contract is not null)
+                if (importAttribute is not null)
                 {
-                    importContracts.Add(new ImportContract(contract, property.Name, property.Locations[0]));
+                    Contract contract = GetImportContract(importAttribute, property.Type);
+
+                    imports.Add(new Import(contract, property.Name, property.Locations[0]));
                 }
             }
         }
@@ -97,71 +97,82 @@ private static void AnalyzeType(SymbolAnalysisContext context)
                     {
                         AttributeData? importAttribute = Utils.GetImportAttribute(parameter.GetAttributes());
 
-                        string? contract = GetImportContract(importAttribute, parameter.Type);
+                        Contract contract = GetImportContract(importAttribute, parameter.Type);
 
-                        if (contract is not null)
-                        {
-                            importContracts.Add(new ImportContract(contract, parameter.Name, parameter.Locations[0]));
-                        }
+                        imports.Add(new Import(contract, parameter.Name, parameter.Locations[0]));
                     }
                 }
             }
         }
 
         // Find duplicates
-        IEnumerable<IGrouping<string, ImportContract>> contractGroups = importContracts.GroupBy(ic => ic.Contract).Where(g => g.Count() > 1);
+        IEnumerable<IGrouping<Contract, Import>> duplicateImportsByContract = imports.GroupBy(ic => ic.Contract).Where(g => g.Count() > 1);
 
-        foreach (IGrouping<string, ImportContract>? group in contractGroups)
+        foreach ((Contract contract, IEnumerable<Import> duplicateImports) in duplicateImportsByContract)
         {
-            foreach (ImportContract? import in group)
+            foreach (Import import in duplicateImports)
             {
                 context.ReportDiagnostic(Diagnostic.Create(
                     Descriptor,
                     import.Location,
                     namedType.Name,
-                    import.Contract));
+                    contract));
             }
         }
     }
 
-    private static string? GetImportContract(AttributeData? importAttribute, ITypeSymbol importType)
+    private static Contract GetImportContract(AttributeData? importAttribute, ITypeSymbol importType)
     {
-        // Note that the actual contract name used by MEF is more complex than this. See ContractNameServices
-        // for the full logic. This approximation suffices for catching duplicates, for the purposes of this analyzer.
+        string? explicitContractName = null;
+        ITypeSymbol? explicitContractType = null;
+
         if (importAttribute is not null)
         {
-            // Check for explicit contract name or type in constructor arguments
-            if (importAttribute.ConstructorArguments is [TypedConstant firstArg, ..])
+            // Check for explicit contract name or type in constructor arguments.
+            // Uses patterns from both System.Composition.ImportAttribute and System.ComponentModel.Composition.ImportAttribute.
+            // An empty or null contract name means "no explicit name".
+            (explicitContractName, explicitContractType) = importAttribute.ConstructorArguments switch
             {
-                if (firstArg.Value is string { Length: not 0 } contractName)
-                {
-                    return contractName;
-                }
-
-                if (firstArg.Value is INamedTypeSymbol contractType)
-                {
-                    return contractType.ToDisplayString();
-                }
-            }
+                [TypedConstant { Value: string { Length: not 0 } contractName }] => (contractName, null),
+                [TypedConstant { Value: INamedTypeSymbol contractType }] => (null, contractType),
+                [TypedConstant { Value: string { Length: not 0 } contractName }, TypedConstant { Value: INamedTypeSymbol contractType }] => (contractName, contractType),
+                [TypedConstant { Value: null or string { Length: 0 } }, TypedConstant { Value: INamedTypeSymbol contractType }] => (null, contractType),
+                _ => (null, null),
+            };
 
             // Check for contract name in named arguments
-            KeyValuePair<string, TypedConstant> contractNameArg = importAttribute.NamedArguments.FirstOrDefault(arg => arg.Key == "ContractName");
-            if (contractNameArg.Key is not null && contractNameArg.Value.Value is string namedContractName && !string.IsNullOrEmpty(namedContractName))
+            TypedConstant? nameArg = importAttribute.NamedArguments.FirstOrDefault(arg => arg.Key == "ContractName").Value;
+            if (nameArg?.Value is string { Length: not 0 } namedContractName)
             {
-                return namedContractName;
+                explicitContractName = namedContractName;
             }
 
             // Check for contract type in named arguments
-            KeyValuePair<string, TypedConstant> contractTypeArg = importAttribute.NamedArguments.FirstOrDefault(arg => arg.Key == "ContractType");
-            if (contractTypeArg.Key is not null && contractTypeArg.Value.Value is INamedTypeSymbol namedContractType)
+            TypedConstant? typeArg = importAttribute.NamedArguments.FirstOrDefault(arg => arg.Key == "ContractType").Value;
+            if (typeArg?.Value is INamedTypeSymbol namedContractType)
             {
-                return namedContractType.ToDisplayString();
+                explicitContractType = namedContractType;
             }
         }
 
-        // Default contract is the type name
-        return importType.ToDisplayString();
+        // Determine the base type for defaulting contract name and type.
+        // If contract type is explicitly specified, use it; otherwise use the import parameter type.
+        ITypeSymbol type = explicitContractType ?? importType;
+        string typeName = type.ToDisplayString();
+
+        // Contract name: use explicit name if provided, otherwise default to type name.
+        string name = explicitContractName ?? typeName;
+
+        return new Contract(typeName, name);
+    }
+
+    // A MEF contract consists of both a contract name and a contract type.
+    // Both must match for two imports to be considered duplicates.
+    // See: https://learn.microsoft.com/en-us/dotnet/framework/mef/attributed-programming-model-overview-mef#import-and-export-basics
+    private readonly record struct Contract(string Type, string? Name)
+    {
+        public override string ToString() => this.Name is null ? this.Type : $"{this.Type} (\"{this.Name}\")";
     }
 
-    private record ImportContract(string Contract, string MemberName, Location Location);
+    private record Import(Contract Contract, string MemberName, Location Location);
 }

test/Microsoft.VisualStudio.Composition.Analyzers.Tests/VSMEF001PropertyMustHaveSetterTests.cs

@@ -1,9 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
-using System.Threading.Tasks;
-using Xunit;
 using VerifyCS = CSharpCodeFixVerifier<Microsoft.VisualStudio.Composition.Analyzers.VSMEF001PropertyMustHaveSetter, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
 using VerifyVB = VisualBasicCodeFixVerifier<Microsoft.VisualStudio.Composition.Analyzers.VSMEF001PropertyMustHaveSetter, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
 

test/Microsoft.VisualStudio.Composition.Analyzers.Tests/VSMEF007DuplicateImportAnalyzerTests.cs

@@ -1,11 +1,32 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using Microsoft.VisualStudio.Composition.Analyzers;
 using VerifyCS = CSharpCodeFixVerifier<Microsoft.VisualStudio.Composition.Analyzers.VSMEF007DuplicateImportAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
 
 public class VSMEF007DuplicateImportAnalyzerTests
 {
+    [Fact]
+    public async Task PlainClassWithoutMefAttributes_NoWarning()
+    {
+        string test = """
+            class Foo
+            {
+                public string Value1 { get; set; }
+                public string Value2 { get; set; }
+                private int field1;
+                private int field2;
+
+                public Foo(string value1, string value2)
+                {
+                    Value1 = value1;
+                    Value2 = value2;
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
     [Fact]
     public async Task ClassWithNoImports_NoWarning()
     {
@@ -503,4 +524,136 @@ public Foo(string implicitString, [Import("CustomStringContract")] string explic
 
         await VerifyCS.VerifyAnalyzerAsync(test);
     }
+
+    [Fact]
+    public async Task ImportingConstructorWithDifferentContractNameAndType_NoWarning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            namespace Foo
+            {
+                class A { }
+            }
+
+            [Export]
+            class Bar
+            {
+                [ImportingConstructor]
+                public Bar([Import("Foo.A")] string s, Foo.A a)
+                {
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task ImportingConstructorWithSameContractNameDifferentTypes_NoWarning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            [Export]
+            class Foo
+            {
+                [ImportingConstructor]
+                public Foo([Import("MyContract")] string value1, [Import("MyContract")] int value2)
+                {
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task ImportingConstructorWithSameTypeDifferentContractNames_NoWarning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            [Export]
+            class Foo
+            {
+                [ImportingConstructor]
+                public Foo([Import("Contract1")] string value1, [Import("Contract2")] string value2)
+                {
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task ImportWithExplicitContractNameAndTypeMatching_Warning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            namespace MyNamespace
+            {
+                class MyType { }
+            }
+
+            [Export]
+            class Foo
+            {
+                [ImportingConstructor]
+                public Foo([Import("MyNamespace.MyType", typeof(MyNamespace.MyType))] MyNamespace.MyType {|VSMEF007:value1|}, MyNamespace.MyType {|VSMEF007:value2|})
+                {
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task PropertyImportsWithDifferentContractNameButSameType_NoWarning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            namespace MyNamespace
+            {
+                class MyType { }
+            }
+
+            [Export]
+            class Foo
+            {
+                [Import("CustomContract")]
+                public MyNamespace.MyType Value1 { get; set; }
+
+                [Import]
+                public MyNamespace.MyType Value2 { get; set; }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+
+    [Fact]
+    public async Task ImportWithTypeofContractTypeButDifferentName_NoWarning()
+    {
+        string test = """
+            using System.ComponentModel.Composition;
+
+            interface IService { }
+
+            [Export]
+            class Foo
+            {
+                [ImportingConstructor]
+                public Foo([Import("CustomName", typeof(IService))] object service1, [Import(typeof(IService))] object service2)
+                {
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
 }