Annotate Result.Fail (and other factory methods) with [Pure] attribute to catch ignored return values

[serber]

Issue

Example of code

var loadUserResult = await LoadUserAsync(userId, cancellationToken);
if (loadUserResult.IsFailed)
{
    ... Some extra actions
    Result.Fail("Error loading user");   // Here is return missed and failure is ignored
}

... Some extra actions
return Result.Ok(user);

Because Result.Fail(...) wasn’t returned, the failure is dropped and the method continues as if everything succeeded.

Proposal

Mark all Result.Fail(…) overloads (and other pure factory methods such as Result.Ok, etc.) with the [Pure] attribute from System.Diagnostics.Contracts. This enables developers to turn on the CA1806: Do not ignore method return value rule and catch any unused results at compile time.

https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.contracts.pureattribute https://learn.microsoft.com/en-gb/dotnet/fundamentals/code-analysis/quality-rules/ca1806

Developers then add to their .editorconfig:

dotnet_diagnostic.CA1806.severity = error

Benefits

• Compile-time safety: Errors whenever a pure factory call isn’t used (e.g. missing return).
• Improved developer experience: Prevents dropped failures and enforces correct control flow.

With [Pure] on Result.Fail, this code

if (loadUserResult.IsFailed)
{
    Result.Fail("Error loading user");
}

produces:

Warning CA1806: The result of the method Result.Fail is ignored; consider using its return value.

and guides developers to write:

if (loadUserResult.IsFailed)
{
    return Result.Fail("Error loading user");
}

Request

1. Add [Pure] to all relevant side-effect-free factory methods in the Result class.
2. Document this in the README so that consumers know they can enable CA1806 for extra safety.

[JasonLandbridge]

Yes this would be awesome!

[DrPepperBianco]

Hi, I have a question. The method FailIf fo instance has a parameter catchHandler which is a delegate. So because this delegate could technically change state if called. So if I call FailIf with such delegate it is not really pure.

How should we handle this? Should we annotate this method as [Pure]? Because if it is pure, depends on the caller? Or shouldn’t we annotate this as [Pure] because it is possible to call it not pure?

[DrPepperBianco]

So I read around a little bit. It seems the general consensus is, that if a function gets a delegate as argument and then calls it, the function therefor is not pure. The reason is, that you cannot (at least in C# and many other languages) restrict the delegate to be pure, therefor you can always call the function with an impure delegate, thus the function itself would be impure then.

Here is the thing, it is of course correct the mark factory methods as pure, so we can do this for all methods without delegate parameters. But wouldn’t it be better to create a new attribute, like "ReturnValueMattersAttribute" and add Analyzer, that checks this? This way we could also mark impure methods as factory methods and catch errors with these methods too.

What I mean is, that we technically misuse [Pure] for a different mechanics here.

PS: We actually could check if the delegate we get also has a [Pure]-Attribute by Reflection. This way we would restrict the method to pure only delegates. But using Reflection for that might be not a good idea, if someone needs this method to run as fast as possible for instance.