From 71a1d43c97e63bc4c6d3f390c9036f010f67b9db Mon Sep 17 00:00:00 2001 From: NewellClark Date: Tue, 18 May 2021 09:55:05 -0400 Subject: [PATCH 1/9] Add ca1845.md --- .../code-analysis/quality-rules/ca1845.md | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/fundamentals/code-analysis/quality-rules/ca1845.md diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md new file mode 100644 index 0000000000000..0f291550a658b --- /dev/null +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -0,0 +1,99 @@ +--- +title: "CA1841: Prefer Dictionary Contains methods (code analysis)" +description: "Learn about code analysis rule CA1841: Prefer Dictionary Contains methods" +ms.date: 04/21/2021 +ms.topic: reference +f1_keywords: + - "PreferDictionaryContainsMethods" + - "CA1841" +helpviewer_keywords: + - "PreferDictionaryContainsMethods" + - "CA1841" +author: NewellClark +dev_langs: + - CSharp + - VB +--- +# CA1841: Prefer Dictionary Contains methods + +| | Value | +|-|-| +| **Rule ID** |CA1841| +| **Category** |[Performance](performance-warnings.md)| +| **Fix is breaking or non-breaking** |Non-breaking| + +## Cause + +This rule locates calls to a `Contains` method on the `Keys` or `Values` collection of an that could be replaced with a call to a `ContainsKey` or `ContainsValue` method on the dictionary itself. + +## Rule description + +Calling `Contains` on the `Keys` or `Values` collection can often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself: + +- Many dictionary implementations lazily instantiate the key and value collections, which means that accessing the `Keys` or `Values` collection may result in extra allocations. +- You may end up calling an extension method on if the keys or values collection uses explicit interface implementation to hide methods on . This can lead to reduced performance, especially when accessing the key collection. Most dictionary implementations are able to provide a fast O(1) containment check for keys, while the `Contains` extension method on usually does a slow O(n) containment check. + +## How to fix violations + +To fix violations, replace calls to `dictionary.Keys.Contains` or `dictionary.Values.Contains` with calls to `dictionary.ContainsKey` or `dictionary.ContainsValue`, respectively. + +The following code snippet shows examples of violations, and how to fix them. + +```csharp +using System.Collections.Generic; +// Importing this namespace brings extension methods for IEnumerable into scope. +using System.Linq; + +class Example +{ + void Method() + { + var dictionary = new Dictionary(); + + // Violation + dictionary.Keys.Contains("hello world"); + + // Fixed + dictionary.ContainsKey("hello world"); + + // Violation + dictionary.Values.Contains(17); + + // Fixed + dictionary.ContainsValue(17); + } +} +``` + +```vb +Imports System.Collection.Generic +' Importing this namespace brings extension methods for IEnumerable(Of T) into scope. +' Note that in Visual Basic, this namespace is often imported automatically throughout the project. +Imports System.Linq + +Class Example + Private Sub Method() + Dim dictionary = New Dictionary(Of String, Of Integer) + + ' Violation + dictionary.Keys.Contains("hello world") + + ' Fixed + dictionary.ContainsKey("hello world") + + ' Violation + dictionary.Values.Contains(17) + + ' Fixed + dictionary.ContainsValue(17) + End Sub +End Class +``` + +## When to suppress warnings + +It is safe to suppress warnings from this rule if the code in question is not performance-critical. + +## See also + +- [Performance rules](performance-warnings.md) From 5eee8619e0b4aeadc3001ca78169732ae68a7cfe Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:01:59 -0400 Subject: [PATCH 2/9] Add ca1845.md to toc.yml --- docs/fundamentals/toc.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/fundamentals/toc.yml b/docs/fundamentals/toc.yml index 70e4557eabb17..86597e47066b2 100644 --- a/docs/fundamentals/toc.yml +++ b/docs/fundamentals/toc.yml @@ -876,6 +876,8 @@ items: href: code-analysis/quality-rules/ca1838.md - name: CA1841 href: code-analysis/quality-rules/ca1841.md + - name: CA1845 + href: code-analysis/quality-rules/ca1845.md - name: SingleFile rules items: - name: Overview From d661d735a4e140fcae30722c2f2719472226eed6 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:11:07 -0400 Subject: [PATCH 3/9] Add ca1845.md to index.md --- docs/fundamentals/code-analysis/quality-rules/index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/fundamentals/code-analysis/quality-rules/index.md b/docs/fundamentals/code-analysis/quality-rules/index.md index 496d29981912a..304884b47c730 100644 --- a/docs/fundamentals/code-analysis/quality-rules/index.md +++ b/docs/fundamentals/code-analysis/quality-rules/index.md @@ -130,6 +130,7 @@ The following table lists code quality analysis rules. > | [CA1837: Use `Environment.ProcessId` instead of `Process.GetCurrentProcess().Id`](ca1837.md) | `Environment.ProcessId` is simpler and faster than `Process.GetCurrentProcess().Id`. | > | [CA1838: Avoid `StringBuilder` parameters for P/Invokes](ca1838.md) | Marshaling of 'StringBuilder' always creates a native buffer copy, resulting in multiple allocations for one marshaling operation. | > | [CA1841: Prefer Dictionary Contains methods](ca1841.md) | Calling `Contains` on the `Keys` or `Values` collection may often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself. | +> | [CA1845: Use span-based 'string.Concat'](ca1845.md) | It is more efficient to use `AsSpan` and `string.Concat`, instead of `Substring` and a concatenation operator. | > | [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 directly. When an asynchronous method awaits a 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 to signal your intention for continuation. | From d6970f24a52d9da2492995be25c2abe377239c79 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:14:19 -0400 Subject: [PATCH 4/9] Add ca1845.md to performance-warnings.md --- .../code-analysis/quality-rules/performance-warnings.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/fundamentals/code-analysis/quality-rules/performance-warnings.md b/docs/fundamentals/code-analysis/quality-rules/performance-warnings.md index a0512d73ab367..9fd26753e6ab0 100644 --- a/docs/fundamentals/code-analysis/quality-rules/performance-warnings.md +++ b/docs/fundamentals/code-analysis/quality-rules/performance-warnings.md @@ -50,3 +50,4 @@ Performance rules support high-performance libraries and applications. | [CA1837: Use `Environment.ProcessId` instead of `Process.GetCurrentProcess().Id`](ca1837.md) | `Environment.ProcessId` is simpler and faster than `Process.GetCurrentProcess().Id`. | | [CA1838: Avoid `StringBuilder` parameters for P/Invokes](ca1838.md) | Marshaling of `StringBuilder` always creates a native buffer copy, resulting in multiple allocations for one marshaling operation. | | [CA1841: Prefer Dictionary Contains methods](ca1841.md) | Calling `Contains` on the `Keys` or `Values` collection may often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself. | +| [CA1845: Use span-based 'string.Concat'](ca1845.md) | It is more efficient to use `AsSpan` and `string.Concat`, instead of `Substring` and a concatenation operator. | From 6a8df5eacdd7f6106b814bfd8667ff6e008cd98f Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:40:11 -0400 Subject: [PATCH 5/9] Update ca1845.md --- .../code-analysis/quality-rules/ca1845.md | 82 ++++++------------- 1 file changed, 24 insertions(+), 58 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md index 0f291550a658b..38b9101ae9786 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca1845.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -1,98 +1,64 @@ --- -title: "CA1841: Prefer Dictionary Contains methods (code analysis)" -description: "Learn about code analysis rule CA1841: Prefer Dictionary Contains methods" -ms.date: 04/21/2021 +title: "CA1845: Use span-based 'string.Concat' (analysis rule)" +description: "Learn about code analysis rule CA1845: Use span-based 'string.Concat'" +ms.date: 05/18/2021 ms.topic: reference f1_keywords: - - "PreferDictionaryContainsMethods" - - "CA1841" + - "UseSpanBasedStringConcat" + - "CA1845" helpviewer_keywords: - - "PreferDictionaryContainsMethods" - - "CA1841" + - "UseSpanBasedStringConcat" + - "CA1845" author: NewellClark dev_langs: - CSharp - VB --- -# CA1841: Prefer Dictionary Contains methods +# CA1845: Use span-based 'string.Concat' | | Value | |-|-| -| **Rule ID** |CA1841| +| **Rule ID** |CA1845| | **Category** |[Performance](performance-warnings.md)| | **Fix is breaking or non-breaking** |Non-breaking| ## Cause -This rule locates calls to a `Contains` method on the `Keys` or `Values` collection of an that could be replaced with a call to a `ContainsKey` or `ContainsValue` method on the dictionary itself. +This rule locates string-concatenation expressions that contain `Substring` calls and suggests replacing `Substring` with `AsSpan` and using the span-based overload of `string.Concat`. ## Rule description -Calling `Contains` on the `Keys` or `Values` collection can often be more expensive than calling `ContainsKey` or `ContainsValue` on the dictionary itself: - -- Many dictionary implementations lazily instantiate the key and value collections, which means that accessing the `Keys` or `Values` collection may result in extra allocations. -- You may end up calling an extension method on if the keys or values collection uses explicit interface implementation to hide methods on . This can lead to reduced performance, especially when accessing the key collection. Most dictionary implementations are able to provide a fast O(1) containment check for keys, while the `Contains` extension method on usually does a slow O(n) containment check. +Calling `Substring` produces a copy of the extracted substring. By using `AsSpan` instead of `Substring` and calling the overload of `string.Concat` which accepts spans, we can eliminate this unnecessary string allocation. ## How to fix violations -To fix violations, replace calls to `dictionary.Keys.Contains` or `dictionary.Values.Contains` with calls to `dictionary.ContainsKey` or `dictionary.ContainsValue`, respectively. +To fix violations, +1) Replace the string concatenation with a call to `string.Concat`, and +2) 2) Replace calls to `Substring` with calls to `AsSpan`. The following code snippet shows examples of violations, and how to fix them. ```csharp -using System.Collections.Generic; -// Importing this namespace brings extension methods for IEnumerable into scope. -using System.Linq; +using System; class Example { - void Method() + public void Method() { - var dictionary = new Dictionary(); - - // Violation - dictionary.Keys.Contains("hello world"); - - // Fixed - dictionary.ContainsKey("hello world"); - - // Violation - dictionary.Values.Contains(17); - - // Fixed - dictionary.ContainsValue(17); + string cliche = "hello world"; + + // Violation: result of Substring only used in concatenation. Extra allocation is unnecessary. + string s1 = cliche.Substring(6, 5) + cliche.Substring(5) + " is reversed!"; + + // Better: by using AsSpan, we avoid allocating temporary strings. + string s2 = string.Concat(cliche.AsSpan(6, 5), cliche.AsSpan(5), " is reversed!"); } } ``` -```vb -Imports System.Collection.Generic -' Importing this namespace brings extension methods for IEnumerable(Of T) into scope. -' Note that in Visual Basic, this namespace is often imported automatically throughout the project. -Imports System.Linq - -Class Example - Private Sub Method() - Dim dictionary = New Dictionary(Of String, Of Integer) - - ' Violation - dictionary.Keys.Contains("hello world") - - ' Fixed - dictionary.ContainsKey("hello world") - - ' Violation - dictionary.Values.Contains(17) - - ' Fixed - dictionary.ContainsValue(17) - End Sub -End Class -``` - ## When to suppress warnings -It is safe to suppress warnings from this rule if the code in question is not performance-critical. +Do not suppress warnings from this rule. There is no reason to use `Substring` over `AsSpan` when the extracted substring is only being passed to a method with a span-based equivalent. ## See also From 83e9a918829c2fa25897e551ca8e639b7c86ebfe Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:47:48 -0400 Subject: [PATCH 6/9] Update ca1845.md --- .../code-analysis/quality-rules/ca1845.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md index 38b9101ae9786..e410e3ba11d83 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca1845.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -34,7 +34,7 @@ Calling `Substring` produces a copy of the extracted substring. By using `AsSpan To fix violations, 1) Replace the string concatenation with a call to `string.Concat`, and -2) 2) Replace calls to `Substring` with calls to `AsSpan`. +2) Replace calls to `Substring` with calls to `AsSpan`. The following code snippet shows examples of violations, and how to fix them. @@ -45,13 +45,13 @@ class Example { public void Method() { - string cliche = "hello world"; + string text = "fwobz the fwutzle"; - // Violation: result of Substring only used in concatenation. Extra allocation is unnecessary. - string s1 = cliche.Substring(6, 5) + cliche.Substring(5) + " is reversed!"; + // Violation: allocations by Substring are wasteful. + string s1 = text.Substring(10) + "---" + text.Substring(0, 5); - // Better: by using AsSpan, we avoid allocating temporary strings. - string s2 = string.Concat(cliche.AsSpan(6, 5), cliche.AsSpan(5), " is reversed!"); + // Fixed: using AsSpan avoids allocations of temporary strings. + string s2 = string.Concat(text.AsSpan(10), "---", text.AsSpan(0, 5)); } } ``` From 3c9c51e44634ece40abba05946b0455c8f2610c9 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 10:56:17 -0400 Subject: [PATCH 7/9] Fix lint errors --- docs/fundamentals/code-analysis/quality-rules/ca1845.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md index e410e3ba11d83..57dd4824a1e96 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca1845.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -28,12 +28,13 @@ This rule locates string-concatenation expressions that contain `Substring` call ## Rule description -Calling `Substring` produces a copy of the extracted substring. By using `AsSpan` instead of `Substring` and calling the overload of `string.Concat` which accepts spans, we can eliminate this unnecessary string allocation. +Calling `Substring` produces a copy of the extracted substring. By using `AsSpan` instead of `Substring` and calling the overload of `string.Concat` which accepts spans, we can eliminate this unnecessary string allocation. ## How to fix violations -To fix violations, -1) Replace the string concatenation with a call to `string.Concat`, and +To fix violations: + +1) Replace the string concatenation with a call to `string.Concat`, and 2) Replace calls to `Substring` with calls to `AsSpan`. The following code snippet shows examples of violations, and how to fix them. From da7d443d533aff271acda6117268dca8814a82d9 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 11:53:47 -0400 Subject: [PATCH 8/9] Update docs/fundamentals/code-analysis/quality-rules/ca1845.md Co-authored-by: Youssef Victor --- docs/fundamentals/code-analysis/quality-rules/ca1845.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md index 57dd4824a1e96..6879837fcb6a9 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca1845.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -34,8 +34,8 @@ Calling `Substring` produces a copy of the extracted substring. By using `AsSpan To fix violations: -1) Replace the string concatenation with a call to `string.Concat`, and -2) Replace calls to `Substring` with calls to `AsSpan`. +1. Replace the string concatenation with a call to `string.Concat`, and +2. Replace calls to `Substring` with calls to `AsSpan`. The following code snippet shows examples of violations, and how to fix them. From de6bba94ec08cc913854ce80dc76f5bf842a732d Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Tue, 18 May 2021 18:02:26 -0400 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com> --- docs/fundamentals/code-analysis/quality-rules/ca1845.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca1845.md b/docs/fundamentals/code-analysis/quality-rules/ca1845.md index 6879837fcb6a9..e0016b9c2f9dd 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca1845.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca1845.md @@ -12,7 +12,6 @@ helpviewer_keywords: author: NewellClark dev_langs: - CSharp - - VB --- # CA1845: Use span-based 'string.Concat' @@ -24,11 +23,11 @@ dev_langs: ## Cause -This rule locates string-concatenation expressions that contain `Substring` calls and suggests replacing `Substring` with `AsSpan` and using the span-based overload of `string.Concat`. +This rule locates string-concatenation expressions that contain calls and suggests replacing with and using the span-based overload of . ## Rule description -Calling `Substring` produces a copy of the extracted substring. By using `AsSpan` instead of `Substring` and calling the overload of `string.Concat` which accepts spans, we can eliminate this unnecessary string allocation. +Calling `Substring` produces a copy of the extracted substring. By using `AsSpan` instead of `Substring` and calling the overload of `string.Concat` that accepts spans, you can eliminate the unnecessary string allocation. ## How to fix violations