This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Fix ArgumentNullException messages passed as parameter names #2889
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Is it still to soon to use nameof() in coreclr? |
|
I think it's corefx only at the moment |
| @@ -36,7 +36,7 @@ public abstract class FieldInfo : MemberInfo, _FieldInfo | |||
| public static FieldInfo GetFieldFromHandle(RuntimeFieldHandle handle) | |||
| { | |||
| if (handle.IsNullHandle()) | |||
| throw new ArgumentException(Environment.GetResourceString("Argument_InvalidHandle")); | |||
| throw new ArgumentException(Environment.GetResourceString("Argument_InvalidHandle"), "handle"); | |||
There was a problem hiding this comment.
Are these parameters backwards i.e. shouldn't "handle" come before the GetResourceString call?
There was a problem hiding this comment.
I don't think so, as this is an ArgumentException
There was a problem hiding this comment.
Nvm, just looked it up and the order is reversed for ArgumentException. That's confusing.
There was a problem hiding this comment.
Very! Stumped me more than a few times
|
Changes look good to me. |
Fixed several ArgumentNullExceptions that throw with the message as the parameter name, leading to potential confusion for developers.
|
@jkotas please take a look, this has been reviewed but unmerged for a while :) |
|
LGTM. Thanks! Sorry for the delay. Would you mind porting the applicable fixes to CoreRT as well? |
|
Will do |
jkotas
pushed a commit
to dotnet/corert
that referenced
this pull request
May 10, 2016
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
…coreclr#2889) Fixed several ArgumentNullExceptions that throw with the message as the parameter name, leading to potential confusion for developers. Commit migrated from dotnet/coreclr@ad2485b
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found one weird parameter name, and git grepped the mscorlib folder to fix the others