Fixed CA1854: Prefer IDictionary.TryGetValue(TKey, out TValue) occurr…

…ences (#66515)

* fixed CA1854 occurrences c#

* fixed CA1854 occurrences VB

* VB init with Nothing + fix formatting

* Address PR feedback

* Update src/Workspaces/Core/Portable/Rename/ConflictEngine/RenamedSpansTracker.cs

Co-authored-by: Jan Jones <[email protected]>

* remove redundant ifs

* Apply suggestions from code review

Co-authored-by: CyrusNajmabadi <[email protected]>

---------

Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: CyrusNajmabadi <[email protected]>

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs

@@ -468,7 +468,9 @@ public PossiblyConditionalState Clone()
                                                 // The expression in the tuple could not be assigned a slot (for example, `a?.b`),
                                                 // so we had to create a slot for the tuple element instead.
                                                 // We'll remember that so that we can apply any learnings to the expression.
+#pragma warning disable CA1854 //Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup
                                                 if (!originalInputMap.ContainsKey(outputSlot))
+#pragma warning restore CA1854
                                                 {
                                                     originalInputMap.Add(outputSlot,
                                                         ((BoundTupleExpression)expression).Arguments[originalTupleElement.TupleElementIndex]);

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs

@@ -2200,7 +2200,9 @@ private void CheckIndexerSignatureCollisions(
 
                 if (Locations.Length == 1 || IsPartial)
                 {
+#pragma warning disable CA1854 //Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup
                     if (membersByName.ContainsKey(indexerName.AsMemory()))
+#pragma warning restore CA1854
                     {
                         // The name of the indexer is reserved - it can only be used by other indexers.
                         Debug.Assert(!membersByName[indexerName.AsMemory()].Any(SymbolExtensions.IsIndexer));

src/Compilers/Core/CodeAnalysisTest/CachingLookupTests.cs

@@ -161,20 +161,20 @@ public void CachingLookupCorrectResults()
 
             var look1 = CreateLookup(numbers, false);
             var look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, false, comparer: c), comparer);
             CompareLookups1(look1, look2, Keys(numbers, false, comparer));
 
             look1 = CreateLookup(numbers, false);
             look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, false, comparer: c), comparer);
             CompareLookups2(look1, look2, Keys(numbers, false, comparer));
             CompareLookups1(look1, look2, Keys(numbers, false, comparer));
 
             look1 = CreateLookup(numbers, false);
             look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, false, comparer: c), comparer);
             CompareLookups2(look2, look1, Keys(numbers, false, comparer));
             CompareLookups1(look1, look2, Keys(numbers, false, comparer));
@@ -193,20 +193,20 @@ public void CachingLookupCaseInsensitive()
 
             var look1 = CreateLookup(numbers, true);
             var look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
 
             look1 = CreateLookup(numbers, true);
             look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups2(look1, look2, Keys(numbers, true, comparer));
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
 
             look1 = CreateLookup(numbers, true);
             look2 = new CachingDictionary<string, int>(
-                s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+                s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                 (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups2(look2, look1, Keys(numbers, true, comparer));
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
@@ -224,18 +224,18 @@ public void CachingLookupCaseInsensitiveNoCacheMissingKeys()
             }
 
             var look1 = CreateLookup(numbers, true);
-            var look2 = new CachingDictionary<string, int>(s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+            var look2 = new CachingDictionary<string, int>(s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                                                                         (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
 
             look1 = CreateLookup(numbers, true);
-            look2 = new CachingDictionary<string, int>(s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+            look2 = new CachingDictionary<string, int>(s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                                                    (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups2(look1, look2, Keys(numbers, true, comparer));
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
 
             look1 = CreateLookup(numbers, true);
-            look2 = new CachingDictionary<string, int>(s => dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>(),
+            look2 = new CachingDictionary<string, int>(s => dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>(),
                                                    (c) => Keys(numbers, true, comparer: c), comparer);
             CompareLookups2(look2, look1, Keys(numbers, true, comparer));
             CompareLookups1(look1, look2, Keys(numbers, true, comparer));
@@ -260,7 +260,7 @@ public void CallExactlyOncePerKey()
             {
                 Assert.False(lookedUp.Contains(s));
                 lookedUp.Add(s);
-                return dict.ContainsKey(s) ? dict[s] : ImmutableArray.Create<int>();
+                return dict.TryGetValue(s, out var value) ? value : ImmutableArray.Create<int>();
             },
                  (c) =>
             {

src/Compilers/Core/CodeAnalysisTest/Collections/List/IEnumerable.Generic.Tests.cs

@@ -586,15 +586,15 @@ public void IEnumerable_Generic_Enumerator_Current_ReturnsSameObjectsOnDifferent
             // Ensures that the elements returned from enumeration are exactly the same collection of
             // elements returned from a previous enumeration
             IEnumerable<T> enumerable = GenericIEnumerableFactory(count);
-            Dictionary<T, int> firstValues = new Dictionary<T, int>(count);
-            Dictionary<T, int> secondValues = new Dictionary<T, int>(count);
+            HashSet<T> firstValues = new HashSet<T>(count);
+            HashSet<T> secondValues = new HashSet<T>(count);
             foreach (T item in enumerable)
-                firstValues[item] = firstValues.ContainsKey(item) ? firstValues[item]++ : 1;
+                Assert.True(firstValues.Add(item));
             foreach (T item in enumerable)
-                secondValues[item] = secondValues.ContainsKey(item) ? secondValues[item]++ : 1;
+                Assert.True(secondValues.Add(item));
             Assert.Equal(firstValues.Count, secondValues.Count);
-            foreach (T key in firstValues.Keys)
-                Assert.Equal(firstValues[key], secondValues[key]);
+            foreach (T item in firstValues)
+                Assert.True(secondValues.Contains(item));
         }
 
         [Theory]

src/Compilers/Core/CodeAnalysisTest/Collections/List/IEnumerable.NonGeneric.Tests.cs

@@ -216,15 +216,15 @@ public void IEnumerable_NonGeneric_Enumerator_Current_ReturnsSameObjectsOnDiffer
             // Ensures that the elements returned from enumeration are exactly the same collection of
             // elements returned from a previous enumeration
             IEnumerable enumerable = NonGenericIEnumerableFactory(count);
-            Dictionary<object, int> firstValues = new Dictionary<object, int>(count);
-            Dictionary<object, int> secondValues = new Dictionary<object, int>(count);
+            HashSet<object> firstValues = new HashSet<object>(count);
+            HashSet<object> secondValues = new HashSet<object>(count);
             foreach (object item in enumerable)
-                firstValues[item] = firstValues.ContainsKey(item) ? firstValues[item]++ : 1;
+                Assert.True(firstValues.Add(item));
             foreach (object item in enumerable)
-                secondValues[item] = secondValues.ContainsKey(item) ? secondValues[item]++ : 1;
+                Assert.True(secondValues.Add(item));
             Assert.Equal(firstValues.Count, secondValues.Count);
-            foreach (object key in firstValues.Keys)
-                Assert.Equal(firstValues[key], secondValues[key]);
+            foreach (object item in firstValues)
+                Assert.True(secondValues.Contains(item));
         }
 
         [Theory]

src/Compilers/Core/MSBuildTaskTests/TestUtilities/MSBuildUtil.cs

@@ -33,7 +33,7 @@ public static ITaskItem CreateTaskItem(string fileName, Dictionary<string, strin
             taskItem.Setup(x => x.GetMetadata(It.IsAny<string>())).Returns<string>(s => s switch
             {
                 "FullPath" => fileName,
-                _ => metadata.ContainsKey(s) ? metadata[s] : string.Empty
+                _ => metadata.TryGetValue(s, out var value) ? value : string.Empty
             });
             return taskItem.Object;
         }

src/Compilers/Test/Core/Compilation/OperationTreeVerifier.cs

@@ -279,12 +279,12 @@ private void LogType(ITypeSymbol type, string header = "Type")
 
         private uint GetLabelId(ILabelSymbol symbol)
         {
-            if (_labelIdMap.ContainsKey(symbol))
+            if (_labelIdMap.TryGetValue(symbol, out var id))
             {
-                return _labelIdMap[symbol];
+                return id;
             }
 
-            var id = _currentLabelId++;
+            id = _currentLabelId++;
             _labelIdMap[symbol] = id;
             return id;
         }

src/Compilers/Test/Utilities/VisualBasic/ParserTestUtilities.vb

@@ -1025,16 +1025,19 @@ Public Module VerificationHelpers
 
         Public Sub IncrementTypeCounter(Node As VisualBasicSyntaxNode, NodeKey As String)
             _Items.Add(Node)
-            If _Dict.ContainsKey(NodeKey) Then
-                _Dict(NodeKey) = _Dict(NodeKey) + 1 'Increment Count
+
+            Dim count As Integer = Nothing
+            If _Dict.TryGetValue(NodeKey, count) Then
+                _Dict(NodeKey) = count + 1 'Increment Count
             Else
                 _Dict.Add(NodeKey, 1) ' New Item
             End If
         End Sub
 
         Public Function GetCount(Node As String) As Integer
-            If _Dict.ContainsKey(Node) Then
-                Return _Dict(Node)
+            Dim count As Integer = Nothing
+            If _Dict.TryGetValue(Node, count) Then
+                Return count
             Else
                 Return 0
             End If

src/EditorFeatures/Core/InlineRename/InlineRenameSession.OpenTextBufferManager.cs

@@ -453,9 +453,8 @@ internal void ApplyConflictResolutionEdits(IInlineRenameReplacementInfo conflict
 
                         // Show merge conflicts comments as unresolvable conflicts, and do not
                         // show any other rename-related spans that overlap a merge conflict comment.
-                        var mergeConflictComments = mergeResult.MergeConflictCommentSpans.ContainsKey(document.Id)
-                            ? mergeResult.MergeConflictCommentSpans[document.Id]
-                            : SpecializedCollections.EmptyEnumerable<TextSpan>();
+                        mergeResult.MergeConflictCommentSpans.TryGetValue(document.Id, out var mergeConflictComments);
+                        mergeConflictComments ??= SpecializedCollections.EmptyEnumerable<TextSpan>();
 
                         foreach (var conflict in mergeConflictComments)
                         {
@@ -737,8 +736,8 @@ public void Dispose()
                 private SnapshotPoint GetNewEndpoint(TextSpan span)
                 {
                     var snapshot = _openTextBufferManager._subjectBuffer.CurrentSnapshot;
-                    var endPoint = _openTextBufferManager._referenceSpanToLinkedRenameSpanMap.ContainsKey(span)
-                        ? _openTextBufferManager._referenceSpanToLinkedRenameSpanMap[span].TrackingSpan.GetEndPoint(snapshot)
+                    var endPoint = _openTextBufferManager._referenceSpanToLinkedRenameSpanMap.TryGetValue(span, out var renameTrackingSpan)
+                        ? renameTrackingSpan.TrackingSpan.GetEndPoint(snapshot)
                         : _openTextBufferManager._referenceSpanToLinkedRenameSpanMap.First(kvp => kvp.Key.OverlapsWith(span)).Value.TrackingSpan.GetEndPoint(snapshot);
                     return _openTextBufferManager.ActiveTextView.BufferGraph.MapUpToBuffer(endPoint, PointTrackingMode.Positive, PositionAffinity.Successor, _openTextBufferManager.ActiveTextView.TextBuffer).Value;
                 }

src/EditorFeatures/Test2/Expansion/AbstractExpansionTest.vb

@@ -2,6 +2,7 @@
 ' The .NET Foundation licenses this file to you under the MIT license.
 ' See the LICENSE file in the project root for more information.
 
+Imports System.Collections.Immutable
 Imports System.Threading
 Imports Microsoft.CodeAnalysis
 Imports Microsoft.CodeAnalysis.CodeActions
@@ -10,6 +11,7 @@ Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
 Imports Microsoft.CodeAnalysis.Formatting
 Imports Microsoft.CodeAnalysis.Simplification
+Imports Microsoft.CodeAnalysis.Text
 Imports Roslyn.Utilities
 
 Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Expansion
@@ -30,13 +32,14 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Expansion
 
                 Dim cleanupOptions = CodeCleanupOptions.GetDefault(document.Project.Services)
 
-                If hostDocument.AnnotatedSpans.ContainsKey("Expand") Then
-                    For Each span In hostDocument.AnnotatedSpans("Expand")
+                Dim spans As ImmutableArray(Of TextSpan) = Nothing
+                If hostDocument.AnnotatedSpans.TryGetValue("Expand", spans) Then
+                    For Each span In spans
                         Dim node = GetExpressionSyntaxWithSameSpan(root.FindToken(span.Start).Parent, span.End)
                         root = root.ReplaceNode(node, Await Simplifier.ExpandAsync(node, document, expandInsideNode:=Nothing, expandParameter:=expandParameter))
                     Next
-                ElseIf hostDocument.AnnotatedSpans.ContainsKey("ExpandAndSimplify") Then
-                    For Each span In hostDocument.AnnotatedSpans("ExpandAndSimplify")
+                ElseIf hostDocument.AnnotatedSpans.TryGetValue("ExpandAndSimplify", spans) Then
+                    For Each span In spans
                         Dim node = GetExpressionSyntaxWithSameSpan(root.FindToken(span.Start).Parent, span.End)
                         root = root.ReplaceNode(node, Await Simplifier.ExpandAsync(node, document, expandInsideNode:=Nothing, expandParameter:=expandParameter))
                         document = document.WithSyntaxRoot(root)

src/EditorFeatures/Test2/FindReferences/FindReferencesTests.vb

@@ -312,8 +312,8 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.FindReferences
                     Dim documentsWithAnnotatedSpans = workspace.Documents.Where(Function(d) d.AnnotatedSpans.Any())
                     Assert.Equal(Of String)(documentsWithAnnotatedSpans.Select(Function(d) GetFilePathAndProjectLabel(d)).Order(), actualDefinitions.Keys.Order())
                     For Each doc In documentsWithAnnotatedSpans
-
-                        Dim expected = If(doc.AnnotatedSpans.ContainsKey(DefinitionKey), doc.AnnotatedSpans(DefinitionKey), ImmutableArray(Of TextSpan).Empty).Order()
+                        Dim spans As ImmutableArray(Of TextSpan) = Nothing
+                        Dim expected = If(doc.AnnotatedSpans.TryGetValue(DefinitionKey, spans), spans, ImmutableArray(Of TextSpan).Empty).Order()
                         Dim actual = actualDefinitions(GetFilePathAndProjectLabel(doc)).Order()
 
                         If Not TextSpansMatch(expected, actual) Then

src/EditorFeatures/Test2/LanguageServices/SyntaxFactsServiceTests.vb

@@ -2,6 +2,7 @@
 ' The .NET Foundation licenses this file to you under the MIT license.
 ' See the LICENSE file in the project root for more information.
 
+Imports System.Collections.Immutable
 Imports System.Threading
 Imports System.Threading.Tasks
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
@@ -212,7 +213,8 @@ $$End Class
                 Dim node = root.FindNode(New TextSpan(cursorPosition, 0))
                 Dim syntaxFactsService = document.GetLanguageService(Of ISyntaxFactsService)()
 
-                Dim expected = If(cursorDocument.AnnotatedSpans.ContainsKey("MemberBodySpan"), cursorDocument.AnnotatedSpans!MemberBodySpan.Single(), Nothing)
+                Dim spans As ImmutableArray(Of TextSpan) = Nothing
+                Dim expected = If(cursorDocument.AnnotatedSpans.TryGetValue("MemberBodySpan", spans), spans.Single(), Nothing)
                 Dim actual = syntaxFactsService.GetMemberBodySpanForSpeculativeBinding(node)
 
                 Assert.Equal(expected, actual)

src/EditorFeatures/Test2/Rename/RenameEngineResult.vb

@@ -2,6 +2,7 @@
 ' The .NET Foundation licenses this file to you under the MIT license.
 ' See the LICENSE file in the project root for more information.
 
+Imports System.Collections.Immutable
 Imports System.Threading
 Imports Microsoft.CodeAnalysis
 Imports Microsoft.CodeAnalysis.CodeActions
@@ -12,6 +13,7 @@ Imports Microsoft.CodeAnalysis.Options
 Imports Microsoft.CodeAnalysis.Remote.Testing
 Imports Microsoft.CodeAnalysis.Rename
 Imports Microsoft.CodeAnalysis.Rename.ConflictEngine
+Imports Microsoft.CodeAnalysis.Text
 Imports Microsoft.VisualStudio.Text
 Imports Xunit.Abstractions
 Imports Xunit.Sdk
@@ -197,10 +199,11 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
             For Each document In _workspace.Documents
                 Dim annotatedSpans = document.AnnotatedSpans
 
-                If annotatedSpans.ContainsKey(label) Then
+                Dim spans As ImmutableArray(Of TextSpan) = Nothing
+                If annotatedSpans.TryGetValue(label, spans) Then
                     Dim syntaxTree = _workspace.CurrentSolution.GetDocument(document.Id).GetSyntaxTreeAsync().Result
 
-                    For Each span In annotatedSpans(label)
+                    For Each span In spans
                         locations.Add(syntaxTree.GetLocation(span))
                     Next
                 End If

src/EditorFeatures/TestUtilities/Workspaces/TestHostDocument.cs

@@ -138,9 +138,9 @@ internal TestHostDocument(
             _documentServiceProvider = documentServiceProvider;
             _roles = roles.IsDefault ? s_defaultRoles : roles;
 
-            if (spans.ContainsKey(string.Empty))
+            if (spans.TryGetValue(string.Empty, out var textSpans))
             {
-                this.SelectedSpans = spans[string.Empty];
+                this.SelectedSpans = textSpans;
             }
 
             foreach (var namedSpanList in spans.Where(s => s.Key != string.Empty))