Add Item #9 - minimize boxing and unboxing

build/targets/compiler/Compiler.props

@@ -11,4 +11,13 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
   </ItemGroup>
+
+  <ItemGroup>
+    <AssemblyAttribute Include="System.CLSCompliantAttribute">
+      <_Parameter1>false</_Parameter1>
+    </AssemblyAttribute>
+    <AssemblyAttribute Include="System.Runtime.InteropServices.ComVisibleAttribute">
+      <_Parameter1>false</_Parameter1>
+    </AssemblyAttribute>
+  </ItemGroup>  
 </Project>

docs/rules/ECS0009.md

@@ -0,0 +1,73 @@
+# ECS0009: Minimize boxing and unboxing
+
+This rule is described in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/).
+
+## Cause
+
+Value types can be converted to `System.Object` or any interface reference. Those conversions may happen implicitly, complicating the task of finding them. The boxing and unboxing operations make copies where you might not expect. That causes bugs.Boxing and unboxing operations can degrade performance and cause subtle bugs through those copies. These operations occur when a value type is converted to a reference type and vice versa. Be on the lookout for any constructs that convert value types to either `System.Object` or interface types: placing values in collections, calling methods defined in `System.Object`, and casts to `System.Object`.
+
+## Rule description
+
+This rule detects scenarios where boxing and unboxing occur implicitly or explicitly. It aims to help developers identify and minimize these operations to improve performance and avoid potential issues.
+
+## How to fix violations
+
+To fix violations, consider using generics, value type collections, or other means to avoid converting value types to reference types.
+
+## When to suppress warnings
+
+Suppress warnings if boxing or unboxing is necessary and there is no performance-critical impact, or if the code is optimized for readability and maintainability rather than performance.
+
+## Example of a violation
+
+### Description
+
+Assigning a value type to a reference type or passing a value type to a method that expects a reference type causes boxing.
+
+### Code
+
+```csharp
+int i = 5;
+object o = i; // boxing
+```
+
+Boxing may also occur in compiler-generated code implicitly.
+
+```csharp
+var attendees = new List<Person>();
+var p = new Person { Name = "Old Name" };
+attendees.Add(p);
+
+// Try to change the name
+var p2 = attendees[0];
+p2.Name = "New Name"; // Boxing occurs here
+
+// Writes "Old Name":
+Console.WriteLine(attendees[0].ToString());
+```
+
+## Example of how to fix
+
+### Description
+
+Use collections or methods that avoid boxing and unboxing operations.
+
+### Code
+
+```csharp
+int i = 5;
+int j = i; // No boxing
+```
+
+For the `Person` value type, create an immutable value type.
+
+```csharp
+public struct Person
+{
+    public string Name { get; }
+
+    public Person(string name) => Name = name;
+
+    public override string ToString() => Name;
+}
+```

src/EffectiveCSharp.Analyzers/AnalyzerReleases.Unshipped.md

@@ -5,5 +5,6 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
-ECS0006 | Refactoring | Warning | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/main/docs/ECS0006.md)
+ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0006.md)
+ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0009.md)
 ECS1000 | Performance | Info | SpanAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/d00a4cc9f61e7d5b392894aad859e46c43a5611c/docs/ECS1000.md)

src/EffectiveCSharp.Analyzers/AvoidStringlyTypedApisAnalyzer.cs

@@ -21,11 +21,10 @@ public class AvoidStringlyTypedApisAnalyzer : DiagnosticAnalyzer
         Title,
         MessageFormat,
         Category,
-        DiagnosticSeverity.Warning,
+        DiagnosticSeverity.Info,
         isEnabledByDefault: true,
         description: Description,
-        helpLinkUri:
-        $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers{ThisAssembly.GitCommitId}/docs/{DiagnosticId}.md");
+        helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers{ThisAssembly.GitCommitId}/docs/{DiagnosticId}.md");
 
     /// <inheritdoc/>
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

src/EffectiveCSharp.Analyzers/AvoidStringlyTypedApisCodeFixProvider.cs

@@ -20,7 +20,7 @@
     public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
     {
         SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
-        Diagnostic diagnostic = context.Diagnostics.First();
+        Diagnostic diagnostic = context.Diagnostics[0];
         TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
 
         SyntaxNode? node = root?.FindNode(diagnosticSpan);
@@ -33,7 +33,7 @@
             // Check if the node is an ArgumentSyntax containing a LiteralExpressionSyntax
             ArgumentSyntax { Expression: LiteralExpressionSyntax argLiteralNode } => argLiteralNode,
 
-            _ => null
+            _ => null,
         };
 
         if (literalExpression != null)
@@ -49,7 +49,7 @@
 
     private static async Task<Solution> UseNameofOperatorAsync(Document document, LiteralExpressionSyntax? literalExpression, CancellationToken cancellationToken)
     {
-        string literalValue = literalExpression?.Token.ValueText;
+        string? literalValue = literalExpression?.Token.ValueText;
 
         // Walk up the syntax tree to find the containing class or method
         TypeDeclarationSyntax? containingClass = literalExpression?.FirstAncestorOrSelf<TypeDeclarationSyntax>();
@@ -65,7 +65,7 @@
             {
                 IEnumerable<string> memberNames = containingTypeSymbol.GetMembers().Select(member => member.Name);
 
-                if (memberNames.Contains(literalValue))
+                if (memberNames.Contains(literalValue, StringComparer.Ordinal))
                 {
                     nameofExpressionText = $"nameof({literalValue})";
                 }
@@ -80,7 +80,7 @@
             {
                 IEnumerable<string> parameterNames = methodSymbol.Parameters.Select(parameter => parameter.Name);
 
-                if (parameterNames.Contains(literalValue))
+                if (parameterNames.Contains(literalValue, StringComparer.Ordinal))
                 {
                     nameofExpressionText = $"nameof({literalValue})";
                 }
@@ -92,7 +92,7 @@
             return document.Project.Solution;
         }
 
         if (literalExpression != null)
         {
             ExpressionSyntax nameofExpression = SyntaxFactory.ParseExpression(nameofExpressionText)
                 .WithTriviaFrom(literalExpression);

src/EffectiveCSharp.Analyzers/Common/DiagnosticExtensions.cs

@@ -2,19 +2,22 @@
 
 internal static class DiagnosticExtensions
 {
+    [DebuggerStepThrough]
     internal static Diagnostic CreateDiagnostic(
         this SyntaxNode node,
         DiagnosticDescriptor rule,
         params object?[]? messageArgs)
         => node.CreateDiagnostic(rule, properties: null, messageArgs);
 
+    [DebuggerStepThrough]
     internal static Diagnostic CreateDiagnostic(
         this SyntaxNode node,
         DiagnosticDescriptor rule,
         ImmutableDictionary<string, string?>? properties,
         params object?[]? messageArgs)
         => node.CreateDiagnostic(rule, additionalLocations: ImmutableArray<Location>.Empty, properties, messageArgs);
 
+    [DebuggerStepThrough]
     internal static Diagnostic CreateDiagnostic(
         this SyntaxNode node,
         DiagnosticDescriptor rule,
@@ -29,6 +32,7 @@ internal static Diagnostic CreateDiagnostic(
                 properties: properties,
                 messageArgs: messageArgs);
 
+    [DebuggerStepThrough]
     internal static Diagnostic CreateDiagnostic(
         this Location location,
         DiagnosticDescriptor rule,
@@ -46,6 +50,7 @@ internal static Diagnostic CreateDiagnostic(
         params object?[]? messageArgs)
         => location.CreateDiagnostic(rule, ImmutableArray<Location>.Empty, properties, messageArgs);
 
+    [DebuggerStepThrough]
     internal static Diagnostic CreateDiagnostic(
         this Location location,
         DiagnosticDescriptor rule,

src/EffectiveCSharp.Analyzers/Common/IOperationExtensions.cs

@@ -0,0 +1,28 @@
+namespace EffectiveCSharp.Analyzers.Common;
+
+internal static class IOperationExtensions
+{
+    /// <summary>
+    /// Determines if a given operation involves boxing through type conversion.
+    /// </summary>
+    /// <param name="operation">The operation to check.</param>
+    /// <returns>True if the operation is a boxing conversion, otherwise false.</returns>
+    internal static bool IsBoxingOperation(this IOperation? operation)
+        => operation is IConversionOperation { Operand.Type.IsValueType: true, Type.IsReferenceType: true };
+
+    /// <summary>
+    /// Determines if a given operation involves unboxing through type conversion.
+    /// </summary>
+    /// <param name="operation">The operation to check.</param>
+    /// <returns>True if the operation is an unboxing conversion, otherwise false.</returns>
+    internal static bool IsUnboxingOperation(this IOperation? operation)
+        => operation is IConversionOperation { Operand.Type.IsReferenceType: true, Type.IsValueType: true };
+
+    /// <summary>
+    /// Determines if a given operation involves boxing or unboxing through type conversion.
+    /// </summary>
+    /// <param name="operation">The operation to check.</param>
+    /// <returns>True if the operation is a boxing or unboxing conversion, otherwise false.</returns>
+    internal static bool IsBoxingOrUnboxingOperation(this IOperation? operation)
+        => operation.IsBoxingOperation() || operation.IsUnboxingOperation();
+}

src/EffectiveCSharp.Analyzers/DiagnosticIds.cs

@@ -4,5 +4,7 @@ internal static class DiagnosticIds
 {
     internal const string PreferReadonlyOverConst = "ECS0002";
     internal const string AvoidStringlyTypedApis = "ECS0006";
+    internal const string MinimizeBoxingUnboxing = "ECS0009";
+    internal const string BeAwareOfValueTypeCopyInReferenceTypes = "ECS0009";
     internal const string UseSpanInstead = "ECS1000";
 }

src/EffectiveCSharp.Analyzers/GlobalUsings.cs

@@ -8,4 +8,5 @@
 global using Microsoft.CodeAnalysis.CSharp;
 global using Microsoft.CodeAnalysis.CSharp.Syntax;
 global using Microsoft.CodeAnalysis.Diagnostics;
+global using Microsoft.CodeAnalysis.Operations;
 global using Microsoft.CodeAnalysis.Text;

src/EffectiveCSharp.Analyzers/MinimizeBoxingUnboxingAnalyzer.cs

@@ -0,0 +1,122 @@
+namespace EffectiveCSharp.Analyzers;
+
+/// <summary>
+/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #9 - Minimize boxing and unboxing.
+/// </summary>
+/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class MinimizeBoxingUnboxingAnalyzer : DiagnosticAnalyzer
+{
+    private const string Id = DiagnosticIds.MinimizeBoxingUnboxing;
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: Id,
+        title: "Minimize boxing and unboxing",
+        messageFormat: "Consider using an alternative implementation to avoid boxing and unboxing",
+        category: "Performance",
+        defaultSeverity: DiagnosticSeverity.Info,
+        isEnabledByDefault: true,
+        helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");
+
+    /// <inheritdoc />
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    /// <inheritdoc />
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterCompilationStartAction(compilationStartAnalysisContext =>
+        {
+            INamedTypeSymbol? dictionarySymbol = compilationStartAnalysisContext.Compilation.GetTypeByMetadataName("System.Collections.Generic.Dictionary`2");
+            INamedTypeSymbol? listSymbol = compilationStartAnalysisContext.Compilation.GetTypeByMetadataName("System.Collections.Generic.List`1");
+
+            compilationStartAnalysisContext.RegisterOperationAction(AnalyzeOperation, OperationKind.Conversion);
+            compilationStartAnalysisContext.RegisterSyntaxNodeAction(
+                syntaxNodeContext => AnalyzeNode(syntaxNodeContext, dictionarySymbol, listSymbol),
+                SyntaxKind.ElementAccessExpression);
+        });
+    }
+
+    private static void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol? dictionarySymbol, INamedTypeSymbol? listSymbol)
+    {
+        if (context.Node is not ElementAccessExpressionSyntax elementAccess)
+        {
+            return;
+        }
+
+        // Get the type of the accessed object
+        TypeInfo typeInfo = context.SemanticModel.GetTypeInfo(elementAccess.Expression, context.CancellationToken);
+
+        if (typeInfo.Type is not INamedTypeSymbol { IsGenericType: true } namedType)
+        {
+            return;
+        }
+
+        INamedTypeSymbol baseType = namedType.ConstructedFrom;
+        if (SymbolEqualityComparer.Default.Equals(baseType, dictionarySymbol))
+        {
+            ITypeSymbol keyType = namedType.TypeArguments[0]; // The TKey in Dictionary<TKey, TValue>
+            if (ReportDiagnosticOnValueType(keyType))
+            {
+                return;
+            }
+
+            ITypeSymbol valueType = namedType.TypeArguments[1]; // The TValue in Dictionary<TKey, TValue>
+            if (ReportDiagnosticOnValueType(valueType))
+            {
+                return;
+            }
+        }
+        else if (SymbolEqualityComparer.Default.Equals(baseType, listSymbol))
+        {
+            ITypeSymbol elementType = namedType.TypeArguments[0]; // The T in List<T>
+            if (ReportDiagnosticOnValueType(elementType))
+            {
+                return;
+            }
+        }
+        else
+        {
+            Debug.Fail($"Unrecognized constructed from named type '{baseType}'.");
+        }
+
+        return;
+
+        bool ReportDiagnosticOnValueType(ITypeSymbol? typeSymbol)
+        {
+            // Check if the struct is read/write; if so, there can be bad things that happen to warn
+            if (typeSymbol is not { IsValueType: true, IsReadOnly: false })
+            {
+                return false;
+            }
+
+            // Create and report a diagnostic if the element is accessed directly
+            Diagnostic diagnostic = elementAccess.GetLocation().CreateDiagnostic(Rule, typeSymbol.Name);
+            context.ReportDiagnostic(diagnostic);
+
+            return true;
+        }
+    }
+
+    private static void AnalyzeOperation(OperationAnalysisContext context)
+    {
+        if (context.Operation is IConversionOperation conversionOperation)
+        {
+            AnalyzeConversionOperation(conversionOperation, context);
+        }
+        else
+        {
+            throw new NotSupportedException($"Unsupported operation kind: {context.Operation.Kind}");
+        }
+    }
+
+    private static void AnalyzeConversionOperation(IConversionOperation conversionOperation, OperationAnalysisContext context)
+    {
+        if (conversionOperation.IsBoxingOrUnboxingOperation())
+        {
+            Diagnostic diagnostic = conversionOperation.Syntax.GetLocation().CreateDiagnostic(Rule);
+            context.ReportDiagnostic(diagnostic);
+        }
+    }
+}

src/EffectiveCSharp.Analyzers/SpanAnalyzer.cs

@@ -14,8 +14,8 @@
         messageFormat: "Consider using Span<T> instead of array for better performance",
         category: "Performance",
         defaultSeverity: DiagnosticSeverity.Info,
-        helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md",
-        isEnabledByDefault: true);
+        isEnabledByDefault: true,
+        helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");
 
     /// <inheritdoc />
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
@@ -50,7 +50,7 @@
     private static bool IsInsideSpanInitialization(ArrayCreationExpressionSyntax arrayCreation)
     {
         // Check if the parent is a Span<T> or ReadOnlySpan<T> creation
         // example: new Span<int>(new int[10]);
         if (arrayCreation.Parent?.Parent?.Parent is not ObjectCreationExpressionSyntax objectCreation)
         {
             return false;