Community
Participate
Working Groups
R3.5 and I20090714-0800 (introduced in R3.5). NamingConventions.suggestVariableNames(int, int, String, IJavaProject, int, String[], boolean) doesn't work if name contains '_'. Test Case, see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=263058#c6 Basically, we call the method like this: NamingConventions.suggestVariableNames(NamingConventions.VK_PARAMETER, NamingConventions.BK_TYPE_NAME, "lMinTriggerPeriod_usec", project, 0, [], true); And get this back: ["lMinTriggerPeriodUsec", "minTriggerPeriodUsec", "triggerPeriodUsec", "periodUsec", "usec"] The Javadoc says: * The different kinds of base names are: * <ul> * <li>{@link #BK_NAME}: the base name is a Java name and the whole base name is considered to compute the variable names. A prefix and a suffix can be added.<br> * There is a heuristic by variable kind. * <ul> * <li>{@link #VK_PARAMETER}, {@link #VK_LOCAL}, {@link #VK_INSTANCE_FIELD} and {@link #VK_STATIC_FIELD}:<br> * In this case the first character will be converted to lower case and the other characters won't be changed.<br> * If the base name is <code>SimpleName</code> then the suggested name will be <code>simpleName</code>.<br></li>
So you expect the '_' character to be there in the proposals ?
Right, that's what I would expect from reading the Javadoc.
This might impact existing users. So we might want to consider fixing the documentation as well.
>So we might want to consider fixing the documentation as well. You mean instead of fixing the bug?
Yes. If existing users have some code that relies on removing the '_', they would be broken once this is fixed.
The API was made on request from JDT UI and most likely they are the only clients of it and currently it's breaking their features. If JDT Core only intends to update the doc then you have to provide a new working API for JDT UI and deprecate the old one.
Ok, I was not aware of this. We should then fix the bug.
So, what do we expect for inputs like these - "MY_FIRST_Type" and "my_first_Type". Right now the output is {"myFIRSTType", "firstType","type"} and {"myFirstType", "firstType", "type"} respectively. I think we need to come up with the scenarios where '_' will be considered a delimiter.
I don't expect any special processing of '_', since '_' has no special meaning inside a base name. If the original name contains '_' and is the name of a constant, clients should already have passed it to NamingConventions.getBaseName(NamingConventions.VK_STATIC_FINAL_FIELD, ..) to convert it into a baseName and get rid of the '_'s. So, I expect what the Javadoc says: - "lMinTriggerPeriod_usec" -> "lMinTriggerPeriod_usec", "minTriggerPeriod_usec", "triggerPeriod_usec", "period_usec" - "MY_FIRST_Type" -> "mY_FIRST_Type" - "my_first_Type" -> "my_first_Type" As an alternative, you could also consider '_' as a word-separator, i.e.: - "lMinTriggerPeriod_usec" -> "lMinTriggerPeriod_usec", "minTriggerPeriod_usec", "triggerPeriod_usec", "period_usec", "usec" - "MY_FIRST_Type" -> "mY_FIRST_Type", "fIRST_TYPE", "tYPE" - "my_first_Type" -> "my_first_Type", "first_Type", "type"
I had a look at the NamingConvensions code and the tests and it looks like the code has been written to convert the first word to lower case instead of the first letter, as mentioned in the doc. I just confirmed this with David. So, do we want to keep this behavior and fix the '_' problem? This will produce outputs like these: MY_FIRST_Type -> "my_FIRST_Type", "first_Type", "type" my_first_Type -> "my_First_Type", "first_Type", "type"
That would also be fine with me, iff you fix the Javadoc and > my_first_Type -> "my_First_Type", [..] was a typo and really meant: -> "my_first_Type"
Created attachment 147076 [details] Proposed Patch Attaching the patch with documentation change and tests. The fix required changes to some of the existing tests.
I didn't verify the patch in detail but the general direction looks good and I verified that - it fixes the original issue from bug 263058 comment 6 - our JDT Text and JDT UI tests still pass
Released in HEAD for 3.6M3 (after minor indentation changes)
Set target milestone.
Verified for 3.6M3 using build I20091026-2000
