From 3dac4bc7700e687a812cd4ad6348c166bd7c372f Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sun, 23 Jul 2023 23:41:31 +0200 Subject: [PATCH 1/3] Remove unnecessary calls to Contains for sets This commit removes calls to Contains if they are immediately followed by either Add or Remove. --- .../Collections/HashSet/ISet_Generic_Tests`1.cs | 3 +-- .../Core/Portable/MetadataReader/MetadataDecoder.cs | 9 ++------- .../Symbols/Source/SourceMemberContainerTypeSymbol.vb | 5 ++--- .../CodeDefinitionWindow/DefinitionContextTracker.cs | 3 +-- .../ChangeSignature/ChangeSignatureDialogViewModel.cs | 6 +----- .../Core/Def/Interop/CleanableWeakComHandleTable.cs | 5 +---- .../Impl/ProjectSystemShim/VisualBasicProject.vb | 4 +--- 7 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/HashSet/ISet_Generic_Tests`1.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/HashSet/ISet_Generic_Tests`1.cs index 5e770c6ce0170..a65d2f96bc642 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/HashSet/ISet_Generic_Tests`1.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/HashSet/ISet_Generic_Tests`1.cs @@ -592,8 +592,7 @@ public void ISet_Generic_SymmetricExceptWith_AfterRemovingElements(EnumerableTyp { ISet set = GenericISetFactory(setLength); T value = CreateT(532); - if (!set.Contains(value)) - set.Add(value); + set.Add(value); set.Remove(value); IEnumerable enumerable = CreateEnumerable(enumerableType, set, enumerableLength, numberOfMatchingElements, numberOfDuplicateElements); Debug.Assert(enumerable != null); diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index eb3ac8cb24285..a6e4b8309331f 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -2070,10 +2070,8 @@ private MethodSymbol FindMethodSymbolInSuperType(TypeDefinitionHandle searchType TypeDefinitionHandle typeDef = typeDefsToSearch.Dequeue(); Debug.Assert(!typeDef.IsNil); - if (!visitedTypeDefTokens.Contains(typeDef)) + if (visitedTypeDefTokens.Add(typeDef)) { - visitedTypeDefTokens.Add(typeDef); - foreach (MethodDefinitionHandle methodDef in Module.GetMethodsOfTypeOrThrow(typeDef)) { if (methodDef == targetMethodDef) @@ -2091,12 +2089,9 @@ private MethodSymbol FindMethodSymbolInSuperType(TypeDefinitionHandle searchType TypeSymbol typeSymbol = typeSymbolsToSearch.Dequeue(); Debug.Assert(typeSymbol != null); - if (!visitedTypeSymbols.Contains(typeSymbol)) + if (visitedTypeSymbols.Add(typeSymbol)) { - visitedTypeSymbols.Add(typeSymbol); - //we're looking for a method def but we're currently on a type *ref*, so just enqueue supertypes - EnqueueTypeSymbolInterfacesAndBaseTypes(typeDefsToSearch, typeSymbolsToSearch, typeSymbol); } } diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb index 54541fa138628..04af2a1f7b5d5 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb @@ -1802,9 +1802,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Dim candidateLocation As Location = candidate.NonMergedLocation Debug.Assert(candidateLocation IsNot Nothing) - If partialMethods.Contains(candidate) Then - ' partial-partial conflict - partialMethods.Remove(candidate) + ' partial-partial conflict + If partialMethods.Remove(candidate) Then ' the 'best' partial method is the one with the 'smallest' ' location, we should report errors on the other diff --git a/src/EditorFeatures/Core/CodeDefinitionWindow/DefinitionContextTracker.cs b/src/EditorFeatures/Core/CodeDefinitionWindow/DefinitionContextTracker.cs index 19aa882a8d3dd..7ccf2d68b973e 100644 --- a/src/EditorFeatures/Core/CodeDefinitionWindow/DefinitionContextTracker.cs +++ b/src/EditorFeatures/Core/CodeDefinitionWindow/DefinitionContextTracker.cs @@ -77,9 +77,8 @@ void ITextViewConnectionListener.SubjectBuffersDisconnected(ITextView textView, if (reason == ConnectionReason.TextViewLifetime || !textView.BufferGraph.GetTextBuffers(b => b.ContentType.IsOfType(ContentTypeNames.RoslynContentType)).Any()) { - if (_subscribedViews.Contains(textView)) + if (_subscribedViews.Remove(textView)) { - _subscribedViews.Remove(textView); textView.Caret.PositionChanged -= OnTextViewCaretPositionChanged; } } diff --git a/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureDialogViewModel.cs b/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureDialogViewModel.cs index 297d5929cf1c9..646ff72b96615 100644 --- a/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureDialogViewModel.cs +++ b/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureDialogViewModel.cs @@ -241,11 +241,7 @@ internal void Remove() { var parameterToRemove = AllParameters[_selectedIndex!.Value]; - if (_parametersWithoutDefaultValues.Contains(parameterToRemove)) - { - _parametersWithoutDefaultValues.Remove(parameterToRemove); - } - else + if (!_parametersWithoutDefaultValues.Remove(parameterToRemove)) { _parametersWithDefaultValues.Remove(parameterToRemove); } diff --git a/src/VisualStudio/Core/Def/Interop/CleanableWeakComHandleTable.cs b/src/VisualStudio/Core/Def/Interop/CleanableWeakComHandleTable.cs index e3deb808f774d..24c4d9f146b1c 100644 --- a/src/VisualStudio/Core/Def/Interop/CleanableWeakComHandleTable.cs +++ b/src/VisualStudio/Core/Def/Interop/CleanableWeakComHandleTable.cs @@ -170,10 +170,7 @@ public TValue Remove(TKey key) { this.AssertIsForeground(); - if (_deadKeySet.Contains(key)) - { - _deadKeySet.Remove(key); - } + _deadKeySet.Remove(key); if (_table.TryGetValue(key, out var handle)) { diff --git a/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb b/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb index 438dcbbf2bf72..bbea3285c113a 100644 --- a/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb +++ b/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb @@ -338,9 +338,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.ProjectSystemShim ' To keep things simple, we'll just remove everything and add everything back in For Each oldRuntimeLibrary In oldRuntimeLibraries ' If this one was added explicitly in addition to our computation, we don't have to remove it - If _explicitlyAddedRuntimeLibraries.Contains(oldRuntimeLibrary) Then - _explicitlyAddedRuntimeLibraries.Remove(oldRuntimeLibrary) - Else + If Not _explicitlyAddedRuntimeLibraries.Remove(oldRuntimeLibrary) Then ProjectSystemProject.RemoveMetadataReference(oldRuntimeLibrary, MetadataReferenceProperties.Assembly) End If Next From 28f42ef0063ee7a30ac421a352a9c1e11a101f52 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Mon, 24 Jul 2023 23:15:40 +0200 Subject: [PATCH 2/3] Move comment back inside If --- .../Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb index 04af2a1f7b5d5..867e7eabf15a8 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMemberContainerTypeSymbol.vb @@ -1802,8 +1802,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Dim candidateLocation As Location = candidate.NonMergedLocation Debug.Assert(candidateLocation IsNot Nothing) - ' partial-partial conflict If partialMethods.Remove(candidate) Then + ' partial-partial conflict ' the 'best' partial method is the one with the 'smallest' ' location, we should report errors on the other From 98a0ae02371ac4d1eb20bc028ac27036223ec2a3 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Tue, 25 Jul 2023 18:55:45 +0200 Subject: [PATCH 3/3] Enable CA1865: Unnecessary call to Contains --- eng/config/globalconfigs/Common.globalconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/config/globalconfigs/Common.globalconfig b/eng/config/globalconfigs/Common.globalconfig index 58113a0c8c411..e6b14a88a32e0 100644 --- a/eng/config/globalconfigs/Common.globalconfig +++ b/eng/config/globalconfigs/Common.globalconfig @@ -4,6 +4,7 @@ dotnet_diagnostic.CA1067.severity = warning dotnet_diagnostic.CA1068.severity = warning dotnet_diagnostic.CA1200.severity = warning dotnet_diagnostic.CA1821.severity = warning +dotnet_diagnostic.CA1865.severity = warning dotnet_diagnostic.CA2009.severity = warning # CA2012: Use ValueTasks correctly