Return Value Ignored

ID

csharp.return_value_ignored

Severity

low

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

dead_code, reliability

Description

Reports calls to known pure / observable functions whose return value is discarded at statement level. For pure functions (Where, Select, Trim, Substring, …​) the call has no observable side effect, so dropping the result means the call accomplishes nothing.

Rationale

The canonical mistake is forgetting to assign the result of an immutable operation: s.Trim(); does not modify s (strings are immutable), and list.Select(…​); does not enumerate the source. Whatever cleanup or filtering the writer intended is silently lost.

Replace is only reported on a string receiver: string.Replace(…​) allocates a new string and leaves the original untouched, but StringBuilder.Replace(…​) mutates the builder in place and returns this for chaining, so discarding that result is correct and not reported.

A call whose result is discarded only to force a lambda passed to an Assert.Throws-style call to execute — the call is meant to raise the expected exception, not to produce a value the test needs — is not reported either.

public class Mistakes
{
    public void Bad(string s)
    {
        s.Trim();                                       // FLAW
    }

    public void BadLinq(System.Collections.Generic.IEnumerable<int> xs)
    {
        xs.Where(x => x > 0);                            // FLAW
    }

    public void BadStringReplace(string s)
    {
        s.Replace("a", "b");                             // FLAW - allocates a new string
    }

    public void OkBuilderReplace(string source)
    {
        var sb = new System.Text.StringBuilder(source);
        sb.Replace("&", "&amp;");                         // OK - StringBuilder mutates in place
    }

    public void OkForcedException(System.Collections.Generic.List<int> xs)
    {
        Assert.Throws<System.InvalidOperationException>(() =>
        {
            xs.Single();                                  // OK - forces the exception Assert.Throws expects
        });
    }

    public string Good(string s)
    {
        return s.Trim();                                 // OK
    }

    public void OkSideEffect(System.Collections.Generic.List<int> xs)
    {
        xs.Add(1);                                       // OK, side effect
    }
}

Remediation

Either consume the returned value (assign it, return it, pass it on) or remove the call entirely. For LINQ pipelines that need to be materialised, append a ToList() / ToArray() call and use the result.

References