Merge pull request #227 from AArnott/fix185

Add NBMsgPack051 analyzer

.github/workflows/build.yml

@@ -54,7 +54,7 @@ jobs:
       run: tools/dotnet-test-cloud.ps1 -Configuration ${{ env.BUILDCONFIGURATION }} -Agent ${{ runner.os }}
       shell: pwsh
     - name: 💅🏻 Verify formatted code
-      run: dotnet format --verify-no-changes --no-restore
+      run: dotnet format --verify-no-changes --no-restore --exclude ./samples/AnalyzerDocs/NBMsgPack051.cs
       shell: pwsh
       if: runner.os == 'Linux'
     - name: 📚 Verify docfx build

docfx/analyzers/NBMsgPack051.md

@@ -0,0 +1,26 @@
+# NBMsgPack051: Prefer modern .NET APIs
+
+Some APIs exist to support targeting .NET Standard or .NET Framework, while other APIs are available when targeting .NET that are far superior.
+This diagnostic is reported when code uses the lesser API while the preferred API is available.
+
+The diagnostic message will direct you to the preferred API.
+
+In multi-targeting projects where switching to the preferred API is inadvisable because it would break the build for older target frameworks or require the use of `#if` sections, you may suppress this warning.
+
+## Example violation
+
+The following type is declared using the non-generic @Nerdbank.MessagePack.KnownSubTypeAttribute.
+This is fine for projects that target .NET Standard or .NET Framework, but if the project targets .NET a warning will be emitted.
+
+[!code-csharp[](../../samples/AnalyzerDocs/NBMsgPack051.cs#Defective)]
+
+## Resolution
+
+Per the message in the warning, switch to the generic attribute:
+
+[!code-csharp[](../../samples/AnalyzerDocs/NBMsgPack051.cs#SwitchFix)]
+
+Or in a multitargeting project, use the preferred API only where it's available:
+
+[!code-csharp[](../../samples/AnalyzerDocs/NBMsgPack051.cs#MultiTargetingFix)]
+

docfx/analyzers/index.md

@@ -24,6 +24,7 @@ Rule ID | Category | Severity | Notes
 [NBMsgPack036](NBMsgPack036.md) | Usage | Error | Async converters should not reuse readers after returning them
 [NBMsgPack037](NBMsgPack037.md) | Usage | Warning | Async converters should override @Nerdbank.MessagePack.MessagePackConverter`1.PreferAsyncSerialization
 [NBMsgPack050](NBMsgPack050.md) | Usage | Warning | Use ref parameters for ref structs
+[NBMsgPack051](NBMsgPack051.md) | Usage | Warning | Prefer modern .NET APIs
 [NBMsgPack100](NBMsgPack100.md) | Migration | Info | Migrate MessagePack-CSharp formatter
 [NBMsgPack101](NBMsgPack101.md) | Migration | Info | Migrate to @Nerdbank.MessagePack.MessagePackConverterAttribute
 [NBMsgPack102](NBMsgPack102.md) | Migration | Info | Remove use of MessagePackObjectAttribute

docfx/analyzers/toc.yml

@@ -34,6 +34,8 @@
   href: NBMsgPack037.md
 - name: NBMsgPack050
   href: NBMsgPack050.md
+- name: NBMsgPack051
+  href: NBMsgPack051.md
 - name: NBMsgPack100
   href: NBMsgPack100.md
 - name: NBMsgPack101

samples/AnalyzerDocs/NBMsgPack051.cs

@@ -0,0 +1,58 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma warning disable NBMsgPackAsync
+
+#if NET
+
+namespace Samples.AnalyzerDocs.NBMsgPack051
+{
+    namespace Defective
+    {
+#pragma warning disable NBMsgPack051
+        #region Defective
+        [KnownSubType(typeof(MyDerived))] // NBMsgPack051: Use the generic version of this attribute instead.
+        class MyType { }
+
+        [GenerateShape]
+        partial class MyDerived : MyType
+        {
+        }
+        #endregion
+#pragma warning restore NBMsgPack051
+    }
+
+    namespace SwitchFix
+    {
+        #region SwitchFix
+        [KnownSubType<MyDerived>]
+        class MyType { }
+
+        [GenerateShape]
+        partial class MyDerived : MyType
+        {
+        }
+        #endregion
+    }
+
+    namespace MultiTargetingFix
+    {
+        #region MultiTargetingFix
+        #if NET
+        [KnownSubType<MyDerived>]
+        #else
+        [KnownSubType(typeof(MyDerived))]
+        #endif
+        class MyType { }
+
+        #if NET
+        [GenerateShape]
+        #endif
+        partial class MyDerived : MyType
+        {
+        }
+        #endregion
+    }
+}
+
+#endif

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

@@ -23,6 +23,7 @@ NBMsgPack035 | Usage | Error | Async converters should return readers
 NBMsgPack036 | Usage | Error | Async converters should not reuse readers after returning them
 NBMsgPack037 | Usage | Warning | Async converters should override PreferAsyncSerialization
 NBMsgPack050 | Usage | Warning | Use ref parameters for ref structs
+NBMsgPack051 | Usage | Warning | Prefer modern .NET APIs
 NBMsgPack100 | Migration | Info | Migrate MessagePack-CSharp formatter
 NBMsgPack101 | Migration | Info | Migrate to MessagePackConverterAttribute
 NBMsgPack102 | Migration | Info | Remove use of MessagePackObjectAttribute

src/Nerdbank.MessagePack.Analyzers/Constants.cs

@@ -18,4 +18,10 @@ public static class KeyAttribute
 		public const string IndexProperty = "Index";
 		public static readonly ImmutableArray<string> Namespace = ["Nerdbank", "MessagePack"];
 	}
+
+	public static class PreferDotNetAlternativeApiAttribute
+	{
+		public const string TypeName = "PreferDotNetAlternativeApiAttribute";
+		public static readonly ImmutableArray<string> Namespace = ["Nerdbank", "MessagePack"];
+	}
 }

src/Nerdbank.MessagePack.Analyzers/DotNetApiUsageAnalyzer.cs

@@ -0,0 +1,107 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Concurrent;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Nerdbank.MessagePack.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class DotNetApiUsageAnalyzer : DiagnosticAnalyzer
+{
+	public const string UseDotNetApiDiagnosticId = "NBMsgPack051";
+
+	public static readonly DiagnosticDescriptor UseDotNetApiDescriptor = new(
+		id: UseDotNetApiDiagnosticId,
+		title: Strings.NBMsgPack051_Title,
+		messageFormat: Strings.NBMsgPack051_MessageFormat,
+		category: "Usage",
+		defaultSeverity: DiagnosticSeverity.Warning,
+		isEnabledByDefault: true,
+		helpLinkUri: AnalyzerUtilities.GetHelpLink(UseDotNetApiDiagnosticId));
+
+	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [UseDotNetApiDescriptor];
+
+	public override void Initialize(AnalysisContext context)
+	{
+		context.EnableConcurrentExecution();
+		context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+		context.RegisterCompilationStartAction(context =>
+		{
+			if (!ReferenceSymbols.TryCreate(context.Compilation, out ReferenceSymbols? referenceSymbols))
+			{
+				return;
+			}
+
+			ConcurrentDictionary<ISymbol, string?> obsoleteMessage = new(SymbolEqualityComparer.Default);
+			context.RegisterOperationAction(
+				context =>
+				{
+					Location? location = null;
+					ISymbol? symbol = context.Operation switch
+					{
+						IInvocationOperation invocation => invocation.TargetMethod,
+						IAttributeOperation attribute => attribute.Type,
+						_ => null,
+					};
+
+					if (symbol is null)
+					{
+						return;
+					}
+
+					if (!obsoleteMessage.TryGetValue(symbol, out string? message))
+					{
+						message = symbol.FindAttributes(Constants.PreferDotNetAlternativeApiAttribute.TypeName, Constants.PreferDotNetAlternativeApiAttribute.Namespace)
+							.FirstOrDefault()
+							?.ConstructorArguments.FirstOrDefault().Value as string;
+						obsoleteMessage.TryAdd(symbol, message);
+					}
+
+					if (message is not null)
+					{
+						location ??= context.Operation.Syntax.GetLocation();
+						context.ReportDiagnostic(Diagnostic.Create(UseDotNetApiDescriptor, location, message));
+					}
+				},
+				OperationKind.Invocation | OperationKind.Attribute);
+
+			context.RegisterSymbolAction(
+				context =>
+				{
+					if (context.Symbol is not INamedTypeSymbol declaredType)
+					{
+						return;
+					}
+
+					foreach (AttributeData att in declaredType.GetAttributes())
+					{
+						if (att.AttributeClass is null)
+						{
+							continue;
+						}
+
+						if (!obsoleteMessage.TryGetValue(att.AttributeClass, out string? message))
+						{
+							message = att.AttributeClass.FindAttributes(Constants.PreferDotNetAlternativeApiAttribute.TypeName, Constants.PreferDotNetAlternativeApiAttribute.Namespace)
+								.FirstOrDefault()
+								?.ConstructorArguments.FirstOrDefault().Value as string;
+							obsoleteMessage.TryAdd(att.AttributeClass, message);
+						}
+
+						if (message is not null)
+						{
+							Location? location = att.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken).GetLocation();
+							if (location is not null)
+							{
+								context.ReportDiagnostic(Diagnostic.Create(UseDotNetApiDescriptor, location, message));
+							}
+						}
+					}
+				},
+				SymbolKind.NamedType);
+		});
+	}
+}

src/Nerdbank.MessagePack.Analyzers/Strings.resx

@@ -231,6 +231,12 @@
   <data name="NBMsgPack050_Title" xml:space="preserve">
     <value>Use ref parameters for ref structs</value>
   </data>
+  <data name="NBMsgPack051_MessageFormat" xml:space="preserve">
+    <value>{0} Consider suppressing this warning for multi-targeting projects.</value>
+  </data>
+  <data name="NBMsgPack051_Title" xml:space="preserve">
+    <value>Prefer modern .NET APIs</value>
+  </data>
   <data name="NBMsgPack100_MessageFormat" xml:space="preserve">
     <value>This is a formatter for the MessagePack-CSharp library. You may want to migrate this to a converter for the newer Nerdbank.MessagePack library.</value>
   </data>

src/Nerdbank.MessagePack/KnownSubTypeAttribute.cs

@@ -120,6 +120,9 @@ public KnownSubTypeAttribute()
 /// Each type referenced by this attribute must have <see cref="GenerateShapeAttribute"/> applied to it or a witness class.
 /// </para>
 /// </remarks>
+#if NET
+[PreferDotNetAlternativeApi("Use the generic version of this attribute instead.")]
+#endif
 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface, Inherited = false, AllowMultiple = true)]
 public class KnownSubTypeAttribute : Attribute
 {
@@ -133,19 +136,13 @@ public class KnownSubTypeAttribute : Attribute
 	/// </summary>
 	/// <param name="subType">The derived-type that the <paramref name="alias"/> represents.</param>
 	/// <param name="alias">A value that identifies the subtype in the serialized data. Must be unique among all the attributes applied to the same class.</param>
-#if NET
-	[Obsolete("Use the generic version of this attribute instead.")]
-#endif
 	public KnownSubTypeAttribute(Type subType, int alias)
 	{
 		this.Alias = alias;
 		this.SubType = subType;
 	}
 
 	/// <inheritdoc cref="KnownSubTypeAttribute(Type, int)" />
-#if NET
-	[Obsolete("Use the generic version of this attribute instead.")]
-#endif
 	public KnownSubTypeAttribute(Type subType, string alias)
 	{
 		this.Alias = alias;
@@ -161,9 +158,6 @@ public KnownSubTypeAttribute(Type subType, string alias)
 	/// Consider cross-platform compatibility when using this constructor, particularly when the serialized form may be exchanged with non-.NET programs
 	/// where the <see cref="Type.FullName"/> has no meaning.
 	/// </remarks>
-#if NET
-	[Obsolete("Use the generic version of this attribute instead.")]
-#endif
 	public KnownSubTypeAttribute(Type subType)
 	{
 		Requires.NotNull(subType);

src/Nerdbank.MessagePack/PreferDotNetAlternativeApiAttribute.cs

@@ -0,0 +1,13 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Nerdbank.MessagePack;
+
+/// <summary>
+/// Indicates that the decorated type or member has a .NET alternative that is preferred over the decorated API.
+/// </summary>
+/// <param name="advice">The message for the diagnostic to be created.</param>
+[AttributeUsage(AttributeTargets.Class | AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property)]
+#pragma warning disable CS9113 // Parameter is unread.
+internal class PreferDotNetAlternativeApiAttribute(string advice) : Attribute;
+#pragma warning restore CS9113 // Parameter is unread.

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

@@ -0,0 +1,58 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using VerifyCS = CodeFixVerifier<Nerdbank.MessagePack.Analyzers.DotNetApiUsageAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
+
+public class DotNetApiUsageAnalyzerTests
+{
+	[Fact]
+	public async Task KnownSubTypeAttribute()
+	{
+#if NET
+		string source = /* lang=c#-test */ """
+			using PolyType;
+			using PolyType.Abstractions;
+			using Nerdbank.MessagePack;
+
+			[{|NBMsgPack051:KnownSubTypeAttribute(typeof(MyDerived))|}]
+			class MyType { }
+
+			partial class MyDerived : MyType, IShapeable<MyDerived>
+			{
+				public static ITypeShape<MyDerived> GetShape() => throw new System.NotImplementedException();
+			}
+			""";
+#else
+		string source = /* lang=c#-test */ """
+			using Nerdbank.MessagePack;
+
+			[KnownSubTypeAttribute(typeof(MyDerived))]
+			class MyType { }
+
+			class MyDerived : MyType { }
+			""";
+#endif
+		await VerifyCS.VerifyAnalyzerAsync(source);
+	}
+
+#if NET
+	[Fact]
+	public async Task KnownSubTypeGenericAttribute()
+	{
+		string source = /* lang=c#-test */ """
+			using PolyType;
+			using PolyType.Abstractions;
+			using Nerdbank.MessagePack;
+
+			[KnownSubTypeAttribute<MyDerived>]
+			class MyType { }
+
+			partial class MyDerived : MyType, IShapeable<MyDerived>
+			{
+				public static ITypeShape<MyDerived> GetShape() => throw new System.NotImplementedException();
+			}
+			""";
+		await VerifyCS.VerifyAnalyzerAsync(source);
+	}
+#endif
+}