rspec/rules/S2589/csharp/rule.adoc

== Why is this an issue?

An operand of a boolean expression that never changes the result of the expression might not match the programmer's intent and can lead to unexpected behavior and potential bugs.

[source,csharp]
----
var a = true;
if (a)
{
    DoSomething();
}
----

This also applies to the https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-coalescing-operator[null coalescing operator] when one of the operands always evaluates to `null`.

[source,csharp]
----
string d = null;
var v1 = d ?? "value";
----

=== Exceptions

This rule will not raise an issue in either of these cases:

* When the condition is a single `const bool`
+
[source,csharp]
----
const bool debug = false;
//...
if (debug)
{
  // Print something
}
----
* When the condition is the literal `true` or `false`.

In these cases, it is obvious the code is as intended.

== How to fix it

The conditions should be reviewed to decide whether:

* to update the unnecessary operand
* to remove the unnecessary operand

=== Code examples

==== Noncompliant code example

[source,csharp]
----
public void Sample(bool b, bool c)
{
    var a = true;
    if (a)                  // Noncompliant: "a" is always "true"
    {
        DoSomething();
    }

    if (b && a)             // Noncompliant: "a" is always "true"
    {
        DoSomething();
    }

    if (c || !a)            // Noncompliant: "!a" is always "false"
    {
        DoSomething();
    }

    string d = null;
    var v1 = d ?? "value";  // Noncompliant: "d" is always null and v1 is always "value".
    var v2 = s ?? d;        // Noncompliant: "d" is always null and v2 is always equal to s.
}
----

==== Compliant solution

The unnecessary operand is updated:

[source,csharp]
----
public void Sample(bool b, bool c, string s)
{
    var a = IsAllowed();
    if (a)                  // Compliant
    {
        DoSomething();
    }

    if (b && a)             // Compliant
    {
        DoSomething();
    }

    if (c || !a)            // Compliant
    {
        DoSomething();
    }

    string d = GetStringData();
    var v1 = d ?? "value";  // Compliant
    var v2 = s ?? d;        // Compliant
}
----

The unnecessary operand is removed:

[source,csharp]
----
public void Sample(bool b, bool c, string s)
{
    DoSomething();

    if (b)                  // Compliant
    {
        DoSomething();
    }

    if (c)                  // Compliant
    {
        DoSomething();
    }

    var v1 = "value";       // Compliant
    var v2 = s;             // Compliant
}
----

== Resources

=== Documentation

* https://cwe.mitre.org/data/definitions/571[MITRE, CWE-571] - Expression is Always True
* https://cwe.mitre.org/data/definitions/570[MITRE, CWE-570] - Expression is Always False
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/boolean-logical-operators#conditional-logical-and-operator-[Conditional logical AND operator &&]
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/boolean-logical-operators#conditional-logical-or-operator-[Conditional logical OR operator ||]
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-coalescing-operator[Null-coalescing operators ?? and ??=]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

* Change this condition so that it does not always evaluate to "[true|false]".
* Change this expression which always evaluates to the same result.


include::../highlighting.adoc[]

'''
== Comments And Links
(visible only on this page)

include::../comments-and-links.adoc[]

endif::env-github,rspecator-view[]