More MEF analyzers and code fixes #659

drewnoakes · 2025-12-08T10:45:30Z

This change adds some new MEF analyzers:

VSMEF008 ensures a MEF import's declared type is assignable to the associated property/parameter
VSMEF009 ensures [ImportMany] is used on an appropriate receiving collection type
VSMEF010 is similar to VSMEF009 but for constructor parameters (which have subtly different rules)
VSMEF011 prohibits use of both Import and ImportMany on a single import
VSMEF012 allows enforcing only a single version of the MEF attributes via .editorconfig

There are two code fixes as well:

VSMEF011 code fix removes whichever of Import or ImportMany does not apply, based on the target type
VSMEF012 code fix migrates from MEFv1 to MEFv2 attributes

This change adds some new MEF analyzers: - VSMEF008 ensures a MEF import's declared type is assignable to the associated property/parameter - VSMEF009 ensures `[ImportMany]` is used on an appropriate receiving collection type - VSMEF010 is similar to VSMEF009 but for constructor parameters (which have subtly different rules) - VSMEF011 prohibits use of both `Import` and `ImportMany` on a single import - VSMEF012 allows enforcing only a single version of the MEF attributes via `.editorconfig` There are two code fixes as well: - VSMEF011 code fix removes whichever of `Import` or `ImportMany` does not apply, based on the target type - VSMEF012 code fix migrates from MEFv1 to MEFv2 attributes

AArnott · 2025-12-09T15:49:35Z

VSMEF011 prohibits use of both Import and ImportMany on a single import

I can't think of a time when two Imports would be valid either. Does any rule prohibit that?

drewnoakes · 2025-12-09T22:29:32Z

VSMEF011 prohibits use of both Import and ImportMany on a single import

I can't think of a time when two Imports would be valid either. Does any rule prohibit that?

Looks like both attributes have AllowMultiple = false, so the compiler should enforce that.

AArnott · 2025-12-09T22:35:55Z

The rest of the description looks good. Is the code ready for review?

drewnoakes · 2025-12-09T23:08:56Z

Yes I think so, if you don't have any concerns with the general goals of these analyzers.

Copilot

Pull request overview

This PR adds five new MEF analyzers and two code fix providers to help developers write correct MEF code:

Summary: The PR introduces analyzers to validate MEF import declarations, ensuring type safety and proper usage of Import/ImportMany attributes, plus a configurable analyzer to enforce MEF version consistency.

Key Changes:

VSMEF008: Validates import contract types are assignable to member types
VSMEF009: Ensures ImportMany is used on appropriate collection types
VSMEF010: Validates ImportMany constructor parameter collection types (MEFv1 only)
VSMEF011: Detects conflicting Import and ImportMany on same member with code fix
VSMEF012: Configurable analyzer to enforce single MEF version with migration code fix

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF008ImportContractTypeMismatchAnalyzer.cs	Analyzer to detect contract type mismatches in Import attributes
src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF009ImportManyMemberCollectionTypeAnalyzer.cs	Analyzer to validate ImportMany collection types on properties/fields with comprehensive pre-initialization detection
src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF010ImportManyParameterCollectionTypeAnalyzer.cs	Analyzer for MEFv1 constructor parameter collection type restrictions
src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF011BothImportAndImportManyAnalyzer.cs	Analyzer to detect mutually exclusive Import/ImportMany usage
src/Microsoft.VisualStudio.Composition.Analyzers/VSMEF012DisallowMefAttributeVersionAnalyzer.cs	Configurable analyzer to enforce MEF version consistency via .editorconfig
src/Microsoft.VisualStudio.Composition.Analyzers.CodeFixes/VSMEF011RemoveDuplicateImportCodeFixProvider.cs	Code fix to remove duplicate Import/ImportMany based on type
src/Microsoft.VisualStudio.Composition.Analyzers.CodeFixes/VSMEF012MigrateToMefV2CodeFixProvider.cs	Code fix to migrate MEFv1 attributes to MEFv2
test/.../VSMEF008ImportContractTypeMismatchAnalyzerTests.cs	Comprehensive test coverage for VSMEF008
test/.../VSMEF009ImportManyMemberCollectionTypeAnalyzerTests.cs	Comprehensive test coverage for VSMEF009
test/.../VSMEF010ImportManyParameterCollectionTypeAnalyzerTests.cs	Comprehensive test coverage for VSMEF010
test/.../VSMEF011BothImportAndImportManyAnalyzerTests.cs	Comprehensive test coverage for VSMEF011 including code fixes
test/.../VSMEF012DisallowMefAttributeVersionAnalyzerTests.cs	Comprehensive test coverage for VSMEF012 including code fixes
test/.../Helpers/CSharpCodeFixVerifier`2.cs	Helper methods to support .editorconfig in tests
src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx	Resource strings for new diagnostics
docfx/analyzers/*.md	Documentation for all new analyzers
src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md	Release notes for new analyzers

Copilot · 2025-12-12T15:13:41Z

src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md

+VSMEF009 | Usage | Error | ImportMany on non-collection type
+VSMEF010 | Usage | Error | ImportMany with unsupported collection type in constructor
+VSMEF011 | Usage | Error | Both Import and ImportMany applied to same member
+VSMEF012 | Usage | Disabled | Disallow MEF attribute version


The diagnostic severity for VSMEF012 in the analyzer release notes is marked as "Disabled" but in the analyzer code it's marked as "Warning" with isEnabledByDefault: false. These should be consistent. The AnalyzerReleases.Unshipped.md file should use "Warning" as the severity since that's what the descriptor specifies, even though it's disabled by default.

Suggested change

VSMEF012 | Usage | Disabled | Disallow MEF attribute version

VSMEF012 | Usage | Warning | Disallow MEF attribute version

Copilot · 2025-12-12T15:13:41Z

...crosoft.VisualStudio.Composition.Analyzers/VSMEF009ImportManyMemberCollectionTypeAnalyzer.cs

+    private static bool ConstructorAssignsProperty(IMethodSymbol constructor, IPropertySymbol property, Compilation compilation)
+    {
+        foreach (SyntaxReference syntaxRef in constructor.DeclaringSyntaxReferences)
+        {
+            SyntaxNode syntax = syntaxRef.GetSyntax();
+
+            // We need the semantic model to resolve IOperation from syntax nodes.
+            // This is necessary to determine if an assignment targets our specific property.
+#pragma warning disable RS1030 // Do not invoke Compilation.GetSemanticModel() method within a diagnostic analyzer
+            SemanticModel? model = compilation.GetSemanticModel(syntax.SyntaxTree);
+#pragma warning restore RS1030
+


The code is using GetSemanticModel within a diagnostic analyzer which triggers RS1030 warning. While the pragma suppression is present, this is a performance anti-pattern. The semantic model should be obtained from the context instead. Consider refactoring to use the compilation context's semantic model cache or refactor to avoid needing GetSemanticModel here.

Suggested change

private static bool ConstructorAssignsProperty(IMethodSymbol constructor, IPropertySymbol property, Compilation compilation)

{

foreach (SyntaxReference syntaxRef in constructor.DeclaringSyntaxReferences)

{

SyntaxNode syntax = syntaxRef.GetSyntax();

// We need the semantic model to resolve IOperation from syntax nodes.

// This is necessary to determine if an assignment targets our specific property.

#pragma warning disable RS1030 // Do not invoke Compilation.GetSemanticModel() method within a diagnostic analyzer

SemanticModel? model = compilation.GetSemanticModel(syntax.SyntaxTree);

#pragma warning restore RS1030

private static bool ConstructorAssignsProperty(IMethodSymbol constructor, IPropertySymbol property, SemanticModel model)

{

foreach (SyntaxReference syntaxRef in constructor.DeclaringSyntaxReferences)

{

SyntaxNode syntax = syntaxRef.GetSyntax();

Copilot · 2025-12-12T15:13:42Z

docfx/analyzers/VSMEF012.md

+## Notes
+
+- This analyzer is disabled by default. You must explicitly enable it and configure the allowed version.
+- This analyzer is distinct from VSMEF002, which warns about *mixing* MEF versions on the same type. VSMEF013 disallows a version entirely.


The documentation references "VSMEF002" and "VSMEF013" but VSMEF013 doesn't exist. This should refer to "VSMEF012" instead. The sentence should read: "This analyzer is distinct from VSMEF002, which warns about mixing MEF versions on the same type. VSMEF012 disallows a version entirely."

Suggested change

- This analyzer is distinct from VSMEF002, which warns about *mixing* MEF versions on the same type. VSMEF013 disallows a version entirely.

- This analyzer is distinct from VSMEF002, which warns about *mixing* MEF versions on the same type. VSMEF012 disallows a version entirely.

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

...crosoft.VisualStudio.Composition.Analyzers/VSMEF009ImportManyMemberCollectionTypeAnalyzer.cs

...VisualStudio.Composition.Analyzers.CodeFixes/VSMEF011RemoveDuplicateImportCodeFixProvider.cs

Copilot · 2025-12-12T15:13:46Z

...crosoft.VisualStudio.Composition.Analyzers/VSMEF009ImportManyMemberCollectionTypeAnalyzer.cs

+                if (operation is ISimpleAssignmentOperation assignment)
+                {
+                    // Check if the target is our property
+                    if (assignment.Target is IPropertyReferenceOperation propRef &&
+                        SymbolEqualityComparer.Default.Equals(propRef.Property, property))
+                    {
+                        // Check if it's assigning to 'this'
+                        if (propRef.Instance is IInstanceReferenceOperation instanceRef &&
+                            instanceRef.ReferenceKind == InstanceReferenceKind.ContainingTypeInstance)
+                        {
+                            return true;
+                        }
+                    }
+                }


These 'if' statements can be combined.

Copilot · 2025-12-12T15:13:47Z

...crosoft.VisualStudio.Composition.Analyzers/VSMEF009ImportManyMemberCollectionTypeAnalyzer.cs

+                    if (assignment.Target is IPropertyReferenceOperation propRef &&
+                        SymbolEqualityComparer.Default.Equals(propRef.Property, property))
+                    {
+                        // Check if it's assigning to 'this'
+                        if (propRef.Instance is IInstanceReferenceOperation instanceRef &&
+                            instanceRef.ReferenceKind == InstanceReferenceKind.ContainingTypeInstance)
+                        {
+                            return true;
+                        }
+                    }


These 'if' statements can be combined.

AArnott

This looks good. Just a few comments, please.

AArnott

Trying again to post my comments...

AArnott · 2025-12-12T15:32:57Z

...VisualStudio.Composition.Analyzers.Tests/VSMEF012DisallowMefAttributeVersionAnalyzerTests.cs

+    private const string EditorConfigDisallowV2 = """
+
+        root = true
+
+        [*.cs]
+        dotnet_diagnostic.VSMEF012.severity = warning
+        dotnet_diagnostic.VSMEF012.allowed_mef_version = V1
+
+        """;


What happens if an unexpected value is assigned to this setting? (e.g. "garbage")
Will the analyzer throw an unhandled exception or report a diagnostic?

AArnott · 2025-12-12T15:33:34Z

...ualStudio.Composition.Analyzers.Tests/VSMEF009ImportManyMemberCollectionTypeAnalyzerTests.cs

+    [Fact]
+    public async Task PlainClassWithoutMefAttributes_NoWarning()
+    {
+        string test = """
+            class Foo
+            {
+                public string Value { get; set; }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(test);
+    }
+


💡 For common "success" cases like this where you'd really like to assert that none of the analyzers misfire on it, I suggest you collect them in a single multi-analyzer test class like I do over at vs-threading.

AArnott · 2025-12-12T15:34:09Z

docfx/analyzers/VSMEF012.md

+# Allow only MEFv1 attributes
+dotnet_diagnostic.VSMEF012.allowed_mef_version = V1
+
+# Or allow only MEFv2 attributes
+dotnet_diagnostic.VSMEF012.allowed_mef_version = V2


While we frequently toss around "v1" and "v2" terms, or even ".NET MEF" and "NuGet MEF", both of these are slang terms that may not have a clear meaning to customers.

Can we accept the namespace here instead?
(FWIW This would be a good case for asking Copilot to address since it's scattered everywhere in your PR and would be mechanical)

AArnott · 2025-12-12T15:34:43Z

docfx/analyzers/VSMEF012.md

+dotnet_diagnostic.VSMEF012.allowed_mef_version = V2
+```
+
+## Examples of violations


Will this catch cases of attributes from arbitrary namespaces that derive from MEF attributes from the official namespaces? That's the most subtle and therefore the hardest case to avoid without an analyzer.

AArnott · 2025-12-12T15:35:13Z

docfx/analyzers/VSMEF012.md

+
+| MEFv1 Attribute | MEFv2 Attribute |
+|-----------------|-----------------|
+| <xref:System.ComponentModel.Composition.ExportAttribute?displayProperty=fullName> | <xref:System.Composition.ExportAttribute?displayProperty=fullName> |


When referencing APIs, please use the <xref:> syntax, which turns them into links to the docs on those APIs, and ensures that you don't have any typos.
This works for namespaces too.

I've already made the change here and for the namespaces at the top of the file.

drewnoakes · 2025-12-19T02:19:49Z

I would like to add another analyzer that ensures any [Export(typeof(T))] accompanies a type that actually implements T. I just found an example of this in CPS where that was not the case.

drewnoakes added 2 commits December 8, 2025 21:49

Localize code fix titles

630582a

drewnoakes force-pushed the more-analyzers branch from 8a0560b to 630582a Compare December 8, 2025 10:52

drewnoakes marked this pull request as ready for review December 9, 2025 23:08

AArnott requested a review from Copilot December 12, 2025 15:05

Copilot started reviewing on behalf of AArnott December 12, 2025 15:05 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Add links in a doc

337ffae

AArnott reviewed Dec 12, 2025

View reviewed changes

	VSMEF012 \| Usage \| Disabled \| Disallow MEF attribute version
	VSMEF012 \| Usage \| Warning \| Disallow MEF attribute version

	- This analyzer is distinct from VSMEF002, which warns about mixing MEF versions on the same type. VSMEF013 disallows a version entirely.
	- This analyzer is distinct from VSMEF002, which warns about mixing MEF versions on the same type. VSMEF012 disallows a version entirely.

More MEF analyzers and code fixes #659

Are you sure you want to change the base?

More MEF analyzers and code fixes #659

Uh oh!

Conversation

drewnoakes commented Dec 8, 2025

Uh oh!

AArnott commented Dec 9, 2025

Uh oh!

drewnoakes commented Dec 9, 2025

Uh oh!

AArnott commented Dec 9, 2025

Uh oh!

drewnoakes commented Dec 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

AArnott left a comment

Choose a reason for hiding this comment

Uh oh!

AArnott left a comment

Choose a reason for hiding this comment

Uh oh!

AArnott Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

AArnott Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

AArnott Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

AArnott Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

AArnott Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

drewnoakes commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants