-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Use nameof in System.Linq.Expressions #6272
Conversation
Mostly automated replacement of parameter name string literals with corresponding usage of nameof.
|
👍 There were a couple of tricky cases around some of the Linq code that i paid attention to, but looks good. Also made your no public API parameter names were changed. |
| } | ||
|
|
||
| public static Expression ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arg, ParameterInfo pi) | ||
| public static Expression ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arg here comes from arguments arguments to public members, and hence "arguments" should be the argument name in an exception that goes back to such methods, either by changing the name or by not using nameof in this case.
It does though mean that at this point we have an arguments for a single argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood on the reasoning (it's the same reasoning that applies to some of the others that looked correct in this PR), but the path back to the public surface area wasn't obvious... does this always come from calls to InvocationExpression.Invoke?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this assembly it can come from Expression.Goto(), Expression.Invoke(), Expression.Call() or Expression.New(), with arguments being the correct name each time. It's also called in Dynamic.Runtime, again with the appropriately named arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks.
|
There were several places where you changed a parameter name. Most of them looked like the correct change, but on quick inspection I wasn't sure about one of them, which I flagged. Otherwise, LGTM. cc: @VSadov |
Use nameof in System.Linq.Expressions
…sions Use nameof in System.Linq.Expressions Commit migrated from dotnet/corefx@5e85625
Mostly automated replacement of parameter name string literals with corresponding usage of
nameof(via a modified version of @jaredpar's https://github.com/jaredpar/UseNameOf) along the lines of #6209, #6265, #6267, and #6271.cc: @stephentoub, @VSadov