Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation for CA1868 #36379

Merged
merged 7 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions docs/fundamentals/code-analysis/quality-rules/ca1865.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
---
title: "CA1865: Unnecessary call to 'Contains' for sets"
description: "Learn about code analyzer rule CA1865 - Unnecessary call to 'Contains' for sets"
ms.date: 07/25/2023
ms.topic: reference
f1_keywords:
- CA1865
- DoNotGuardSetAddOrRemoveByContainsAnalyzer
helpviewer_keywords:
- CA1865
author: mpidash
dev_langs:
- CSharp
- VB
---

# CA1865: Unnecessary call to 'Contains' for sets

| | Value |
| ----------------------------------- |----------------------------------------|
| **Rule ID** | CA1865 |
| **Category** | [Performance](performance-warnings.md) |
| **Fix is breaking or non-breaking** | Non-breaking |
| **Enabled by default in .NET 7** | No |

## Cause

An <xref:System.Collections.Generic.ISet%601.Add%2A> or <xref:System.Collections.Generic.ICollection%601.Remove%2A> (<xref:System.Collections.Immutable.IImmutableSet%601.Add%2A?displayProperty=nameWithType> or <xref:System.Collections.Immutable.IImmutableSet%601.Remove%2A?displayProperty=nameWithType>) call is guarded by a call to <xref:System.Collections.Generic.ICollection%601.Contains%2A> (<xref:System.Collections.Immutable.IImmutableSet%601.Contains%2A?displayProperty=nameWithType>).

## Rule description

Both <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> perform a lookup which makes calling <xref:System.Collections.Generic.ICollection%601.Contains%2A> before redundant. It is more efficient to call <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> directly, which will return a Boolean value indicating whether an item has been added or removed.
mpidash marked this conversation as resolved.
Show resolved Hide resolved

This also applies to <xref:System.Collections.Immutable.IImmutableSet%601.Add%2A?displayProperty=nameWithType> and <xref:System.Collections.Immutable.IImmutableSet%601.Remove%2A?displayProperty=nameWithType>, except that they either return a new set when an item is added or removed, or the original set otherwise.
mpidash marked this conversation as resolved.
Show resolved Hide resolved

## How to fix violations

Replace the call to <xref:System.Collections.Generic.ICollection%601.Contains%2A> (<xref:System.Collections.Immutable.IImmutableSet%601.Contains%2A?displayProperty=nameWithType>) that is followed by a call to <xref:System.Collections.Generic.ISet%601.Add%2A> or <xref:System.Collections.Generic.ICollection%601.Remove%2A> (<xref:System.Collections.Immutable.IImmutableSet%601.Add%2A?displayProperty=nameWithType> or <xref:System.Collections.Immutable.IImmutableSet%601.Remove%2A?displayProperty=nameWithType>) with a single call to the latter.
mpidash marked this conversation as resolved.
Show resolved Hide resolved

## Example

The following code snippet shows a violation of CA1865:

```csharp
void Run(ISet<string> set)
{
if (!set.Contains("Hello World"))
{
set.Add("Hello World");
}
}
```

```vb
Sub Run(set As ISet(Of String))
If Not set.Contains("Hello World") Then
set.Add("Hello World")
End If
End Sub
```

The following code snippet fixes the violation:

```csharp
void Run(ISet<string> set)
{
set.Add("Hello World");
}
```

```vb
Sub Run(set As ISet(Of String))
set.Add("Hello World")
End Sub
```

## When to suppress warnings

It's safe to suppress this warning if performance isn't a concern.

## Suppress a warning

If you just want to suppress a single violation, add preprocessor directives to your source file to disable and then re-enable the rule.

```csharp
#pragma warning disable CA1865
// The code that's violating the rule is on this line.
#pragma warning restore CA1865
```

To disable the rule for a file, folder, or project, set its severity to `none` in the [configuration file](../configuration-files.md).

```ini
[*.{cs,vb}]
dotnet_diagnostic.CA1865.severity = none
```

For more information, see [How to suppress code analysis warnings](../suppress-warnings.md).
1 change: 1 addition & 0 deletions docs/fundamentals/code-analysis/quality-rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ The following table lists code quality analysis rules.
> | [CA1860: Avoid using 'Enumerable.Any()' extension method](ca1860.md) | It's more efficient and clearer to use `Length`, `Count`, or `IsEmpty` (if possible) than to call <xref:System.Linq.Enumerable.Any%2A?displayProperty=nameWithType> to determine whether a collection type has any elements. |
> | [CA1861: Avoid constant arrays as arguments](ca1861.md) | Constant arrays passed as arguments are not reused which implies a performance overhead. Consider extracting them to 'static readonly' fields to improve performance. |
> | [CA1864: Prefer the 'IDictionary.TryAdd(TKey, TValue)' method](ca1864.md) | Both <xref:System.Collections.Generic.Dictionary%602.ContainsKey(%600)?displayProperty=nameWithType> and <xref:System.Collections.Generic.Dictionary%602.Add%2A?displayProperty=nameWithType> perform a lookup, which is redundant. It's is more efficient to call <xref:System.Collections.Generic.Dictionary%602.TryAdd%2A?displayProperty=nameWithType>, which returns a `bool` indicating if the value was added or not. `TryAdd` doesn't overwrite the key's value if the key is already present. |
> | [CA1865: Unnecessary call to 'Contains' for sets](ca1865.md) | Both <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> perform a lookup which makes calling <xref:System.Collections.Generic.ICollection%601.Contains%2A> before redundant. It is more efficient to call <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> directly, which will return a Boolean value indicating whether an item has been added or removed. |
gewarren marked this conversation as resolved.
Show resolved Hide resolved
> | [CA2000: Dispose objects before losing scope](ca2000.md) | Because an exceptional event might occur that will prevent the finalizer of an object from running, the object should be explicitly disposed before all references to it are out of scope. |
> | [CA2002: Do not lock on objects with weak identity](ca2002.md) |An object is said to have a weak identity when it can be directly accessed across application domain boundaries. A thread that tries to acquire a lock on an object that has a weak identity can be blocked by a second thread in a different application domain that has a lock on the same object. |
> | [CA2007: Do not directly await a Task](ca2007.md) | An asynchronous method [awaits](../../../csharp/language-reference/operators/await.md) a <xref:System.Threading.Tasks.Task> directly. When an asynchronous method awaits a <xref:System.Threading.Tasks.Task> directly, continuation occurs in the same thread that created the task. This behavior can be costly in terms of performance and can result in a deadlock on the UI thread. Consider calling <xref:System.Threading.Tasks.Task.ConfigureAwait(System.Boolean)?displayProperty=nameWithType> to signal your intention for continuation. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,4 @@ Performance rules support high-performance libraries and applications.
| [CA1860: Avoid using 'Enumerable.Any()' extension method](ca1860.md) | It's more efficient and clearer to use `Length`, `Count`, or `IsEmpty` (if possible) than to call <xref:System.Linq.Enumerable.Any%2A?displayProperty=nameWithType> to determine whether a collection type has any elements. |
| [CA1861: Avoid constant arrays as arguments](ca1861.md) | Constant arrays passed as arguments are not reused which implies a performance overhead. Consider extracting them to 'static readonly' fields to improve performance. |
| [CA1864: Prefer the 'IDictionary.TryAdd(TKey, TValue)' method](ca1864.md) | Both <xref:System.Collections.Generic.Dictionary%602.ContainsKey(%600)?displayProperty=nameWithType> and <xref:System.Collections.Generic.Dictionary%602.Add%2A?displayProperty=nameWithType> perform a lookup, which is redundant. It's is more efficient to call <xref:System.Collections.Generic.Dictionary%602.TryAdd%2A?displayProperty=nameWithType>, which returns a `bool` indicating if the value was added or not. `TryAdd` doesn't overwrite the key's value if the key is already present. |
| [CA1865: Unnecessary call to 'Contains' for sets](ca1865.md) | Both <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> perform a lookup which makes calling <xref:System.Collections.Generic.ICollection%601.Contains%2A> before redundant. It is more efficient to call <xref:System.Collections.Generic.ISet%601.Add%2A> and <xref:System.Collections.Generic.ICollection%601.Remove%2A> directly, which will return a Boolean value indicating whether an item has been added or removed. |
gewarren marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions docs/navigate/tools-diagnostics/toc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,8 @@ items:
href: ../../fundamentals/code-analysis/quality-rules/ca1861.md
- name: CA1864
href: ../../fundamentals/code-analysis/quality-rules/ca1864.md
- name: CA1865
href: ../../fundamentals/code-analysis/quality-rules/ca1865.md
- name: SingleFile rules
items:
- name: Overview
Expand Down