Prefer Dictionary<K, V>.TryAdd(key) over guarded Add(key)

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpPreferDictionaryTryAddValueOverGuardedAddFixer.cs

@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Editing;
+using Microsoft.NetCore.Analyzers.Performance;
+
+namespace Microsoft.NetCore.CSharp.Analyzers.Performance
+{
+    public sealed class CSharpPreferDictionaryTryAddValueOverGuardedAddFixer : PreferDictionaryTryAddValueOverGuardedAddFixer
+    {
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            var diagnostic = context.Diagnostics[0];
+            var dictionaryAddLocation = diagnostic.AdditionalLocations[0];
+
+            Document document = context.Document;
+            SyntaxNode root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+
+            var dictionaryAdd = root.FindNode(dictionaryAddLocation.SourceSpan, getInnermostNodeForTie: true);
+            if (dictionaryAdd is not InvocationExpressionSyntax dictionaryAddInvocation
+                || root.FindNode(context.Span) is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax containsKeyAccess } containsKeyInvocation)
+            {
+                return;
+            }
+
+            var action = CodeAction.Create(CodeFixTitle, async ct =>
+            {
+                var editor = await DocumentEditor.CreateAsync(document, ct).ConfigureAwait(false);
+                var generator = editor.Generator;
+
+                var tryAddValueAccess = generator.MemberAccessExpression(containsKeyAccess.Expression, TryAdd);
+                var dictionaryAddArguments = dictionaryAddInvocation.ArgumentList.Arguments;
+                var tryAddInvocation = generator.InvocationExpression(tryAddValueAccess, dictionaryAddArguments[0], dictionaryAddArguments[1]);
+
+                var ifStatement = containsKeyInvocation.FirstAncestorOrSelf<IfStatementSyntax>();
+                if (ifStatement is null)
+                {
+                    return editor.OriginalDocument;
+                }
+
+                if (ifStatement.Condition is PrefixUnaryExpressionSyntax unary && unary.IsKind(SyntaxKind.LogicalNotExpression))
+                {
+                    if (ifStatement.Statement is BlockSyntax { Statements.Count: 1 } or ExpressionStatementSyntax)
+                    {
+                        if (ifStatement.Else is null)
+                        {
+                            // d.Add() is the only statement in the if and is guarded with a !d.ContainsKey().
+                            // Since there is no else-branch, we can replace the entire if-statement with a d.TryAdd() call.
+                            var invocationWithTrivia = tryAddInvocation.WithTriviaFrom(ifStatement);
+                            editor.ReplaceNode(ifStatement, generator.ExpressionStatement(invocationWithTrivia));
+                        }
+                        else
+                        {
+                            // d.Add() is the only statement in the if and is guarded with a !d.ContainsKey().
+                            // In this case, we switch out the !d.ContainsKey() call with a !d.TryAdd() call and move the else-branch into the if.
+                            editor.ReplaceNode(containsKeyInvocation, tryAddInvocation);
+                            editor.ReplaceNode(ifStatement.Statement, ifStatement.Else.Statement);
+                            editor.RemoveNode(ifStatement.Else, SyntaxRemoveOptions.KeepNoTrivia);
+                        }
+                    }
+                    else
+                    {
+                        // d.Add() is one of many statements in the if and is guarded with a !d.ContainsKey().
+                        // In this case, we switch out the !d.ContainsKey() call for a d.TryAdd() call.
+                        editor.RemoveNode(dictionaryAddInvocation.Parent, SyntaxRemoveOptions.KeepNoTrivia);
+                        editor.ReplaceNode(unary, tryAddInvocation);
+                    }
+                }
+                else if (ifStatement.Condition.IsKind(SyntaxKind.InvocationExpression) && ifStatement.Else is not null)
+                {
+                    var negatedTryAddInvocation = generator.LogicalNotExpression(tryAddInvocation);
+                    editor.ReplaceNode(containsKeyInvocation, negatedTryAddInvocation);
+                    if (ifStatement.Else.Statement is BlockSyntax { Statements.Count: 1 } or ExpressionStatementSyntax)
+                    {
+                        // d.Add() is the only statement the else-branch and guarded by a d.ContainsKey() call in the if.
+                        // In this case we replace the d.ContainsKey() call with a !d.TryAdd() call and remove the entire else-branch.
+                        editor.RemoveNode(ifStatement.Else);
+                    }
+                    else
+                    {
+                        // d.Add() is one of many statements in the else-branch and guarded by a d.ContainsKey() call in the if.
+                        // In this case we replace the d.ContainsKey() call with a !d.TryAdd() call and remove the d.Add() call in the else-branch.
+                        editor.RemoveNode(dictionaryAddInvocation.Parent, SyntaxRemoveOptions.KeepNoTrivia);
+                    }
+                }
+
+                return editor.GetChangedDocument();
+            }, CodeFixTitle);
+
+            context.RegisterCodeFix(action, context.Diagnostics);
+        }
+    }
+}

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -12,6 +12,7 @@ CA1856 | Performance | Error | ConstantExpectedAnalyzer, [Documentation](https:/
 CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1857)
 CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
 CA1859 | Performance | Info | UseConcreteTypeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859)
+CA1863 | Performance | Info | PreferDictionaryTryAddValueOverGuardedAddAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1863)
 
 ### Removed Rules
 

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -1998,4 +1998,16 @@
   <data name="UseConcreteTypeForParameterMessage" xml:space="preserve">
     <value>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</value>
   </data>
+  <data name="PreferDictionaryTryAddValueCodeFixTitle" xml:space="preserve">
+    <value>Use 'TryAdd(TKey, TValue)'</value>
+  </data>
+  <data name="PreferDictionaryTryAddTitle" xml:space="preserve">
+    <value>Prefer the 'IDictionary.TryAdd(TKey, TValue)' method</value>
+  </data>
+  <data name="PreferDictionaryTryAddMessage" xml:space="preserve">
+    <value>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check to avoid double lookup</value>
+  </data>
+  <data name="PreferDictionaryTryAddDescription" xml:space="preserve">
+    <value>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check. 'TryAdd' behaves the same as 'Add' except that it doesn't throw an exception when a value for a given key already exists, but rather returns 'false'.</value>
+  </data>
 </root>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/PreferDictionaryTryAddOverGuardedAddAnalyzer.cs

@@ -0,0 +1,145 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+using static Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources;
+
+namespace Microsoft.NetCore.Analyzers.Performance
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+    public sealed class PreferDictionaryTryAddOverGuardedAddAnalyzer : DiagnosticAnalyzer
+    {
+        internal const string RuleId = "CA1863";
+
+        private const string ContainsKey = nameof(IDictionary<dynamic, dynamic>.ContainsKey);
+        private const string Add = nameof(IDictionary<dynamic, dynamic>.Add);
+        private const string TryAdd = nameof(TryAdd);
+
+        private static readonly DiagnosticDescriptor DiagnosticDescriptor = DiagnosticDescriptorHelper.Create(
+            RuleId,
+            CreateLocalizableResourceString(nameof(PreferDictionaryTryAddTitle)),
+            CreateLocalizableResourceString(nameof(PreferDictionaryTryAddMessage)),
+            DiagnosticCategory.Performance,
+            RuleLevel.IdeSuggestion,
+            CreateLocalizableResourceString(nameof(PreferDictionaryTryAddDescription)),
+            isPortedFxCopRule: false,
+            isDataflowRule: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagnosticDescriptor);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.EnableConcurrentExecution();
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.RegisterCompilationStartAction(OnCompilationStart);
+        }
+
+        private static void OnCompilationStart(CompilationStartAnalysisContext context)
+        {
+            if (!TryGetDictionaryTypeAndMembers(context.Compilation, out var iDictionaryType, out var containsKeySymbol, out var addSymbol))
+            {
+                return;
+            }
+
+            context.RegisterOperationAction(ctx => OnOperationAnalysis(iDictionaryType, containsKeySymbol, addSymbol, ctx), OperationKind.Conditional);
+        }
+
+        private static void OnOperationAnalysis(INamedTypeSymbol iDictionaryType, IMethodSymbol containsKeySymbol, IMethodSymbol addSymbol, OperationAnalysisContext context)
+        {
+            var conditional = (IConditionalOperation)context.Operation;
+            var (invocation, branch) = conditional.Condition switch
+            {
+                IInvocationOperation i => (i, conditional.WhenFalse),
+                IUnaryOperation { OperatorKind: UnaryOperatorKind.Not, Operand: IInvocationOperation i } => (i, conditional.WhenTrue),
+                _ => (null, null)
+            };
+            if (invocation is not null
+                && IsValidDictionaryType(invocation.TargetMethod.ContainingType, iDictionaryType, invocation.SemanticModel)
+                && DoesSignatureMatch(invocation.TargetMethod, containsKeySymbol)
+                && TryGetEligibleDictionaryAddLocation(branch, invocation.Instance, invocation.Arguments[0].Value, addSymbol, out var dictionaryAddLocation))
+            {
+                var additionalLocations = ImmutableArray.Create(dictionaryAddLocation);
+                context.ReportDiagnostic(invocation.CreateDiagnostic(DiagnosticDescriptor, additionalLocations, null));
+            }
+        }
+
+        private static bool TryGetEligibleDictionaryAddLocation(IOperation? conditionalBody, IOperation dictionaryInstance, IOperation containsKeyArgument, IMethodSymbol addSymbol,
+            [NotNullWhen(true)]
+            out Location? dictionaryAddLocation)
+        {
+            dictionaryAddLocation = null;
+            if (conditionalBody is null)
+            {
+                return false;
+            }
+
+            if (conditionalBody is IBlockOperation block)
+            {
+                dictionaryAddLocation = block.Operations.OfType<IExpressionStatementOperation>().FirstOrDefault(e => e.Operation is IInvocationOperation i && IsEligibleDictionaryAddInvocation(i))
+                    ?.Operation.Syntax.GetLocation();
+            }
+            else if (conditionalBody is IExpressionStatementOperation { Operation: IInvocationOperation iOperation } && IsEligibleDictionaryAddInvocation(iOperation))
+            {
+                dictionaryAddLocation = iOperation.Syntax.GetLocation();
+            }
+
+            bool IsEligibleDictionaryAddInvocation(IInvocationOperation invocation)
+            {
+                return SymbolEqualityComparer.Default.Equals(invocation.Instance.GetSymbolFromReference(), dictionaryInstance.GetSymbolFromReference())
+                       && DoesSignatureMatch(invocation.TargetMethod, addSymbol)
+                       && invocation.Arguments[0].Value.Syntax.IsEquivalentTo(containsKeyArgument.Syntax)
+                       && invocation.Arguments[1].Value.Kind == OperationKind.Literal;
+            }
+
+            return dictionaryAddLocation is not null;
+        }
+
+        private static bool TryGetDictionaryTypeAndMembers(
+            Compilation compilation,
+            [NotNullWhen(true)]
+            out INamedTypeSymbol? iDictionaryType,
+            [NotNullWhen(true)]
+            out IMethodSymbol? containsKeySymbol,
+            [NotNullWhen(true)]
+            out IMethodSymbol? addSymbol)
+        {
+            iDictionaryType = WellKnownTypeProvider.GetOrCreate(compilation).GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericIDictionary2);
+            if (iDictionaryType is null)
+            {
+                containsKeySymbol = null;
+                addSymbol = null;
+
+                return false;
+            }
+
+            containsKeySymbol = iDictionaryType.GetMembers(ContainsKey).OfType<IMethodSymbol>().FirstOrDefault();
+            addSymbol = iDictionaryType.GetMembers(Add).OfType<IMethodSymbol>().FirstOrDefault();
+
+            return containsKeySymbol is not null && addSymbol is not null;
+        }
+
+        private static bool IsValidDictionaryType(INamedTypeSymbol suspectedDictionaryType, ISymbol iDictionaryType, SemanticModel semanticModel)
+        {
+            // Either the type is the IDictionary or it is a type which (indirectly) implements it.
+            // If the type does not have a TryAdd() method (e.g. netstandard), we return false.
+            return (suspectedDictionaryType.OriginalDefinition.Equals(iDictionaryType, SymbolEqualityComparer.Default)
+                    || suspectedDictionaryType.AllInterfaces.Any((@interface, dictionary) => @interface.OriginalDefinition.Equals(dictionary, SymbolEqualityComparer.Default), iDictionaryType))
+                   && semanticModel.LookupSymbols(0, suspectedDictionaryType, TryAdd, true).Length > 0;
+        }
+
+        private static bool DoesSignatureMatch(IMethodSymbol suspected, IMethodSymbol comparator)
+        {
+            return suspected.OriginalDefinition.ReturnType.Name == comparator.ReturnType.Name
+                   && suspected.Name == comparator.Name
+                   && suspected.Parameters.Length == comparator.Parameters.Length
+                   && suspected.Parameters.Zip(comparator.Parameters, (p1, p2) => p1.OriginalDefinition.Type.Name == p2.Type.Name).All(isParameterEqual => isParameterEqual);
+        }
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/PreferDictionaryTryAddValueOverGuardedAdd.Fixer.cs

@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Microsoft.NetCore.Analyzers.Performance
+{
+    public abstract class PreferDictionaryTryAddValueOverGuardedAddFixer : CodeFixProvider
+    {
+        protected const string TryAdd = nameof(TryAdd);
+
+        protected static string CodeFixTitle => MicrosoftNetCoreAnalyzersResources.PreferDictionaryTryAddValueCodeFixTitle;
+
+        public override FixAllProvider GetFixAllProvider()
+        {
+            return WellKnownFixAllProviders.BatchFixer;
+        }
+
+        public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(PreferDictionaryTryAddOverGuardedAddAnalyzer.RuleId);
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf

@@ -1967,6 +1967,26 @@
         <target state="translated">Pro typ slovníku{0} preferovat ContainsValue před Values.Contains</target>
         <note />
       </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddDescription">
+        <source>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check. 'TryAdd' behaves the same as 'Add' except that it doesn't throw an exception when a value for a given key already exists, but rather returns 'false'.</source>
+        <target state="new">Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check. 'TryAdd' behaves the same as 'Add' except that it doesn't throw an exception when a value for a given key already exists, but rather returns 'false'.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddMessage">
+        <source>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check to avoid double lookup</source>
+        <target state="new">Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check to avoid double lookup</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddTitle">
+        <source>Prefer the 'IDictionary.TryAdd(TKey, TValue)' method</source>
+        <target state="new">Prefer the 'IDictionary.TryAdd(TKey, TValue)' method</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddValueCodeFixTitle">
+        <source>Use 'TryAdd(TKey, TValue)'</source>
+        <target state="new">Use 'TryAdd(TKey, TValue)'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PreferHashDataCodefixTitle">
         <source>Replace with 'HashData' method</source>
         <target state="translated">Nahraďte metodou HashData.</target>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf

@@ -1967,6 +1967,26 @@
         <target state="translated">Bevorzugen Sie "ContainsValue" gegenüber "Values.Contains" für den Wörterbuchtyp "{0}".</target>
         <note />
       </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddDescription">
+        <source>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check. 'TryAdd' behaves the same as 'Add' except that it doesn't throw an exception when a value for a given key already exists, but rather returns 'false'.</source>
+        <target state="new">Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check. 'TryAdd' behaves the same as 'Add' except that it doesn't throw an exception when a value for a given key already exists, but rather returns 'false'.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddMessage">
+        <source>Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check to avoid double lookup</source>
+        <target state="new">Prefer a 'TryAdd' call over an 'Add' call guarded by a 'ContainsKey' check to avoid double lookup</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddTitle">
+        <source>Prefer the 'IDictionary.TryAdd(TKey, TValue)' method</source>
+        <target state="new">Prefer the 'IDictionary.TryAdd(TKey, TValue)' method</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="PreferDictionaryTryAddValueCodeFixTitle">
+        <source>Use 'TryAdd(TKey, TValue)'</source>
+        <target state="new">Use 'TryAdd(TKey, TValue)'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PreferHashDataCodefixTitle">
         <source>Replace with 'HashData' method</source>
         <target state="translated">Ersetzen Sie dies durch die Methode „HashData“.</target>