No Optional On Ref Out Parameter

ID

csharp.no_optional_on_ref_out_parameter

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Best Practice

Language

CSharp

Tags

api_design, attributes, best_practice, parameters

Description

Reports a parameter that is declared ref or out and also carries the Optional attribute. Both spellings of the attribute name are recognised, qualified or not.

The in modifier is not reported: it passes a read-only reference, so nothing travels back to the caller through it and declaring it optional is not a contradiction.

Rationale

[Optional] announces that callers need not supply the argument. ref and out exist so the method can write a result into the variable the caller passed. Put together they describe an argument that may be absent and that the method will nevertheless fill in — there is no variable to fill in when it is absent.

The compiler does not resolve the conflict either. C# requires every ref and out argument to be written at the call site, modifier included, so no C# caller can take the attribute up on its offer. What is left is a signature that reads as if the argument were negotiable, plus a default value handed to reflection and interop callers that no caller of theirs will ever see.

using System.Runtime.InteropServices;

public class Reader
{
    public void Read([Optional] out int value)            // FLAW
    {
        value = 0;
    }

    public void Adjust([Optional] ref int level) { }       // FLAW

    public void Track([Optional] int level) { }            // OK, passed by value

    public void Fetch(out int value) { value = 0; }        // OK, no attribute

    public double Span([Optional] in double from) => from;  // OK, 'in' is read-only
}

Remediation

Decide which of the two the parameter really is. If the value has to come back to the caller, keep ref or out and drop the attribute. If the argument is genuinely optional, drop ref or out and give the parameter a default value, returning the result instead — or split the method into an overload that does not take the parameter at all.