Describe the bug
Both Result.IsSuccess and Result<T>.IsSuccess properties are decorated with the [MemberNotNullWhen(false, nameof(Problem))] attribute. This is incorrect; Problem can be null for unsuccessful Result instances.
To Reproduce
Steps to reproduce the behavior:
- Create a Result with Result.Fail()
- In a conditional block where
IsSuccess is false, access the Problem property
- Note that analysis shows it should not be null, but it is
Expected behavior
Code analysis should not guarantee that Problem is not null when it is possibly null.
Code Sample
var result = Result.Fail();
if (!result.IsSuccess)
{
Problem problem = result.Problem; // According to code analysis: 'Problem' is not null here.
Debug.Assert(problem is not null); // However, this assertion fails
}
Additional context
Additional thoughts:
- This is also a problem when using the
default instance of Result.
Problem has a public set; setter. This weakens any guarantees.
Suggested fixes
- Remove the internal
Result.Create(bool isSuccess, Problem? problem = null) method & replace it with CreateSuccess() & CreateFailure(Problem) methods.
- Make the
Result(bool, Problem?) constructor private.
- Parameterless
Result.Fail() method will create a 'generic error' Problem (Titles.Error, Messages.GenericError in ProblemConstants).
Result.Problem property getter implemented to return a fallback Problem if there is no problem assigned (for the default instance of Result). This would either be a 'generic error' problem as mentioned above, or a new error specifying that a default instance was used which is likely a user error.
Result.Problem setter should be private. Note that this is a breaking change because it was previously public.
Describe the bug
Both
Result.IsSuccessandResult<T>.IsSuccessproperties are decorated with the[MemberNotNullWhen(false, nameof(Problem))]attribute. This is incorrect;Problemcan be null for unsuccessfulResultinstances.To Reproduce
Steps to reproduce the behavior:
IsSuccessis false, access theProblempropertyExpected behavior
Code analysis should not guarantee that
Problemis not null when it is possibly null.Code Sample
Additional context
Additional thoughts:
defaultinstance ofResult.Problemhas a publicset;setter. This weakens any guarantees.Suggested fixes
Result.Create(bool isSuccess, Problem? problem = null)method & replace it withCreateSuccess()&CreateFailure(Problem)methods.Result(bool, Problem?)constructor private.Result.Fail()method will create a 'generic error' Problem (Titles.Error,Messages.GenericErrorinProblemConstants).Result.Problemproperty getter implemented to return a fallback Problem if there is no problem assigned (for thedefaultinstance ofResult). This would either be a 'generic error' problem as mentioned above, or a new error specifying that a default instance was used which is likely a user error.Result.Problemsetter should be private. Note that this is a breaking change because it was previously public.