-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Use correct exceptions #19212
Copy link
Copy link
Closed
Labels
Issue-Enhancementthe issue is more of a feature request than a bugthe issue is more of a feature request than a bugResolution-No ActivityIssue has had no activity for 6 months or moreIssue has had no activity for 6 months or moreUp-for-GrabsUp-for-grabs issues are not high priorities, and may be opportunities for external contributorsUp-for-grabs issues are not high priorities, and may be opportunities for external contributors
Metadata
Metadata
Assignees
Labels
Issue-Enhancementthe issue is more of a feature request than a bugthe issue is more of a feature request than a bugResolution-No ActivityIssue has had no activity for 6 months or moreIssue has had no activity for 6 months or moreUp-for-GrabsUp-for-grabs issues are not high priorities, and may be opportunities for external contributorsUp-for-grabs issues are not high priorities, and may be opportunities for external contributors
Summary of the new feature / enhancement
In PowerShell code base there are some places where we are not accurate with exceptions.
Typical example:
PowerShell/src/System.Management.Automation/engine/CommandInfo.cs
Lines 286 to 294 in 4314e63
Here we throw ArgumentNullException that is not correct if the argument is empty. (There are examples with other exceptions.)
My proposal is to fix such code and throw ArgumentNullException if argument is null and ArgumentException is argument is empty.
This is not even a breaking change, as it is not a functional exception, and it is worth correcting it for the correct one. If someone even uses this behavior in their code, it is catastrophically bad code.
The proposal comes from the fact that we are blocked from using new .Net API like ArgumentException.ThrowIfNullOrEmpty(). The API makes code more friendly for .Net Rumtime optimizations (like inlining). We already use this kind of API in a lot of places and it's worth updating the rest.
Proposed technical implementation details (optional)
No response