diff --git a/Directory.Packages.props b/Directory.Packages.props index 6dbefcc32..445a84251 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -8,18 +8,18 @@ 2.0.226 5.3.0 - 1.1.2 + 1.1.3 - - + + - - + + diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Shipped.md b/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Shipped.md deleted file mode 100644 index c97b488db..000000000 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Shipped.md +++ /dev/null @@ -1,13 +0,0 @@ -## Release 17.10 - -### New Rules -Rule ID | Category | Severity | Notes ---------|----------|----------|------- -VSMEF001 | Usage | Error | VSMEF001PropertyMustHaveSetter - -## Release 17.11 - -### New Rules -Rule ID | Category | Severity | Notes ---------|----------|----------|------- -VSMEF002 | Usage | Warning | VSMEF002AvoidMixingAttributeLibraries diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md b/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md deleted file mode 100644 index e49e3c032..000000000 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/AnalyzerReleases.Unshipped.md +++ /dev/null @@ -1,17 +0,0 @@ -; Unshipped analyzer release -; - -### New Rules - -Rule ID | Category | Severity | Notes ---------|----------|----------|------- -VSMEF003 | Usage | Warning | Exported type not implemented by exporting class -VSMEF004 | Usage | Error | Exported type missing importing constructor -VSMEF005 | Usage | Error | Multiple importing constructors -VSMEF006 | Usage | Warning | Import nullability and AllowDefault mismatch -VSMEF007 | Usage | Warning | Duplicate import contract -VSMEF008 | Usage | Warning | Import contract type not assignable to member type -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 | Warning | Disallow MEF attribute version diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/IDE0044ImportFieldSuppressor.cs b/src/Microsoft.VisualStudio.Composition.Analyzers/IDE0044ImportFieldSuppressor.cs new file mode 100644 index 000000000..1e5f7b7b5 --- /dev/null +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/IDE0044ImportFieldSuppressor.cs @@ -0,0 +1,81 @@ +// 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; + +/// +/// Suppresses IDE0044 ("Make field readonly") for fields decorated with MEF +/// [Import] or [ImportMany] attributes, +/// since such fields are assigned at runtime via reflection and cannot be made readonly. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] +public class IDE0044ImportFieldSuppressor : DiagnosticSuppressor +{ + /// + /// The suppressor ID. + /// + public const string Id = "VSMEF013"; + + private const string SuppressedDiagnosticId = "IDE0044"; + + /// + /// The descriptor for this suppressor. + /// + internal static readonly SuppressionDescriptor Descriptor = new( + id: Id, + suppressedDiagnosticId: SuppressedDiagnosticId, + justification: Strings.VSMEF013_Justification); + + /// + public override ImmutableArray SupportedSuppressions => + ImmutableArray.Create(Descriptor); + + /// + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + foreach (Diagnostic diagnostic in context.ReportedDiagnostics) + { + if (diagnostic.Id != SuppressedDiagnosticId) + { + continue; + } + + SyntaxTree? syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree is null) + { + continue; + } + + SemanticModel semanticModel = context.GetSemanticModel(syntaxTree); + SyntaxNode root = syntaxTree.GetRoot(context.CancellationToken); + SyntaxNode? node = root.FindNode(diagnostic.Location.SourceSpan); + + IFieldSymbol? field = null; + while (node is not null) + { + ISymbol? symbol = semanticModel.GetDeclaredSymbol(node, context.CancellationToken); + if (symbol is IFieldSymbol f) + { + field = f; + break; + } + + node = node.Parent; + } + + if (field is null) + { + continue; + } + + foreach (AttributeData attribute in field.GetAttributes()) + { + if (Utils.IsFieldImportAttribute(attribute.AttributeClass)) + { + context.ReportSuppression(Suppression.Create(Descriptor, diagnostic)); + break; + } + } + } + } +} diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/Microsoft.VisualStudio.Composition.Analyzers.csproj b/src/Microsoft.VisualStudio.Composition.Analyzers/Microsoft.VisualStudio.Composition.Analyzers.csproj index 80f0e702f..216f4f921 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/Microsoft.VisualStudio.Composition.Analyzers.csproj +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/Microsoft.VisualStudio.Composition.Analyzers.csproj @@ -6,6 +6,7 @@ Microsoft.VisualStudio.Composition.Analyzers.Only true false + $(NoWarn);RS2008 diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/README.md b/src/Microsoft.VisualStudio.Composition.Analyzers/README.md index 8146aff52..280c82d5c 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/README.md +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/README.md @@ -11,3 +11,9 @@ VSMEF004 | Ensures exported types have a parameterless constructor or importing VSMEF005 | Detects multiple constructors marked with `[ImportingConstructor]`. VSMEF006 | Ensures import nullability matches `AllowDefault` setting. VSMEF007 | Detects when a type imports the same contract multiple times. + +## Diagnostic Suppressors + +Suppressor ID | Suppressed Diagnostic | Description +--|--|-- +VSMEF013 | IDE0044 | Suppresses "Make field readonly" for fields decorated with MEF `[Import]` or `[ImportMany]` attributes, since such fields are assigned at runtime via reflection. diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx index e714c38b1..4c83a0e5c 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/Strings.resx @@ -274,4 +274,8 @@ MEFv2 attribute "{0}" is not allowed. Use the MEFv1 equivalent from System.ComponentModel.Composition. {Locked="MEFv1","MEFv2","System.ComponentModel.Composition"} + + Fields with [Import] or [ImportMany] attributes are assigned at runtime via reflection and cannot be made readonly. + {Locked="[Import]"} {Locked="[ImportMany]"} The attribute names in brackets should not be localized. + diff --git a/src/Microsoft.VisualStudio.Composition.Analyzers/Utils.cs b/src/Microsoft.VisualStudio.Composition.Analyzers/Utils.cs index db35983fe..564857064 100644 --- a/src/Microsoft.VisualStudio.Composition.Analyzers/Utils.cs +++ b/src/Microsoft.VisualStudio.Composition.Analyzers/Utils.cs @@ -284,6 +284,18 @@ internal static bool IsNamespaceMatch(INamespaceSymbol? actual, ReadOnlySpan IsImportAttribute(attr.AttributeClass)); } + internal static bool IsFieldImportAttribute(INamedTypeSymbol? attributeType) + { + if (attributeType is null) + { + return false; + } + + // MEFv2 attributes don't support field imports, so only check MEFv1 Import and ImportMany attributes. + return IsAttributeOfType(attributeType, "ImportAttribute", MefV1AttributeNamespace.AsSpan()) || + IsAttributeOfType(attributeType, "ImportManyAttribute", MefV1AttributeNamespace.AsSpan()); + } + internal static bool IsImportAttribute(INamedTypeSymbol? attributeType) { if (attributeType is null) diff --git a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Helpers/CSharpMultiAnalyzerVerifier+Test.cs b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Helpers/CSharpMultiAnalyzerVerifier+Test.cs index 5466a33c6..01651deb1 100644 --- a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Helpers/CSharpMultiAnalyzerVerifier+Test.cs +++ b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Helpers/CSharpMultiAnalyzerVerifier+Test.cs @@ -56,6 +56,7 @@ protected override IEnumerable GetDiagnosticAnalyzers() new VSMEF010ImportManyParameterCollectionTypeAnalyzer(), new VSMEF011BothImportAndImportManyAnalyzer(), new VSMEF012DisallowMefAttributeVersionAnalyzer(), + new IDE0044ImportFieldSuppressor(), ]; } diff --git a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/IDE0044ImportFieldSuppressorTests.cs b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/IDE0044ImportFieldSuppressorTests.cs new file mode 100644 index 000000000..24e614c47 --- /dev/null +++ b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/IDE0044ImportFieldSuppressorTests.cs @@ -0,0 +1,162 @@ +// 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.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.VisualStudio.Composition.Analyzers; + +public class IDE0044ImportFieldSuppressorTests +{ + [Fact] + public async Task FieldWithMefV1ImportAttribute_SuppressesIDE0044() + { + string test = """ + using System.ComponentModel.Composition; + + class Foo + { + [Import] + private object someField; + } + """; + + await new Test + { + TestCode = test, + ExpectedDiagnostics = { Ide0044Diagnostic(6, 20, 6, 29, isSuppressed: true) }, + }.RunAsync(); + } + + [Fact] + public async Task FieldWithMefV1ImportManyAttribute_SuppressesIDE0044() + { + string test = """ + using System.Collections.Generic; + using System.ComponentModel.Composition; + + class Foo + { + [ImportMany] + private IEnumerable someField; + } + """; + + await new Test + { + TestCode = test, + ExpectedDiagnostics = { Ide0044Diagnostic(7, 33, 7, 42, isSuppressed: true) }, + }.RunAsync(); + } + + [Fact] + public async Task FieldWithoutMefAttribute_DoesNotSuppressIDE0044() + { + string test = """ + class Foo + { + private object {|IDE0044:someField|}; + } + """; + + await new Test { TestCode = test }.RunAsync(); + } + + [Fact] + public async Task ReadonlyField_NoDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + class Foo + { + private readonly object someField = null; + } + """; + + await new Test { TestCode = test }.RunAsync(); + } + + [Fact] + public async Task ClassWithMixedFields_OnlyNonMefFieldsGetDiagnostic() + { + string test = """ + using System.ComponentModel.Composition; + + class Foo + { + [Import] + private object mefField; + + private object {|IDE0044:regularField|}; + } + """; + + await new Test + { + TestCode = test, + ExpectedDiagnostics = { Ide0044Diagnostic(6, 20, 6, 28, isSuppressed: true) }, + }.RunAsync(); + } + + private static DiagnosticResult Ide0044Diagnostic(int startLine, int startColumn, int endLine, int endColumn, bool isSuppressed = false) + { + DiagnosticResult result = new DiagnosticResult(FakeIDE0044Analyzer.Descriptor).WithSpan(startLine, startColumn, endLine, endColumn); + return isSuppressed ? result.WithIsSuppressed(true) : result; + } + + /// + /// A fake analyzer that produces IDE0044 for non-readonly, non-const fields, + /// simulating IDE behavior for suppressor testing. + /// +#pragma warning disable RS1001 // Missing DiagnosticAnalyzerAttribute - intentionally omitted for test-only class + private sealed class FakeIDE0044Analyzer : DiagnosticAnalyzer +#pragma warning restore RS1001 + { +#pragma warning disable RS2008 // Enable analyzer release tracking - this is a test-only fake analyzer + internal static readonly DiagnosticDescriptor Descriptor = new( + id: "IDE0044", + title: "Make field readonly", + messageFormat: "Make field readonly", + category: "Style", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); +#pragma warning restore RS2008 + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(Descriptor); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSymbolAction(AnalyzeField, SymbolKind.Field); + } + + private static void AnalyzeField(SymbolAnalysisContext context) + { + var field = (IFieldSymbol)context.Symbol; + if (!field.IsReadOnly && !field.IsConst) + { + Location? location = field.Locations.FirstOrDefault(); + if (location is not null) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, location)); + } + } + } + } + + private sealed class Test : CSharpCodeFixTest + { + public Test() + { + this.ReferenceAssemblies = ReferencesHelper.DefaultReferences; + } + + protected override IEnumerable GetDiagnosticAnalyzers() + { + yield return new FakeIDE0044Analyzer(); + yield return new IDE0044ImportFieldSuppressor(); + } + } +} diff --git a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Microsoft.VisualStudio.Composition.Analyzers.Tests.csproj b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Microsoft.VisualStudio.Composition.Analyzers.Tests.csproj index ada21cdfa..49ccee144 100644 --- a/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Microsoft.VisualStudio.Composition.Analyzers.Tests.csproj +++ b/test/Microsoft.VisualStudio.Composition.Analyzers.Tests/Microsoft.VisualStudio.Composition.Analyzers.Tests.csproj @@ -10,9 +10,9 @@ - + - +