No Optional Params

ID

csharp.no_optional_params

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Best Practice

Language

CSharp

Tags

api_design, best_practice, parameters, versioning

Description

Reports a default parameter value declared by a method or constructor that is reachable from outside the assembly — public, protected or protected internal, on a type that is itself visible.

One issue is raised per member, at its first defaulted parameter.

Members that cannot be reached from outside the assembly are not reported: private, internal and private protected ones, and anything declared on a type that is not itself visible. Those are compiled together with every caller they have, so the hazard below cannot arise.

Five visible shapes are also left alone: an override, a member implementing a method of an interface the containing type implements, the constructor of an attribute class, an ASP.NET MVC/Web API action or Razor Pages handler (its parameters are model-bound by the framework, not called with a plain argument list), and a test method carrying an NUnit/xUnit/MSTest test attribute ([Theory], [Fact], [Test], [TestMethod], …​) — the test runner supplies the arguments itself, so there is no independently-compiled caller to go out of step.

The interface/implementation deduplication is not limited to a single file: an implementation is matched against an interface method of the same name and parameter count declared anywhere in the project, provided the implementing file can reach that interface’s namespace (its own namespace, or one it imports with using). The interface declaration is the one reported — callers bind to it, and it is where the contract, default included, was actually decided.

That project-wide match is by simple name, not full type resolution, so it can occasionally find two unrelated interfaces that happen to share a name and both be reachable from the implementing file — one genuinely implemented, one a coincidence elsewhere in the project. When that happens the implementation is reported rather than suppressed: a tie between two equally plausible interfaces is not evidence that either one is actually implemented, so guessing which is not an option.

Rationale

A default value is not stored with the method the way a parameter type is. The compiler copies it into every call site that omits the argument, so the constant ends up compiled into the caller’s assembly.

Publishing a new version of the library with a different default therefore changes nothing for the programs already built against it. They keep passing the old value, invisibly, until somebody rebuilds them — at which point their behaviour changes without a line of their own source having moved. Two builds of the same unchanged call site then mean two different things, which is a difference no amount of reading the calling code can reveal.

Overloads do not have this problem: the value lives in the body of the shorter overload, inside the library, and every caller picks up a new one the moment they load the new assembly.

using System;

public interface IFormatter
{
    string Format(string text, bool trim = true);       // FLAW
}

public class Publisher
{
    public void Send(string topic, int retries = 3) { } // FLAW

    public Publisher(string root, bool cache = true) { } // FLAW

    internal void Trace(string message, bool full = false) { }  // OK, not visible outside the assembly

    public void Flush(string topic) => Flush(topic, 3); // OK, an overload keeps the value in the library

    public void Flush(string topic, int retries) { }    // OK, no default value
}

Remediation

Replace the optional parameter with an overload that does not take it and supplies the value in its own body. The value then lives inside the library, callers bind to the shorter signature, and changing it in a later version reaches every caller as soon as they load the new assembly — no rebuild needed.

When several parameters are optional and the overload count would explode, take an options object instead: a small type whose properties carry the defaults. Callers set only what they care about, and the defaults stay on the library side where they can still be changed.

If the member must keep the signature for an external contract, treat the default as part of the published API: document the value and change it only with a major version, exactly as for a parameter type.

References