-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[generator] Ensure studly-case doesn't produce invalid C# field name. #451
Conversation
@@ -110,9 +110,14 @@ public static Field CreateField (XElement elem) | |||
|
|||
if (elem.Attribute ("managedName") != null) | |||
field.Name = elem.XGetAttribute ("managedName"); | |||
else | |||
else { | |||
field.Name = TypeNameUtilities.StudlyCase (char.IsLower (field.JavaName [0]) || field.JavaName.ToLower ().ToUpper () != field.JavaName ? field.JavaName : field.JavaName.ToLower ()); |
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.
Not part of the bug, obviously, but since I noticed it...
Shouldn't this use .ToLowerInvariant()
and .ToUpperInvariant()
, so that we avoid the "Turkish-I" problem?
field.Name = TypeNameUtilities.StudlyCase (char.IsLower (field.JavaName [0]) || field.JavaName.ToLower ().ToUpper () != field.JavaName ? field.JavaName : field.JavaName.ToLower ()); | ||
|
||
// Preserve starting underscore if 2nd char is a number (removing it is an invalid C# name) | ||
if (field.Name.Length > 0 && char.IsNumber (field.Name [0]) && field.JavaName.StartsWith ("_")) |
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 care about the Java side in this case? $
is also a valid Java identifier character, but not in C#, so what if we hit:
class Example {
public int $x;
}
(Yes, this compiles. I have no idea what will happen with binding. Would be interesting to find out.)
If field.Name[0]
is not a valid "start identifier" character -- or method[0]
, or property[0]
, or anything[0]
[0]
! -- then we need to "escape" it somehow, probably by prefixing with _
. And if field.JavaName
is $x
, then we still won't properly escape...which would be a problem.
We did not have any support for handling the dollar-sign case. I added code that will replace the |
One issue with the BouncyCastle .jar in #424 is that it contains this field:
We studly-case Java field names, however that removes underscores and we end up with
3desEdeCbc
, which is invalid C#. This PR preserves a starting underscore if there was one and the resulting name starts with a number. (_3desEdeCbc
for this case.)