Generate better names for expressions used as parameters. #19334
Conversation
|
Tagging @dotnet/roslyn-ide When generating names for expressions, we now check if the expression is in an argument location, and we use the name of the corresponding parameter to help inform the name. |
| @@ -556,7 +555,7 @@ public class @class | |||
| { | |||
| } | |||
|
|
|||
| public static void Add(object t) | |||
| public static void Add(object @class) | |||
There was a problem hiding this comment.
❕ This change makes it impossible to tell if the test is using the correct value for the renamed identifier. You should rename class @class at the same time so it's clear that the verbatim identifier comes from the parameter name, not the type name.
| If argument.IsNamed Then | ||
| Return DirectCast(argument, SimpleArgumentSyntax).NameColonEquals.Name.Identifier.ValueText | ||
| ElseIf Not argument.IsOmitted Then | ||
| Return semanticModel.GenerateNameForExpression(argument.GetExpression()) | ||
| Return semanticModel.GenerateNameForExpression( | ||
| argument.GetExpression(), capitalize:=False, cancellationToken:=cancellationToken) |
There was a problem hiding this comment.
❔ Is there no option for this in VB?
There was a problem hiding this comment.
i don't know what you're asking.
There was a problem hiding this comment.
➡️ As was explained by Cyrus in response to my other comment, the answer to my vague question is no such option current exists for parameters, so we pass False since that's what users are generally expecting today. 😄
| @@ -222,7 +224,8 @@ private int NonParamsParameterCount(IMethodSymbol method) | |||
| } | |||
| else | |||
| { | |||
| var name = semanticFacts.GenerateNameForExpression(semanticModel, expression); | |||
| var name = semanticFacts.GenerateNameForExpression( | |||
| semanticModel, expression, capitalize: false, cancellationToken: cancellationToken); | |||
There was a problem hiding this comment.
❔ Should this be following some style option?
There was a problem hiding this comment.
not sure what you're asking.
There was a problem hiding this comment.
Sorry about that, in both cases I was asking why capitalize was hard-coded as false.
There was a problem hiding this comment.
i see. Technically we likely want to use some sort of naming service that can follow user preferences. However, in practice, it's highly unlikely that users are going to do anything special with parameters, so i think it's fine to keep the current behavior (which is to make a camel-cased name).
|
Tagging @MattGertz Customer scenario Our code generation features try to generate good names when tehy can. One area this was lacking was when passing an expression as an argument to a method. In such a case we could use the name of the method parameter to help inform the generated name. Bugs this fixes: Fixes #2423 Workarounds, if any None. Risk Low. Performance impact low. Is this a regression from a previous update? No. Root cause analysis: This is a generate improvement. How was the bug found? From dogfooding. |
Fixes #2423