Use string.IsNullOrEmpty and ArgumentException.ThrowIfNullOrEmpty in many more places

[stephentoub]

We have an analyzer for ThrowIfNullOrEmpty; I'll separately look into why it wasn't firing.

We should consider an analyzer for string.IsNullOrEmpty. There's no perf benefit today, other than smaller IL, but the resulting code is easier to read IMHO, and there can be a benefit in the few cases where a complicated expression is duplicated and the JIT can't CSE the whole thing.

[ghost]

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

---

We have an analyzer for ThrowIfNullOrEmpty; I'll separately look into why it wasn't firing.

We should consider an analyzer for string.IsNullOrEmpty. There's no perf benefit today, other than smaller IL, but the resulting code is easier to read IMHO, and there can be a benefit in the few cases where a complicated expression is duplicated and the JIT can't CSE the whole thing.

Author:	stephentoub
Assignees:	stephentoub
Labels:	area-Meta
Milestone:	-

[am11]

Unfortunately, forward substitution still does not handle IsNullOrEmpty() vs. ? true : false, as latter is better in some cases. i.e. in https://godbolt.org/z/rc4Kx56aq M2 is worse than M3 and M1 has the best code.

[stephentoub]

Unfortunately, forward substitution still does not handle IsNullOrEmpty() vs. ? true : false, as latter is better in some cases. i.e. in https://godbolt.org/z/rc4Kx56aq M2 is worse than M3 and M1 has the best code.

Is this just a general comment or is this relevant to the changes made in this PR? This PR didn't remove any uses of ? true : false, and in your godbolt example the differences between s is null || s.Length == 0 vs string.IsNullOrEmpty() are purely about the number of returns as used by the C# compiler:
SharpLab
and even that difference evaporates if you do anything other than directly return the result of the operation, which also doesn't happen in this PR.

[MichalPetryka]

are purely about the number of returns as used by the C# compiler

In this case the number of returns matches.

[stephentoub]

In this case the number of returns matches.

Yes, but again, where in this PR is there code that does that?

(And even if there is an example or two, it doesn't matter in practice... and even if it did matter in practice, that's an issue to be addressed in the JIT, not by changing the code a developer would write to avoid idiomatic patterns.)