From 8486a45f51fafeb55ce275cf206a01e20821166c Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 6 Jan 2025 09:06:37 +0100 Subject: [PATCH 1/7] Document MSTEST0038: Don't use 'Assert.AreSame' with value types --- .../testing/mstest-analyzers/mstest0038.md | 41 +++++++++++++++++++ .../testing/mstest-analyzers/usage-rules.md | 1 + 2 files changed, 42 insertions(+) create mode 100644 docs/core/testing/mstest-analyzers/mstest0038.md diff --git a/docs/core/testing/mstest-analyzers/mstest0038.md b/docs/core/testing/mstest-analyzers/mstest0038.md new file mode 100644 index 0000000000000..ebab00de5be5e --- /dev/null +++ b/docs/core/testing/mstest-analyzers/mstest0038.md @@ -0,0 +1,41 @@ +--- +title: "Don't use 'Assert.AreSame' with value types" +description: "Learn about code analysis rule MSTEST0038: Don't use 'Assert.AreSame' with value types" +ms.date: 01/06/2025 +f1_keywords: +- MSTEST0038 +- AvoidAssertAreSameWithValueTypesAnalyzer +helpviewer_keywords: +- AvoidAssertAreSameWithValueTypesAnalyzer +- MSTEST0038 +author: Youssef1313 +ms.author: ygerges +--- +# MSTEST0038: Don't use 'Assert.AreSame' with value types + +| Property | Value | +|-------------------------------------|------------------------------------------------------------------------| +| **Rule ID** | MSTEST0038 | +| **Title** | Don't use 'Assert.AreSame' with value types | +| **Category** | Usage | +| **Fix is breaking or non-breaking** | Non-breaking | +| **Enabled by default** | Yes | +| **Default severity** | Warning | +| **Introduced in version** | 3.8.0 | +| **Is there a code fix** | Yes | + +## Cause + +The use of with one or both arguments being a value type. + +## Rule description + +The way works is by comparing the *reference* of the given expected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). So, the assert will always fail. + +## How to fix violations + +Use `Assert.AreEqual` instead of `Assert.AreSame`. + +## When to suppress warnings + +Do not suppress a warning from this rule. Ignoring this rule will result in an assertion that will fail. \ No newline at end of file diff --git a/docs/core/testing/mstest-analyzers/usage-rules.md b/docs/core/testing/mstest-analyzers/usage-rules.md index c8a742fc973af..856b82ed7c3f5 100644 --- a/docs/core/testing/mstest-analyzers/usage-rules.md +++ b/docs/core/testing/mstest-analyzers/usage-rules.md @@ -28,3 +28,4 @@ Identifier | Name | Description [MSTEST0024](mstest0024.md) | DoNotStoreStaticTestContextAnalyzer | Do not store TestContext in a static member [MSTEST0026](mstest0026.md) | AssertionArgsShouldAvoidConditionalAccessRuleId | Avoid conditional access in assertions [MSTEST0037](mstest0037.md) | UseProperAssertMethodsAnalyzer | Use proper `Assert` methods +[MSTEST0038](mstest0038.md) | AvoidAssertAreSameWithValueTypesAnalyzer | Don't use 'Assert.AreSame' with value types From 0985cb855d3f52446226c0818276619f160cd000 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Mon, 6 Jan 2025 10:55:47 +0100 Subject: [PATCH 2/7] Improve doc --- docs/core/testing/mstest-analyzers/mstest0038.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/core/testing/mstest-analyzers/mstest0038.md b/docs/core/testing/mstest-analyzers/mstest0038.md index ebab00de5be5e..94e2d86e9a932 100644 --- a/docs/core/testing/mstest-analyzers/mstest0038.md +++ b/docs/core/testing/mstest-analyzers/mstest0038.md @@ -32,9 +32,11 @@ The use of works is by comparing the *reference* of the given expected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). So, the assert will always fail. +The only case where this assert will pass is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. + ## How to fix violations -Use `Assert.AreEqual` instead of `Assert.AreSame`. +If both arguments are nullable value types whose values are expected to be null, then use two separate `Assert.IsNull` calls. Otherwise, use `Assert.AreEqual` instead of `Assert.AreSame`. ## When to suppress warnings From 859348075c769fb27fa2355723aaa0a9f37aed27 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Tue, 7 Jan 2025 08:33:53 +0100 Subject: [PATCH 3/7] Satisfy markdownlint From b622828e34460c782c747b813ac6aff74e6b4f74 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Tue, 7 Jan 2025 08:45:02 +0100 Subject: [PATCH 4/7] Update MSTEST0038 rule to include `Assert.AreNotSame` --- .../testing/mstest-analyzers/mstest0038.md | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/docs/core/testing/mstest-analyzers/mstest0038.md b/docs/core/testing/mstest-analyzers/mstest0038.md index 94e2d86e9a932..052f18383d45d 100644 --- a/docs/core/testing/mstest-analyzers/mstest0038.md +++ b/docs/core/testing/mstest-analyzers/mstest0038.md @@ -1,6 +1,6 @@ --- -title: "Don't use 'Assert.AreSame' with value types" -description: "Learn about code analysis rule MSTEST0038: Don't use 'Assert.AreSame' with value types" +title: "Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types" +description: "Learn about code analysis rule MSTEST0038: Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types" ms.date: 01/06/2025 f1_keywords: - MSTEST0038 @@ -11,33 +11,36 @@ helpviewer_keywords: author: Youssef1313 ms.author: ygerges --- -# MSTEST0038: Don't use 'Assert.AreSame' with value types +# MSTEST0038: Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types | Property | Value | |-------------------------------------|------------------------------------------------------------------------| | **Rule ID** | MSTEST0038 | -| **Title** | Don't use 'Assert.AreSame' with value types | +| **Title** | Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types | | **Category** | Usage | | **Fix is breaking or non-breaking** | Non-breaking | | **Enabled by default** | Yes | | **Default severity** | Warning | | **Introduced in version** | 3.8.0 | -| **Is there a code fix** | Yes | +| **Is there a code fix** | No | ## Cause -The use of with one or both arguments being a value type. +The use of or with one or both arguments being a value type. ## Rule description -The way works is by comparing the *reference* of the given expected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). So, the assert will always fail. +The way and work is by comparing the *reference* of the given expected/notExpected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). -The only case where this assert will pass is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. +If using `AreSame`, the assert will always fail. If using `AreNotSame`, the assert will always pass. + +The only case for `AreSame` when this assert will pass is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. ## How to fix violations -If both arguments are nullable value types whose values are expected to be null, then use two separate `Assert.IsNull` calls. Otherwise, use `Assert.AreEqual` instead of `Assert.AreSame`. +Use `Assert.AreEqual` and `Assert.AreNotEqual` instead of `Assert.AreSame` and `Assert.AreNotSame`. +If using `Assert.AreSame` and both arguments are nullable value types whose values are expected to be null, then two separate `Assert.IsNull` calls may be a better fit than `AreEqual`, depending on the intent of the test. ## When to suppress warnings -Do not suppress a warning from this rule. Ignoring this rule will result in an assertion that will fail. \ No newline at end of file +Do not suppress a warning from this rule. Ignoring this rule will result in an assertion that will always fail or always pass. \ No newline at end of file From 47299fe3c9d45a2c27e1363121f95227ba24b33b Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Tue, 7 Jan 2025 08:45:23 +0100 Subject: [PATCH 5/7] Update usage-rules.md --- docs/core/testing/mstest-analyzers/usage-rules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/testing/mstest-analyzers/usage-rules.md b/docs/core/testing/mstest-analyzers/usage-rules.md index 856b82ed7c3f5..8cd8faee4327d 100644 --- a/docs/core/testing/mstest-analyzers/usage-rules.md +++ b/docs/core/testing/mstest-analyzers/usage-rules.md @@ -28,4 +28,4 @@ Identifier | Name | Description [MSTEST0024](mstest0024.md) | DoNotStoreStaticTestContextAnalyzer | Do not store TestContext in a static member [MSTEST0026](mstest0026.md) | AssertionArgsShouldAvoidConditionalAccessRuleId | Avoid conditional access in assertions [MSTEST0037](mstest0037.md) | UseProperAssertMethodsAnalyzer | Use proper `Assert` methods -[MSTEST0038](mstest0038.md) | AvoidAssertAreSameWithValueTypesAnalyzer | Don't use 'Assert.AreSame' with value types +[MSTEST0038](mstest0038.md) | AvoidAssertAreSameWithValueTypesAnalyzer | Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types From 29eb63887944ef250342808d8c1750d3777e8f1b Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Tue, 7 Jan 2025 08:50:49 +0100 Subject: [PATCH 6/7] Satisfy markdownlint --- docs/core/testing/mstest-analyzers/mstest0038.md | 4 ++-- docs/core/testing/mstest-analyzers/usage-rules.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/core/testing/mstest-analyzers/mstest0038.md b/docs/core/testing/mstest-analyzers/mstest0038.md index 052f18383d45d..3f8c3f948e7fe 100644 --- a/docs/core/testing/mstest-analyzers/mstest0038.md +++ b/docs/core/testing/mstest-analyzers/mstest0038.md @@ -32,7 +32,7 @@ The use of and work is by comparing the *reference* of the given expected/notExpected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). -If using `AreSame`, the assert will always fail. If using `AreNotSame`, the assert will always pass. +If using `AreSame`, the assert will always fail. If using `AreNotSame`, the assert will always pass. The only case for `AreSame` when this assert will pass is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. @@ -43,4 +43,4 @@ If using `Assert.AreSame` and both arguments are nullable value types whose valu ## When to suppress warnings -Do not suppress a warning from this rule. Ignoring this rule will result in an assertion that will always fail or always pass. \ No newline at end of file +Do not suppress a warning from this rule. Ignoring this rule will result in an assertion that will always fail or always pass. diff --git a/docs/core/testing/mstest-analyzers/usage-rules.md b/docs/core/testing/mstest-analyzers/usage-rules.md index 8cd8faee4327d..dafef4793f875 100644 --- a/docs/core/testing/mstest-analyzers/usage-rules.md +++ b/docs/core/testing/mstest-analyzers/usage-rules.md @@ -28,4 +28,4 @@ Identifier | Name | Description [MSTEST0024](mstest0024.md) | DoNotStoreStaticTestContextAnalyzer | Do not store TestContext in a static member [MSTEST0026](mstest0026.md) | AssertionArgsShouldAvoidConditionalAccessRuleId | Avoid conditional access in assertions [MSTEST0037](mstest0037.md) | UseProperAssertMethodsAnalyzer | Use proper `Assert` methods -[MSTEST0038](mstest0038.md) | AvoidAssertAreSameWithValueTypesAnalyzer | Don't use 'Assert.AreSame' or 'Assert.AreNotSame' with value types +[MSTEST0038](mstest0038.md) | AvoidAssertAreSameWithValueTypesAnalyzer | Don't use `Assert.AreSame` or `Assert.AreNotSame` with value types From 3efde8e2dacaa8a4b4aa0c373505b9bb4147deef Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Wed, 8 Jan 2025 16:10:05 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com> --- docs/core/testing/mstest-analyzers/mstest0038.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/core/testing/mstest-analyzers/mstest0038.md b/docs/core/testing/mstest-analyzers/mstest0038.md index 3f8c3f948e7fe..41240248c0223 100644 --- a/docs/core/testing/mstest-analyzers/mstest0038.md +++ b/docs/core/testing/mstest-analyzers/mstest0038.md @@ -26,20 +26,20 @@ ms.author: ygerges ## Cause -The use of or with one or both arguments being a value type. +The use of or with one or both arguments being a value type. ## Rule description -The way and work is by comparing the *reference* of the given expected/notExpected and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it will be [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). + and work by comparing the *reference* of the given `expected`/`notExpected` and actual arguments via `ReferenceEquals`. Hence, when you pass a value type, it is [boxed](../../../csharp/programming-guide/types/boxing-and-unboxing.md#boxing). If using `AreSame`, the assert will always fail. If using `AreNotSame`, the assert will always pass. -The only case for `AreSame` when this assert will pass is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. +For `AreSame`, the only case when the assert passes is if both arguments are nullable value types whose values are both null. In this case, it's clearer to have two separate `Assert.IsNull` calls. ## How to fix violations Use `Assert.AreEqual` and `Assert.AreNotEqual` instead of `Assert.AreSame` and `Assert.AreNotSame`. -If using `Assert.AreSame` and both arguments are nullable value types whose values are expected to be null, then two separate `Assert.IsNull` calls may be a better fit than `AreEqual`, depending on the intent of the test. +If using `Assert.AreSame` and both arguments are nullable value types whose values are expected to be null, then two separate `Assert.IsNull` calls might be a better fit than `AreEqual`, depending on the intent of the test. ## When to suppress warnings