2ea2899a46
https://sonarsource.github.io/rspec/#/rspec/S3346/csharp ## Review A dedicated reviewer checked the rule description successfully for: - [ ] logical errors and incorrect information - [ ] information gaps and missing content - [ ] text style and tone - [ ] PR summary and labels follow [the guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule)
91 lines
3.2 KiB
Plaintext
91 lines
3.2 KiB
Plaintext
== Why is this an issue?
|
|
|
|
An assertion is a piece of code that's used during development when the https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-set-debug-and-release-configurations[compilation debug mode is activated]. It allows a program to check itself as it runs. When an assertion is `true`, that means everything is operating as expected.
|
|
|
|
In non-debug mode, all https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debug.assert[`Debug.Assert`] calls are automatically left out (via the https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.conditionalattribute[`Conditional("DEBUG")`] mechanism). So, by contract, the boolean expressions that are evaluated by those assertions must not contain any https://en.wikipedia.org/wiki/Side_effect_(computer_science)[side effects]. Otherwise, when leaving the debug mode, the functional behavior of the application is not the same anymore.
|
|
|
|
The rule will raise if the method name starts with any of the following `remove`, `delete`, `add`, `pop`, `update`, `retain`, `insert`, `push`, `append`, `clear`, `dequeue`, `enqueue`, `dispose`, `put`, or `set`, although `SetEquals` will be ignored.
|
|
|
|
== How to fix it
|
|
|
|
In the following example, the assertion checks the return value of the remove method in the argument. Because the whole line is skipped in non-debug builds, the call to `Remove` never happens in such builds.
|
|
|
|
=== Code examples
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,csharp,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
Debug.Assert(list.Remove("dog"));
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
The `Remove` call must be extracted and the return value needs to be asserted instead.
|
|
|
|
[source,csharp,diff-id=1,diff-type=compliant]
|
|
----
|
|
bool result = list.Remove("dog");
|
|
Debug.Assert(result);
|
|
----
|
|
|
|
== Resources
|
|
|
|
=== Documentation
|
|
|
|
* Microsoft Learn https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debug.assert/[`Debug.Assert` Method]
|
|
* Microsoft Learn https://learn.microsoft.com/en-us/dotnet/framework/debug-trace-profile/[Debugging, tracing, and profiling]
|
|
* Microsoft Learn https://learn.microsoft.com/en-us/dotnet/framework/debug-trace-profile/how-to-compile-conditionally-with-trace-and-debug[How to: Compile Conditionally with Trace and Debug]
|
|
* Microsoft Learn https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/general#conditional-attribute[Miscellaneous attributes interpreted by the C# compiler - `Conditional` attribute]
|
|
|
|
=== Articles & blog posts
|
|
|
|
* Wikipedia https://en.wikipedia.org/wiki/Side_effect_(computer_science)[Side effect (computer science)]
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Expressions used in "Debug.Assert" should not produce side effects.
|
|
|
|
|
|
include::../highlighting.adoc[]
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== on 28 Mar 2017, 09:38:41 Valeri Hristov wrote:
|
|
The JAVA implementation checks for certain verbs in the method name:
|
|
|
|
|
|
* "remove"
|
|
* "delete"
|
|
* "put"
|
|
* "set"
|
|
* "add"
|
|
* "pop"
|
|
* "update"
|
|
* "retain"
|
|
|
|
We could add a few more too:
|
|
|
|
* "insert"
|
|
* "push"
|
|
* "append"
|
|
* "clear"
|
|
* "dequeue"
|
|
|
|
|
|
|
|
=== on 22 May 2017, 14:22:31 Michal Barczyk wrote:
|
|
Also added "dispose"
|
|
|
|
include::../comments-and-links.adoc[]
|
|
|
|
endif::env-github,rspecator-view[]
|