Add more analyzer and code fixes for more complete migrations

docfx/analyzers/NBMsgPack104.md

@@ -0,0 +1,6 @@
+# NBMsgPack104: Remove use of IgnoreMemberAttribute
+
+This diagnostic is emitted where a `[MessagePack.IgnoreMemberAttribute]` attribute is used.
+A code fix is offered to remove this attribute or switch to with @PolyType.PropertyShapeAttribute with its @PolyType.PropertyShapeAttribute.Ignore property set to `true`.
+
+[Learn more about migrating](../docs/migrating.md).

docfx/analyzers/index.md

@@ -21,3 +21,4 @@ Rule ID | Category | Severity | Notes
 [NBMsgPack101](NBMsgPack101.md) | Migration | Info | Migrate to MessagePackConverterAttribute
 [NBMsgPack102](NBMsgPack102.md) | Migration | Info | Remove use of MessagePackObjectAttribute
 [NBMsgPack103](NBMsgPack103.md) | Migration | Info | Use newer KeyAttribute
+[NBMsgPack104](NBMsgPack104.md) | Migration | Info | Remove use of IgnoreMemberAttribute

docfx/analyzers/toc.yml

@@ -28,3 +28,5 @@
   href: NBMsgPack102.md
 - name: NBMsgPack103
   href: NBMsgPack103.md
+- name: NBMsgPack104
+  href: NBMsgPack104.md

docfx/docs/migrating.md

@@ -73,8 +73,32 @@ Learn more about this in our [Getting Started](getting-started.md) guide.
  }
 ```
 
+#### `KeyAttribute`
+
 Nerdbank.MessagePack also supports @Nerdbank.MessagePack.KeyAttribute, which serves the same function as in MessagePack-CSharp: to change the serialized schema from that of a map of property name=value to an array of values.
-Thus, you may keep the `[Key(0)]`, `[Key(1)]`, etc., attributes on your types if you wish to maintain the schema of the serialized data.
+Thus, you may keep the `[Key(0)]`, `[Key(1)]`, etc., attributes on your types if you wish to maintain the schema of the serialized data, provided you change the namespace.
+
+If using `[Key("name")]` attributes as a means to change the serialized property names, this must be replaced with @PolyType.PropertyShapeAttribute with @PolyType.PropertyShapeAttribute.Name?displayProperty=nameWithType set to the serialized name.
+
+```diff
+-[Key("name")]
++[PropertyShape(Name = "name")]
+ public string SomeProperty { get; set; }
+```
+
+#### `IgnoreMemberAttribute`
+
+The `[IgnoreMemberAttribute]` that comes from MessagePack-CSharp can be removed from non-public members, which are never considered for serialization by default.
+For public members that should be ignored, replace this attribute with @PolyType.PropertyShapeAttribute with @PolyType.PropertyShapeAttribute.Ignore?displayProperty=nameWithType set to `true`.
+
+```diff
+-[IgnoreMember]
++[PropertyShape(Ignore = true)]
+ public int SomeProperty { get; set; }
+
+-[IgnoreMember]
+ internal int AnotherProperty { get; set; }
+```
 
 ### `UnionAttribute`
 

src/Nerdbank.MessagePack.Analyzers.CodeFixes/MigrationCodeFix.cs

@@ -26,11 +26,12 @@ public class MigrationCodeFix : CodeFixProvider
 		MigrationAnalyzer.FormatterAttributeDiagnosticId,
 		MigrationAnalyzer.MessagePackObjectAttributeUsageDiagnosticId,
 		MigrationAnalyzer.KeyAttributeUsageDiagnosticId,
+		MigrationAnalyzer.IgnoreMemberAttributeUsageDiagnosticId,
 	];
 
 	public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
 
-	public override Task RegisterCodeFixesAsync(CodeFixContext context)
+	public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 	{
 		foreach (Diagnostic diagnostic in context.Diagnostics)
 		{
@@ -61,17 +62,61 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
 						diagnostic);
 					break;
 				case MigrationAnalyzer.KeyAttributeUsageDiagnosticId:
-					context.RegisterCodeFix(
-						CodeAction.Create(
-							title: "Use Nerdbank.MessagePack.KeyAttribute",
-							createChangedDocument: cancellationToken => this.ReplaceKeyAttributeAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken),
-							equivalenceKey: "Use Nerdbank.MessagePack.KeyAttribute"),
-						diagnostic);
+					// Determine whether to replace with KeyAttribute or PropertyShapeAttribute.
+					SyntaxNode? tree = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
+					if (tree?.FindNode(context.Span) is AttributeSyntax keyAttribute && IsStringKeyAttribute(keyAttribute))
+					{
+						context.RegisterCodeFix(
+							CodeAction.Create(
+								title: "Use PropertyShape(Name) instead",
+								createChangedDocument: cancellationToken => this.ReplaceKeyAttributeAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken),
+								equivalenceKey: "Use PropertyShapeAttribute.Name"),
+							diagnostic);
+					}
+					else
+					{
+						context.RegisterCodeFix(
+							CodeAction.Create(
+								title: "Use Nerdbank.MessagePack.KeyAttribute",
+								createChangedDocument: cancellationToken => this.ReplaceKeyAttributeAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken),
+								equivalenceKey: "Use Nerdbank.MessagePack.KeyAttribute"),
+							diagnostic);
+					}
+
+					break;
+				case MigrationAnalyzer.IgnoreMemberAttributeUsageDiagnosticId:
+					// Determine whether to remove or replace the attribute and offer the appropriate code fix for it.
+					tree = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
+					if (tree?.FindNode(context.Span) is AttributeSyntax { Parent.Parent: MemberDeclarationSyntax member } att)
+					{
+						if (member.Modifiers.Any(SyntaxKind.PublicKeyword))
+						{
+							context.RegisterCodeFix(
+								CodeAction.Create(
+									title: "Replace IgnoreMemberAttribute with PropertyShapeAttribute",
+									createChangedDocument: cancellationToken => this.ReplaceIgnoreMemberAttribute(context.Document, att, false, cancellationToken),
+									equivalenceKey: "Replace IgnoreMemberAttribute with PropertyShapeAttribute"),
+								diagnostic);
+						}
+						else
+						{
+							context.RegisterCodeFix(
+								CodeAction.Create(
+									title: "Remove IgnoreMemberAttribute",
+									createChangedDocument: cancellationToken => this.ReplaceIgnoreMemberAttribute(context.Document, att, true, cancellationToken),
+									equivalenceKey: "Remove IgnoreMemberAttribute"),
+								diagnostic);
+						}
+					}
+
 					break;
 			}
 		}
+	}
 
-		return Task.CompletedTask;
+	private static bool IsStringKeyAttribute(AttributeSyntax attribute)
+	{
+		return attribute is { ArgumentList.Arguments: [AttributeArgumentSyntax { Expression: LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } }] };
 	}
 
 	private static NameSyntax NameInNamespace(SimpleNameSyntax name) => NameInNamespace(name, Namespace);
@@ -231,17 +276,51 @@ private async Task<Document> ReplaceKeyAttributeAsync(Document document, TextSpa
 			return document;
 		}
 
-		AttributeSyntax newAttribute = attribute.WithName(NameInNamespace(IdentifierName("Key")));
+		// One of:
+		// => PropertyShape(Name = "")
+		// => [Key(n)]
+		AttributeSyntax newAttribute = IsStringKeyAttribute(attribute)
+			? Attribute(NameInNamespace(IdentifierName("PropertyShape"), IdentifierName("PolyType")))
+				.AddArgumentListArguments(attribute.ArgumentList!.Arguments[0].WithNameEquals(NameEquals(IdentifierName("Name"))).WithAdditionalAnnotations(Formatter.Annotation))
+			: attribute.WithName(NameInNamespace(IdentifierName("Key")));
+
 		root = root.ReplaceNode(attribute, newAttribute);
 
-		return document.WithSyntaxRoot(root);
+		return await this.AddImportAndSimplifyAsync(document.WithSyntaxRoot(root), cancellationToken);
+	}
+
+	private async Task<Document> ReplaceIgnoreMemberAttribute(Document document, AttributeSyntax attribute, bool remove, CancellationToken cancellationToken)
+	{
+		CompilationUnitSyntax? root = (CompilationUnitSyntax)await attribute.SyntaxTree.GetRootAsync(cancellationToken);
+
+		if (remove)
+		{
+			root = attribute.Parent is AttributeListSyntax { Attributes.Count: 1 }
+				? root.RemoveNode(attribute.Parent, SyntaxRemoveOptions.KeepEndOfLine)
+				: root.RemoveNode(attribute, SyntaxRemoveOptions.KeepNoTrivia);
+		}
+		else
+		{
+			AttributeSyntax propertyShapeAttribute = Attribute(NameInNamespace(IdentifierName("PropertyShape"), IdentifierName("PolyType")))
+				.AddArgumentListArguments(
+					AttributeArgument(LiteralExpression(SyntaxKind.TrueLiteralExpression)).WithNameEquals(NameEquals(IdentifierName("Ignore"))).WithAdditionalAnnotations(Formatter.Annotation));
+			root = root.ReplaceNode(attribute, propertyShapeAttribute);
+		}
+
+		if (root is null)
+		{
+			return document;
+		}
+
+		return await this.AddImportAndSimplifyAsync(document.WithSyntaxRoot(root), cancellationToken);
 	}
 
 	private async Task<Document> AddImportAndSimplifyAsync(Document document, CancellationToken cancellationToken)
 	{
 		Document modifiedDocument = document;
 		modifiedDocument = await ImportAdder.AddImportsAsync(document, cancellationToken: cancellationToken);
 		modifiedDocument = await Simplifier.ReduceAsync(modifiedDocument, cancellationToken: cancellationToken);
+		modifiedDocument = await Formatter.FormatAsync(modifiedDocument, cancellationToken: cancellationToken);
 		return modifiedDocument;
 	}
 

src/Nerdbank.MessagePack.Analyzers/AnalyzerReleases.Unshipped.md

@@ -20,3 +20,4 @@ NBMsgPack100 | Migration | Info | Migrate MessagePack-CSharp formatter
 NBMsgPack101 | Migration | Info | Migrate to MessagePackConverterAttribute
 NBMsgPack102 | Migration | Info | Remove use of MessagePackObjectAttribute
 NBMsgPack103 | Migration | Info | Use newer KeyAttribute
+NBMsgPack104 | Migration | Info | Remove use of IgnoreMemberAttribute

src/Nerdbank.MessagePack.Analyzers/MessagePackCSharpReferenceSymbols.cs

@@ -13,6 +13,7 @@ public record MessagePackCSharpReferenceSymbols(
 	INamedTypeSymbol MessagePackFormatterAttribute,
 	INamedTypeSymbol MessagePackObjectAttribute,
 	INamedTypeSymbol KeyAttribute,
+	INamedTypeSymbol IgnoreMemberAttribute,
 	INamedTypeSymbol MessagePackSecurity,
 	IMethodSymbol DepthStep,
 	INamedTypeSymbol MessagePackReader,
@@ -89,6 +90,13 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out Me
 			return false;
 		}
 
+		INamedTypeSymbol? ignoreMemberAttribute = oldLibraryAnnotationsAssembly.GetTypeByMetadataName("MessagePack.IgnoreMemberAttribute");
+		if (ignoreMemberAttribute is null)
+		{
+			referenceSymbols = null;
+			return false;
+		}
+
 		INamedTypeSymbol? messagePackSecurity = oldLibraryAssembly.GetTypeByMetadataName("MessagePack.MessagePackSecurity");
 		if (messagePackSecurity is null)
 		{
@@ -160,6 +168,7 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out Me
 			messagePackFormatterAttribute,
 			messagePackObjectAttribute,
 			keyAttribute,
+			ignoreMemberAttribute,
 			messagePackSecurity,
 			depthStep,
 			reader,

src/Nerdbank.MessagePack.Analyzers/MigrationAnalyzer.cs

@@ -12,6 +12,7 @@ public class MigrationAnalyzer : DiagnosticAnalyzer
 	public const string FormatterAttributeDiagnosticId = "NBMsgPack101";
 	public const string MessagePackObjectAttributeUsageDiagnosticId = "NBMsgPack102";
 	public const string KeyAttributeUsageDiagnosticId = "NBMsgPack103";
+	public const string IgnoreMemberAttributeUsageDiagnosticId = "NBMsgPack104";
 
 	public static readonly DiagnosticDescriptor FormatterDiagnostic = new(
 		id: FormatterDiagnosticId,
@@ -49,11 +50,21 @@ public class MigrationAnalyzer : DiagnosticAnalyzer
 		isEnabledByDefault: true,
 		helpLinkUri: AnalyzerUtilities.GetHelpLink(KeyAttributeUsageDiagnosticId));
 
+	public static readonly DiagnosticDescriptor IgnoreMemberAttributeUsageDiagnostic = new(
+		id: IgnoreMemberAttributeUsageDiagnosticId,
+		title: Strings.NBMsgPack104_Title,
+		messageFormat: Strings.NBMsgPack104_MessageFormat,
+		category: "Migration",
+		defaultSeverity: DiagnosticSeverity.Info,
+		isEnabledByDefault: true,
+		helpLinkUri: AnalyzerUtilities.GetHelpLink(IgnoreMemberAttributeUsageDiagnosticId));
+
 	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [
 		FormatterDiagnostic,
 		FormatterAttributeDiagnostic,
 		MessagePackObjectAttributeUsageDiagnostic,
 		KeyAttributeUsageDiagnostic,
+		IgnoreMemberAttributeUsageDiagnostic,
 	];
 
 	public override void Initialize(AnalysisContext context)
@@ -77,6 +88,12 @@ public override void Initialize(AnalysisContext context)
 					{
 						context.ReportDiagnostic(Diagnostic.Create(KeyAttributeUsageDiagnostic, keyAttribute.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken).GetLocation()));
 					}
+
+					// Look for applications of [IgnoreMember]
+					if (context.Symbol.FindAttributes(oldLibrarySymbols.IgnoreMemberAttribute).FirstOrDefault() is AttributeData ignoreMemberAttribute)
+					{
+						context.ReportDiagnostic(Diagnostic.Create(IgnoreMemberAttributeUsageDiagnostic, ignoreMemberAttribute.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken).GetLocation()));
+					}
 				},
 				SymbolKind.Property,
 				SymbolKind.Field);

src/Nerdbank.MessagePack.Analyzers/Strings.resx

@@ -213,4 +213,10 @@
   <data name="NBMsgPack103_Title" xml:space="preserve">
     <value>Use newer KeyAttribute</value>
   </data>
+  <data name="NBMsgPack104_MessageFormat" xml:space="preserve">
+    <value>Use PolyType.PropertyShape(Ignore = true) instead.</value>
+  </data>
+  <data name="NBMsgPack104_Title" xml:space="preserve">
+    <value>Avoid IgnoreMemberAttribute</value>
+  </data>
 </root>

test/Nerdbank.MessagePack.Analyzers.Tests/MigrationAnalyzerTests.cs

@@ -130,7 +130,7 @@ public override void Write(ref Nerdbank.MessagePack.MessagePackWriter writer, in
 		await this.VerifyCodeFixAsync(source, fixedSource);
 	}
 
-	[Fact(Skip = "Not yet passing due to unexpected iteration count")]
+	[Fact]
 	public async Task MessagePackObject_Keys()
 	{
 		string source = /* lang=c#-test */ """
@@ -156,7 +156,45 @@ public class MyType
 			}
 			""";
 
-		await this.VerifyCodeFixAsync(source, fixedSource);
+		await this.VerifyCodeFixAsync(source, fixedSource, 2);
+	}
+
+	[Fact]
+	public async Task MessagePackObject_IgnoreMember()
+	{
+		string source = /* lang=c#-test */ """
+			using MessagePack;
+			using MessagePack.Formatters;
+
+			public class MyType
+			{
+				public string Name { get; set; }
+
+				[{|NBMsgPack104:IgnoreMember|}]
+				public string PublicIgnored { get; set; }
+
+				[{|NBMsgPack104:IgnoreMember|}]
+				internal string NonPublicIgnored { get; set; }
+			}
+			""";
+
+		string fixedSource = /* lang=c#-test */ """
+			using MessagePack;
+			using MessagePack.Formatters;
+			using PolyType;
+
+			public class MyType
+			{
+				public string Name { get; set; }
+
+				[PropertyShape(Ignore = true)]
+				public string PublicIgnored { get; set; }
+
+				internal string NonPublicIgnored { get; set; }
+			}
+			""";
+
+		await this.VerifyCodeFixAsync(source, fixedSource, 2);
 	}
 
 	[Fact]
@@ -170,20 +208,27 @@ public async Task MessagePackObject_Map()
 			public class MyType
 			{
 				public string Name { get; set; }
+
+				[{|NBMsgPack103:Key("AnotherName")|}]
+				public string AnotherProperty { get; set; }
 			}
 			""";
 
 		string fixedSource = /* lang=c#-test */ """
 			using MessagePack;
 			using MessagePack.Formatters;
+			using PolyType;
 
 			public class MyType
 			{
 				public string Name { get; set; }
+
+				[PropertyShape(Name = "AnotherName")]
+				public string AnotherProperty { get; set; }
 			}
 			""";
 
-		await this.VerifyCodeFixAsync(source, fixedSource);
+		await this.VerifyCodeFixAsync(source, fixedSource, 2);
 	}
 
 	/// <summary>
@@ -235,7 +280,7 @@ void Foo()
 		await this.VerifyCodeFixAsync(source, fixedSource);
 	}
 
-	private Task VerifyCodeFixAsync([StringSyntax("c#-test")] string source, [StringSyntax("c#-test")] string fixedSource)
+	private Task VerifyCodeFixAsync([StringSyntax("c#-test")] string source, [StringSyntax("c#-test")] string fixedSource, int iterations = 1)
 	{
 		return new VerifyCS.Test
 		{
@@ -244,6 +289,10 @@ private Task VerifyCodeFixAsync([StringSyntax("c#-test")] string source, [String
 			ReferenceAssemblies = ReferencesHelper.DefaultTargetFrameworkReferences.WithPackages([
 				new PackageIdentity("MessagePack", "2.5.187"),
 			]),
+			NumberOfFixAllInDocumentIterations = iterations,
+			NumberOfFixAllInProjectIterations = iterations,
+			NumberOfFixAllIterations = iterations,
+			NumberOfIncrementalIterations = iterations,
 		}.RunAsync();
 	}
 }