Enable new analyzer CA1868: 'Unnecessary call to 'Contains' for sets'…

… and fix findings (#89652)
dotnet · Jul 29, 2023 · 4ed3550 · 4ed3550
1 parent d5c4a4e
commit 4ed3550
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 19 deletions.
diff --git a/eng/CodeAnalysis.src.globalconfig b/eng/CodeAnalysis.src.globalconfig
@@ -483,6 +483,9 @@ dotnet_diagnostic.CA1863.severity = suggestion
 # CA1864: Prefer the 'IDictionary.TryAdd(TKey, TValue)' method
 dotnet_diagnostic.CA1864.severity = warning
 
+# CA1868: Unnecessary call to 'Contains' for sets
+dotnet_diagnostic.CA1868.severity = warning
+
 # CA2000: Dispose objects before losing scope
 dotnet_diagnostic.CA2000.severity = none
 

diff --git a/eng/CodeAnalysis.test.globalconfig b/eng/CodeAnalysis.test.globalconfig
@@ -480,6 +480,9 @@ dotnet_diagnostic.CA1863.severity = none
 # CA1864: Prefer the 'IDictionary.TryAdd(TKey, TValue)' method
 dotnet_diagnostic.CA1864.severity = none
 
+# CA1868: Unnecessary call to 'Contains' for sets
+dotnet_diagnostic.CA1868.severity = none
+
 # CA2000: Dispose objects before losing scope
 dotnet_diagnostic.CA2000.severity = none
 

diff --git a/...lr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ModuleInitializerListNode.cs b/...lr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ModuleInitializerListNode.cs
@@ -113,9 +113,8 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
                 {
                     foreach (var module in allModules)
                     {
-                        if (!markedModules.Contains(module))
+                        if (markedModules.Add(module))
                         {
-                            markedModules.Add(module);
                             if (modulesWithCctor.Contains(module.Module))
                                 sortedModules.Add(module.Module);
                             break;

diff --git a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DgmlWriter.cs b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DgmlWriter.cs
@@ -158,10 +158,8 @@ void IDependencyAnalyzerLogNodeVisitor<DependencyContextType>.VisitCombinedNode(
         void IDependencyAnalyzerLogEdgeVisitor<DependencyContextType>.VisitEdge(DependencyNodeCore<DependencyContextType> nodeDepender, DependencyNodeCore<DependencyContextType> nodeDependerOther, DependencyNodeCore<DependencyContextType> nodeDependedOn, string reason)
         {
             var combinedNode = new Tuple<DependencyNodeCore<DependencyContextType>, DependencyNodeCore<DependencyContextType>>(nodeDepender, nodeDependerOther);
-            if (!_combinedNodesEdgeVisited.Contains(combinedNode))
+            if (_combinedNodesEdgeVisited.Add(combinedNode))
             {
-                _combinedNodesEdgeVisited.Add(combinedNode);
-
                 _xmlWrite.WriteStartElement("Link");
                 _xmlWrite.WriteAttributeString("Source", _nodeMappings[nodeDepender].ToString());
                 _xmlWrite.WriteAttributeString("Target", _nodeMappings[combinedNode].ToString());

diff --git a/...c/System/ComponentModel/Composition/ReflectionModel/ReflectionComposablePartDefinition.cs b/...c/System/ComponentModel/Composition/ReflectionModel/ReflectionComposablePartDefinition.cs
@@ -185,14 +185,7 @@ internal override bool TryGetExports(ImportDefinition definition, out Tuple<Comp
                                 }
                                 else
                                 {
-                                    if (candidates.Contains(candidatePart))
-                                    {
-                                        alreadyProcessed = true;
-                                    }
-                                    else
-                                    {
-                                        candidates.Add(candidatePart);
-                                    }
+                                    alreadyProcessed |= !candidates.Add(candidatePart);
                                 }
                                 if (!alreadyProcessed)
                                 {

diff --git a/.../System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs b/.../System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
@@ -472,9 +472,8 @@ private void IncrementRefCount(string clientId, EventCommandEventArgs command)
                     }
                 }
 
-                if (!_sharedSessionClientIds.Contains(clientId))
+                if (_sharedSessionClientIds.Add(clientId))
                 {
-                    _sharedSessionClientIds.Add(clientId);
                     Interlocked.Increment(ref _sharedSessionRefCount);
                 }
             }

diff --git a/src/tasks/MobileBuildTasks/Android/AndroidProject.cs b/src/tasks/MobileBuildTasks/Android/AndroidProject.cs
@@ -122,9 +122,8 @@ private static string BuildClangArgs(AndroidBuildOptions buildOptions)
                 string rootPath = Path.GetDirectoryName(lib)!;
                 string libName = Path.GetFileName(lib);
 
-                if (!libDirs.Contains(rootPath))
+                if (libDirs.Add(rootPath))
                 {
-                    libDirs.Add(rootPath);
                     ret.Append($"-L {rootPath} ");
                 }
                 ret.Append($"-l:{libName} ");

diff --git a/src/tools/illink/src/ILLink.Shared/Annotations.cs b/src/tools/illink/src/ILLink.Shared/Annotations.cs
@@ -79,8 +79,7 @@ private static DynamicallyAccessedMemberTypes[] GetAllDynamicallyAccessedMemberT
 			var values = new HashSet<DynamicallyAccessedMemberTypes> (
 								Enum.GetValues (typeof (DynamicallyAccessedMemberTypes))
 								.Cast<DynamicallyAccessedMemberTypes> ());
-			if (!values.Contains (DynamicallyAccessedMemberTypes.Interfaces))
-				values.Add (DynamicallyAccessedMemberTypes.Interfaces);
+			values.Add (DynamicallyAccessedMemberTypes.Interfaces);
 			return values.ToArray ();
 		}