Merge pull request #658 from drewnoakes/vsmef004-and-recomposition

VSMEF004 support for recomposition/PartNotDiscoverable

Author: AArnott

docfx/analyzers/VSMEF004.md

@@ -70,6 +70,34 @@ class Foo
 }
 ```
 
+### Option 4: Add [PartNotDiscoverable] for manually constructed parts
+
+If your part is intended to be manually constructed and inserted into the composition (e.g., for recomposition or mocking scenarios), you can mark it with `[PartNotDiscoverable]`:
+
+```cs
+[Export]
+[PartNotDiscoverable]
+class Foo
+{
+    public Foo(string parameter)
+    {
+        // This part will be manually constructed and composed
+    }
+}
+```
+
+This tells MEF not to discover and instantiate this part automatically. You must then manually create instances and add them to the composition container.
+
+## When to suppress warnings
+
+You can suppress this diagnostic if:
+
+1. **Manual construction scenarios**: Your part is intentionally constructed outside of MEF and inserted into the composition using methods like `CompositionContainer.ComposeParts()` or `CompositionContainer.SatisfyImportsOnce()`. Consider using `[PartNotDiscoverable]` instead of suppressing, as it makes the intent explicit.
+
+2. **Mocking/testing scenarios**: The type is used for testing purposes where you manually construct mock instances. Again, `[PartNotDiscoverable]` may be a better choice.
+
+3. **Very advanced recomposition scenarios**: You're using MEF v1's recomposition feature where parts can be dynamically added to or removed from the composition at runtime.
+
 ## Examples that trigger this rule
 
 ### Class-level export with non-default constructor

src/Microsoft.VisualStudio.Composition.Analyzers.CodeFixes/VSMEF004ExportWithoutImportingConstructorCodeFixProvider.cs

@@ -74,19 +74,27 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
         // Primary fix: Add [ImportingConstructor] attribute
         var addAttributeAction = CodeAction.Create(
-            title: "Add [ImportingConstructor] attribute",
+            title: Strings.VSMEF004_CodeFix_AddImportingConstructorAttribute,
             createChangedDocument: c => AddImportingConstructorAttributeAsync(context.Document, constructorDeclaration, mefVersion, c),
             equivalenceKey: "AddImportingConstructorAttribute");
 
         context.RegisterCodeFix(addAttributeAction, diagnostic);
 
         // Secondary fix: Add parameterless constructor
         var addConstructorAction = CodeAction.Create(
-            title: "Add parameterless constructor",
+            title: Strings.VSMEF004_CodeFix_AddParameterlessConstructor,
             createChangedDocument: c => AddParameterlessConstructorAsync(context.Document, classDeclaration, c),
             equivalenceKey: "AddParameterlessConstructor");
 
         context.RegisterCodeFix(addConstructorAction, diagnostic);
+
+        // Tertiary fix: Add [PartNotDiscoverable] attribute (for manually constructed parts)
+        var addPartNotDiscoverableAction = CodeAction.Create(
+            title: Strings.VSMEF004_CodeFix_AddPartNotDiscoverableAttribute,
+            createChangedDocument: c => AddPartNotDiscoverableAttributeAsync(context.Document, classDeclaration, mefVersion, c),
+            equivalenceKey: "AddPartNotDiscoverableAttribute");
+
+        context.RegisterCodeFix(addPartNotDiscoverableAction, diagnostic);
     }
 
     private static async Task<Document> AddImportingConstructorAttributeAsync(
@@ -173,6 +181,43 @@ private static async Task<Document> AddParameterlessConstructorAsync(
         return newDocument;
     }
 
+    private static async Task<Document> AddPartNotDiscoverableAttributeAsync(
+        Document document,
+        ClassDeclarationSyntax classDeclaration,
+        MefVersion mefVersion,
+        CancellationToken cancellationToken)
+    {
+        SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+        if (root is null)
+        {
+            return document;
+        }
+
+        // Create the PartNotDiscoverable attribute with proper annotations for simplification
+        string attributeName = mefVersion == MefVersion.V1
+            ? "System.ComponentModel.Composition.PartNotDiscoverable"
+            : "System.Composition.PartNotDiscoverable";
+
+        AttributeSyntax attribute = SyntaxFactory.Attribute(
+            SyntaxFactory.ParseName(attributeName)
+                .WithAdditionalAnnotations(Simplifier.AddImportsAnnotation, Simplifier.Annotation));
+        AttributeListSyntax attributeList = SyntaxFactory.AttributeList(SyntaxFactory.SingletonSeparatedList(attribute));
+
+        // Add the attribute to the class
+        ClassDeclarationSyntax newClass = classDeclaration.WithAttributeLists(
+            classDeclaration.AttributeLists.Add(attributeList));
+
+        SyntaxNode newRoot = root.ReplaceNode(classDeclaration, newClass);
+
+        // Apply simplification and formatting
+        document = document.WithSyntaxRoot(newRoot);
+        document = await ImportAdder.AddImportsAsync(document, Simplifier.AddImportsAnnotation, cancellationToken: cancellationToken).ConfigureAwait(false);
+        document = await Simplifier.ReduceAsync(document, cancellationToken: cancellationToken).ConfigureAwait(false);
+        document = await Formatter.FormatAsync(document, Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false);
+
+        return document;
+    }
+
     private static MefVersion DetermineMefVersion(ClassDeclarationSyntax classDeclaration, SemanticModel semanticModel)
     {
         // Check the class and its members for MEF attributes to determine which version to use

src/Microsoft.VisualStudio.Composition.Analyzers.CodeFixes/VSMEF006ImportNullabilityCodeFixProvider.cs

@@ -151,15 +151,15 @@ private static void RegisterFixes(CodeFixContext context, SyntaxNode root, Diagn
             // Fix 1: Add AllowDefault = true
             context.RegisterCodeFix(
                 CodeAction.Create(
-                    title: "Add AllowDefault = true",
+                    title: Strings.VSMEF006_CodeFix_AddAllowDefault,
                     createChangedDocument: cancellationToken => AddAllowDefaultAsync(context.Document, root, targetNode, attributeLists, cancellationToken),
                     equivalenceKey: "AddAllowDefault"),
                 diagnostic);
 
             // Fix 2: Make type non-nullable
             context.RegisterCodeFix(
                 CodeAction.Create(
-                    title: "Make type non-nullable",
+                    title: Strings.VSMEF006_CodeFix_MakeTypeNonNullable,
                     createChangedDocument: cancellationToken => MakeTypeNonNullableAsync(context.Document, root, targetNode, typeSyntax, cancellationToken),
                     equivalenceKey: "MakeNonNullable"),
                 diagnostic);
@@ -170,15 +170,15 @@ private static void RegisterFixes(CodeFixContext context, SyntaxNode root, Diagn
             // Fix 1: Make type nullable
             context.RegisterCodeFix(
                 CodeAction.Create(
-                    title: "Make type nullable",
+                    title: Strings.VSMEF006_CodeFix_MakeTypeNullable,
                     createChangedDocument: cancellationToken => MakeTypeNullableAsync(context.Document, root, targetNode, typeSyntax, cancellationToken),
                     equivalenceKey: "MakeNullable"),
                 diagnostic);
 
             // Fix 2: Remove AllowDefault = true
             context.RegisterCodeFix(
                 CodeAction.Create(
-                    title: "Remove AllowDefault = true",
+                    title: Strings.VSMEF006_CodeFix_RemoveAllowDefault,
                     createChangedDocument: cancellationToken => RemoveAllowDefaultAsync(context.Document, root, targetNode, attributeLists, cancellationToken),
                     equivalenceKey: "RemoveAllowDefault"),
                 diagnostic);

src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx

@@ -122,15 +122,18 @@
   </resheader>
   <data name="VSMEF001_MessageFormat" xml:space="preserve">
     <value>A property with an [ImportAttribute] must define a setter so the export can be set on the property.</value>
+    <comment>{Locked="[ImportAttribute]"} The attribute name in brackets should not be localized.</comment>
   </data>
   <data name="VSMEF001_Title" xml:space="preserve">
     <value>Importing property must have setter</value>
   </data>
   <data name="VSMEF002_MessageFormat" xml:space="preserve">
     <value>The type "{0}" contains a mix of attributes from MEFv1 and MEFv2. Consolidate to just one variety of attributes.</value>
+    <comment>{Locked="MEFv1"} {Locked="MEFv2"} The framework version names should not be localized.</comment>
   </data>
   <data name="VSMEF002_Title" xml:space="preserve">
     <value>Avoid mixing MEF attribute varieties</value>
+    <comment>{Locked="MEF"} The framework name should not be localized.</comment>
   </data>
   <data name="VSMEF003_MessageFormat" xml:space="preserve">
     <value>The type "{0}" does not implement the exported type "{1}". This may be an authoring mistake.</value>
@@ -140,29 +143,59 @@
   </data>
   <data name="VSMEF004_MessageFormat" xml:space="preserve">
     <value>The type "{0}" defines exports but has non-default constructors that are not annotated with [ImportingConstructor]. Add a parameterless constructor, or annotate a constructor with [ImportingConstructor].</value>
+    <comment>{Locked="[ImportingConstructor]"} The attribute name in brackets should not be localized.</comment>
   </data>
   <data name="VSMEF004_Title" xml:space="preserve">
     <value>Exported type missing importing constructor</value>
   </data>
   <data name="VSMEF005_MessageFormat" xml:space="preserve">
     <value>The type "{0}" has multiple constructors annotated with [ImportingConstructor]. Only one constructor should be marked as importing.</value>
+    <comment>{Locked="[ImportingConstructor]"} The attribute name in brackets should not be localized.</comment>
   </data>
   <data name="VSMEF005_Title" xml:space="preserve">
     <value>Multiple importing constructors</value>
   </data>
   <data name="VSMEF006_AllowDefaultWithoutNullable_MessageFormat" xml:space="preserve">
     <value>The import "{0}" has AllowDefault = true but is not nullable. Consider making the type nullable or removing AllowDefault.</value>
+    <comment>{Locked="AllowDefault"} {Locked="true"} The property name and value should not be localized.</comment>
   </data>
   <data name="VSMEF006_NullableWithoutAllowDefault_MessageFormat" xml:space="preserve">
     <value>The import "{0}" is nullable but does not have AllowDefault = true. Consider adding AllowDefault = true or making the type non-nullable (and either add a 'required' modifier or an '= null!' initializer).</value>
+    <comment>{Locked="AllowDefault"} {Locked="true"} {Locked="required"} {Locked="= null!"} The property name, value, and C# keywords/syntax should not be localized.</comment>
   </data>
   <data name="VSMEF006_Title" xml:space="preserve">
     <value>Import nullability and AllowDefault mismatch</value>
+    <comment>{Locked="AllowDefault"} The property name should not be localized.</comment>
   </data>
   <data name="VSMEF007_MessageFormat" xml:space="preserve">
     <value>The type "{0}" imports the same contract "{1}" multiple times. Each contract should only be imported once per type.</value>
   </data>
   <data name="VSMEF007_Title" xml:space="preserve">
     <value>Duplicate import contract</value>
   </data>
+  <data name="VSMEF004_CodeFix_AddImportingConstructorAttribute" xml:space="preserve">
+    <value>Add [ImportingConstructor] attribute</value>
+    <comment>{Locked="[ImportingConstructor]"} The attribute name in brackets should not be localized.</comment>
+  </data>
+  <data name="VSMEF004_CodeFix_AddParameterlessConstructor" xml:space="preserve">
+    <value>Add parameterless constructor</value>
+  </data>
+  <data name="VSMEF004_CodeFix_AddPartNotDiscoverableAttribute" xml:space="preserve">
+    <value>Add [PartNotDiscoverable] attribute</value>
+    <comment>{Locked="[PartNotDiscoverable]"} The attribute name in brackets should not be localized.</comment>
+  </data>
+  <data name="VSMEF006_CodeFix_AddAllowDefault" xml:space="preserve">
+    <value>Add AllowDefault = true</value>
+    <comment>{Locked="AllowDefault"} {Locked="true"} The property name and value should not be localized.</comment>
+  </data>
+  <data name="VSMEF006_CodeFix_MakeTypeNonNullable" xml:space="preserve">
+    <value>Make type non-nullable</value>
+  </data>
+  <data name="VSMEF006_CodeFix_MakeTypeNullable" xml:space="preserve">
+    <value>Make type nullable</value>
+  </data>
+  <data name="VSMEF006_CodeFix_RemoveAllowDefault" xml:space="preserve">
+    <value>Remove AllowDefault = true</value>
+    <comment>{Locked="AllowDefault"} {Locked="true"} The property name and value should not be localized.</comment>
+  </data>
 </root>

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

@@ -92,6 +92,26 @@ internal static bool IsImportingConstructorAttribute(INamedTypeSymbol? attribute
                IsAttributeOfType(attributeType, "ImportingConstructorAttribute", MefV2AttributeNamespace.AsSpan());
     }
 
+    /// <summary>
+    /// Determines whether the specified attribute type is a MEF PartNotDiscoverable attribute.
+    /// </summary>
+    /// <param name="attributeType">The attribute type to check.</param>
+    /// <returns><see langword="true"/> if the attribute type is a PartNotDiscoverable attribute; otherwise, <see langword="false"/>.</returns>
+    /// <remarks>
+    /// This method checks for both MEF v1 and MEF v2 PartNotDiscoverable attributes.
+    /// </remarks>
+    internal static bool IsPartNotDiscoverableAttribute(INamedTypeSymbol? attributeType)
+    {
+        if (attributeType is null)
+        {
+            return false;
+        }
+
+        // Check if it's a PartNotDiscoverable attribute (not typically subclassed, but check inheritance for consistency)
+        return IsAttributeOfType(attributeType, "PartNotDiscoverableAttribute", MefV1AttributeNamespace.AsSpan()) ||
+               IsAttributeOfType(attributeType, "PartNotDiscoverableAttribute", MefV2AttributeNamespace.AsSpan());
+    }
+
     /// <summary>
     /// Determines whether the specified attribute type matches the expected type and namespace, including inheritance hierarchy.
     /// </summary>
@@ -215,6 +235,24 @@ internal static bool HasImportingConstructorAttribute(IMethodSymbol constructor)
         return false;
     }
 
+    /// <summary>
+    /// Determines whether the specified type has the PartNotDiscoverable attribute.
+    /// </summary>
+    /// <param name="symbol">The type symbol to check.</param>
+    /// <returns><see langword="true"/> if the type has the PartNotDiscoverable attribute; otherwise, <see langword="false"/>.</returns>
+    internal static bool HasPartNotDiscoverableAttribute(INamedTypeSymbol symbol)
+    {
+        foreach (AttributeData attribute in symbol.GetAttributes())
+        {
+            if (IsPartNotDiscoverableAttribute(attribute.AttributeClass))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /// <summary>
     /// Determines whether the specified namespace matches the expected namespace components.
     /// </summary>

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

@@ -66,6 +66,12 @@ static void AnalyzeSymbol(SymbolAnalysisContext context)
                 return;
             }
 
+            // Skip types marked with [PartNotDiscoverable] as they are intended for manual construction
+            if (Utils.HasPartNotDiscoverableAttribute(symbol))
+            {
+                return;
+            }
+
             // Get all constructors
             var constructors = symbol.Constructors.Where(c => !c.IsStatic && !c.IsImplicitlyDeclared).ToList();