Add analyzer warning about throw expressions passed to GetOrElse

[Mafii]

When calling GetOrElse(<alternative>) at the end of a option chain, sometimes developers forget to pass a method and pass the throw expression directly. This happens because the throw expression will statisfy any type for the compiler.

Example of code that compiles, seems fine, but will throw an exception in both None and Some cases:

Option.FromNullable(someStringThatCanBeNullable)
  .Where(value => value != "Hello")
  .GetOrElse(throw new Exception("Unexpected...."));

what the person probably wanted to do:

Option.FromNullable(someStringThatCanBeNullable)
  .Where(value => value != "Hello")
  .GetOrElse(() => throw new Exception("Unexpected...."));

This is also what I propose as refactoring that should be presented to the user (quick fix): Just add a () => in front of the throw expression.

[bash]

I agree, that's something that I've seen done wrong quite a few times.

Of course, this may also be a hint that the API isn't as obvious as it could be.
That said, I don't have any better alternatives, so an analyzer is probably our best option here.

[FreeApophis]

An alternative would be to remove the value overload and only accept a Func. In almost all cases except the primitive types, this would improve the runtime performance, because the Func will only be evaluated when really necessary.

[Mafii]

An alternative would be to remove the value overload and only accept a Func. In almost all cases except the primitive types, this would improve the runtime performance, because the Func will only be evaluated when really necessary.

While that is a possibility, I think it would be noise in cases where variables or primitives are already present.

You could argue it's still better however, as you can't accidentially calculate the fallback value (if you call a method) instead of passing it as lazy. There is no good way to write an analyzer for that. I would not strongly oppose removing the value overload.

[bash]

@Mafii Are you sure that your first example compiles? I get a compiler error.

[Mafii]

You're right, it does not compile - I sadly don't have the original anymore, and wrote this out of my head.

Here's one that does compile:

Funcky.Monads.Option.FromNullable(someStringThatCanBeNullable)
  .Where(value => value != "Hello")
  .GetOrElse(() => throw new Exception("Unexpected....")());

[Mafii]

I found a more reasonable case where this behaviour can happen unexpectedly:

string? someStringThatCanBeNullable = null;
string? nullableFallbackValue = null;
Funcky.Monads.Option.FromNullable(someStringThatCanBeNullable)
  .Where(value => value != "Hello")
  .GetOrElse(nullableFallbackValue ?? throw new Exception("Unexpected...."));

The same can happen with ternary expressions, and all other cases where throw expressions can be part of an expression that directly returns a value (none on the top of my head but there might be some)

[Mafii]

Here's some possible rules I have in mind for the analyzer to show a warning (or not):

1. value passed to GetOrElse is a Func<T> or a method group -> show no warning
2. value passed is not Func<T> or a method group and contains a throw statement -> show warning
3. value passed is the result of a method call -> show warning (unless 4. is true)
4. value passed is the return value of a method that is called, but the method returns a type statisfying Point 1 -> show no warning

[FreeApophis]

See #521 and coordinate