Community
Participate
Working Groups
20030521 NamingConventions offers methods to suggest a field/argument or variable name for a given _type_. In the Getter/Setter wizard, people want to have an argument name based on the _field name_. --- private int count; -> setCount(int _count) (assuming _ is the argument-prefix) --- So as a trick I used NamingConventions.suggestArgumentNames but with the (normalized) field name as 'type'. This works nicely. However, from the API side this looks strange and like a hack. public static char[][] suggestArgumentNames(IJavaProject javaProject, char[] packageName, char[] qualifiedTypeName, int dim, char[][] excludedNames) { I was wondering: Why do I have to pass a 'packageName'? AFAIK, it is not used in the algorithm. Couldn't we get rid of 'packageName' and also rename 'qualifiedTypeName' with 'baseName'? Note that for types like 'int', 'packageName' is empty anyway. This effects all suggest methods. Note that it might also be better to only offer the String variants. Another reason for a complete reorg would be the oportunity to merge prefix and suffix lists to a list like 'f*, f*_, *_, *' (* represents the base name). This would solve the problem of knowing the user's favorite choice and to express if he/she whant prefix AND suffix together. (let me know if you prefer to have this in a separate bug report)
I just discovered a problem with my hacky solution: When passing in a field name like 'isSubType', the environment suggests a name 'type', as it shortens the name (isSubType, subType, type) So it might be better to have a second guessArgument method that is optimixed for normal names (not type names) and does not shorten, or we add an extra flag to hint that long names are preferred (Note that this also might be a user preference for type names: Always take the longest name)
Will reconsider post 3.0
Another strangeness: For AbType, suggestions are "type" and "abType". But for ABType, the only suggestion is "type".
considering for 3.4
We have worked out a API suggestion that has the power we need. You can basically move it down as is; its a facade to the existing APIs plus missing functionality. We also changed almost all of the references to NamingConventions in JDT.UI to use this API. StubUtility.getVariableNameSuggestions(): public static final int STATIC_FIELD= 1; public static final int INSTANCE_FIELD= 2; public static final int CONSTANT_FIELD= 3; public static final int PARAMETER= 4; public static final int LOCAL= 5; /** * Returns variable name suggestions for the given base name. This is a * layer over the JDT.Core NamingConventions API to fix its shortcomings. * JDT UI code should only use this API. * @param variableKind Specifies what type the variable is: {@link #LOCAL}, * {@link #PARAMETER}, {@link #STATIC_FIELD}, {@link #INSTANCE_FIELD} or * {@link #CONSTANT_FIELD}. * @param project the current project * @param baseName the base name to make a suggestion on. the base name * is expected to be a name without any pre- or suffixes in singular form. * Type name are accepted as well. * @param dimensions if greater than 0, the resulting name will be in * plural form * @param excluded a collection containing all excluded names or * <code>null</code> if no names are excluded * @param evaluateDefault if set, the result is guaranteed to contain * at least one result. If not, the result can be an empty array. * * @return returns the name suggestions sorted by relevance (best proposal * first). If <code>evaluateDefault</code> is set to true, the returned * array is never empty. * If <code>evaluateDefault</code> is set to false, an empty array is * returned if there is no good suggestion for the given base name. */ public static String[] getVariableNameSuggestions(int variableKind, IJavaProject project, String baseName, int dimensions, Collection excluded, boolean evaluateDefault) { public static String removePrefixAndSuffixForVariable(IJavaProject project, IVariableBinding binding) { Built on these APIs is the advanced functionality is the guessing of a variable name by the assigned type or expression: public static String[] getVariableNameSuggestions(int variableKind, IJavaProject project, ITypeBinding expectedType, Expression assignedExpression, Collection excluded) { public static String[] getVariableNameSuggestions(int variableKind, IJavaProject project, Type expectedType, Expression assignedExpression, Collection excluded) {
Time permitting for 3.4
Deferring post 3.4
Targeting M3
Created attachment 113229 [details] API proposal Several improvements are asked in this bug 1) Do not generate name from a package name (not really used) and a simple type name but generate the name from a base name. 2) Propose only one method to generate all kinds of variable name 3) Propose only methods with String parameters 4) Be able to generate only the longest variable name 5) Use a new heuristic to generate constant field name (bug 85946) 6) Change options for prefixes and suffixes I propose the following API to solve the points 1, 2, 3, 4 and 5. These API does not solve the point 6 but before investigate this issue i would like to know if you really need these new options. The proposed API look like the API suggested in comment 5. /** * Suggest names for a variable. The name is computed from a base name * and possible prefixes or suffixes are added. * <p> * The variable can be a local variable, a parameter, an instance field, a static field or a constant field. * Each type of variable is specified by a variable kind: * <ul> * <li>{@link #VK_PARAMETER} for parameter</li> * <li>{@link #VK_LOCAL} for local variable</li> * <li>{@link #VK_INSTANCE_FIELD} for instance field</li> * <li>{@link #VK_STATIC_FIELD} for static field</li> * <li>{@link #VK_CONSTANT_FIELD} for constant field</li> * </ul> * Some other kinds could be added in the future. * </p> * <p> * The base name is used to compute the variable name. * Some different kinds of base name are possible and each kind is associated to a different heuristic to compute variable names. * The different kinds of base name are: * <ul> * <li>{@link #BK_SIMPLE_NAME}: If the prefix is <code>pre</code> and the suffix is <code>suf</code> then the proposed name is * <code>preSimpleNamesuf</code>.</li> * <li>{@link #BK_SIMPLE_TYPE_NAME}: If the type is <code>TypeName</code>, the prefix is <code>pre</code> and the suffix is <code>suf</code> * then the proposed names are <code>preTypeNamesuf</code> and <code>preNamesuf</code>.</li> * </ul> * Some other kinds could be added in the future. * </p> * <p> * Each variable kind is affected by the following JavaCore options: * <ul> * <li>{@link #VK_PARAMETER}: {@link JavaCore#CODEASSIST_ARGUMENT_PREFIXES} and {@link JavaCore#CODEASSIST_ARGUMENT_SUFFIXES}</li> * <li>{@link #VK_LOCAL}: {@link JavaCore#CODEASSIST_LOCAL_PREFIXES} and {@link JavaCore#CODEASSIST_LOCAL_SUFFIXES}</li> * <li>{@link #VK_INSTANCE_FIELD}: {@link JavaCore#CODEASSIST_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_FIELD_SUFFIXES}</li> * <li>{@link #VK_STATIC_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FIELD_SUFFIXES}</li> * <li>{@link #VK_CONSTANT_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_SUFFIXES}</li> * </ul> * </p> * <p> * For a complete description of these configurable options, see <code>getDefaultOptions</code>. * For programmaticaly change these options, see <code>JavaCore#setOptions()</code>. * </p> * * @param variableKind specifies what type the variable is: {@link #VK_LOCAL}, {@link #VK_PARAMETER}, {@link #VK_STATIC_FIELD}, * {@link #VK_INSTANCE_FIELD} or {@link #VK_CONSTANT_FIELD}. * @param baseNameKind specifies what type the base name is: {@link #BK_SIMPLE_NAME} or {@link #BK_SIMPLE_TYPE_NAME} * @param baseName name used to compute the suggested names. * @param javaProject project which contains the variable. * @param dim variable dimension (0 if the field is not an array). * @param excluded a list of names which cannot be suggested (already used names). * Can be <code>null</code> if there is no excluded names. * @param evaluateDefault if set, the result is guaranteed to contain at least one result. If not, the result can be an empty array. * @return String[] an array of names. * @see JavaCore#setOptions(java.util.Hashtable) * @see JavaCore#getDefaultOptions() */ public static String[] getVariableNames( int variableKind, int baseNameKind, String baseName, IJavaProject javaProject, int dim, String[] excluded, boolean evaluateDefault) {} If we change the method to suggest variable name we also need to update the method to remove prefix and suffix /** * Remove prefix and suffix from a variable name. * * <p> * The variable can be a local variable, a parameter, an instance field, a static field or a constant field. * Each type of variable is specified by a variable kind: * <ul> * <li>{@link #VK_PARAMETER}</li> * <li>{@link #VK_LOCAL}</li> * <li>{@link #VK_INSTANCE_FIELD}</li> * <li>{@link #VK_STATIC_FIELD}</li> * <li>{@link #VK_CONSTANT_FIELD}</li> * </ul> * </p> * <p> * If variable name prefix is <code>pre</code> and variable name suffix is <code>suf</code> * then for a variable named <code>preVariablesuf</code> the result of this method is <code>variable</code>. * If there is no prefix or suffix defined in JavaCore options the result is the unchanged * name <code>preVariablesuf</code>. * </p> * <p> * For each variable kind this method is affected by the following JavaCore options: * <ul> * <li>{@link #VK_PARAMETER}: {@link JavaCore#CODEASSIST_ARGUMENT_PREFIXES} and {@link JavaCore#CODEASSIST_ARGUMENT_SUFFIXES}</li> * <li>{@link #VK_LOCAL}: {@link JavaCore#CODEASSIST_LOCAL_PREFIXES} and {@link JavaCore#CODEASSIST_LOCAL_SUFFIXES}</li> * <li>{@link #VK_INSTANCE_FIELD}: {@link JavaCore#CODEASSIST_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_FIELD_SUFFIXES}</li> * <li>{@link #VK_STATIC_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FIELD_SUFFIXES}</li> * <li>{@link #VK_CONSTANT_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_SUFFIXES}</li> * </ul> * </p> * <p> * For a complete description of these configurable options, see <code>getDefaultOptions</code>. * For programmaticaly change these options, see <code>JavaCore#setOptions()</code>. * </p> * * @param variableKind specifies what type the variable is: {@link #VK_LOCAL}, {@link #VK_PARAMETER}, {@link #VK_STATIC_FIELD}, * {@link #VK_INSTANCE_FIELD} or {@link #VK_CONSTANT_FIELD}. * @param javaProject project which contains the variable. * @param name variable name. * @return String the name without prefix and suffix. * @see JavaCore#setOptions(java.util.Hashtable) * @see JavaCore#getDefaultOptions() * * @since 3.5 */ public static String removeVariablePrefixAndSuffix( int variableKind, IJavaProject javaProject, String name) {} What do you think about these new API ?
a) The patch is missing a few @since tags and Javadocs, which should be flagged by the API tooling if you set 3.4 as baseline (vote for bug 233643 if it's too slow for you). b) > 4) Be able to generate only the longest variable name I guess that's BK_SIMPLE_NAME vs. BK_SIMPLE_TYPE_NAME? This is lacking some documentation. The doc I found was in getVariableNames(..), where examples are not really self-explanatory (and for BK_SIMPLE_NAME, even the used baseName is missing). Could you document the constants and add some more examples that also show what happens with baseNames with different casing (start lowercase: "typeName", start with multi caps: "URLTypeName"), and tell explicitly that BK_SIMPLE_TYPE_NAME considers camel case segments while BK_SIMPLE_NAME does not? c) I think it's a bug that BK_SIMPLE_TYPE_NAME skips the first segment if it starts lowercase or is all caps, e.g. testSuggestFieldName021 with "myType" or "MYType" should return both "MY_TYPE" and "TYPE". StubUtility has workarounds for this, but the new API should not require further processing on the client side. d) > * @return name suggestions sorted by relevance (best proposal first) I'm missing this in the implementation. Sorting order should be: 1. longest proposal with pre- and suffixes 2. proposals with camel case segments removed, with pre- and suffixes, longer proposals first 3... same without pre- and suffixes If you implement (g) below, I would expect: 1. all proposals with first pattern, longer proposals first 2. all proposals with second pattern, longer proposals first ... e) It would be nice if getVariableNames(..) also worked with a null project (takes workspace settings). f) I would rename getVariableNames(..) to suggestVariableNames(..), for consistency with suggestGetterName(..), etc. g) > 6) Change options for prefixes and suffixes This is not a technical requirement, but it would be a big advantage for users, see last paragraph of comment 0. E.g. if you look at org.eclipse.jdt.core.tests.model.NamingConventionTests.testSuggestFieldName006(), I doubt that any user wants this big list of proposals. They're probably only interested in {"preNamesuf", "preOneNamesuf"}, but there's no way to express that currently. h) removeVariablePrefixAndSuffix(..) should be the reverse of suggestVariableNames(..). For constants, it should also convert the ALL_CAPS name to camelCase (see StubUtility#getCamelCaseFromUpper(..)), e.g: public void testRemovePrefixAndSuffixForConstantName001() { Hashtable options = JavaCore.getOptions(); options.put(JavaCore.CODEASSIST_FIELD_PREFIXES,"PRE_"); //$NON-NLS-1$ options.put(JavaCore.CODEASSIST_FIELD_SUFFIXES,"SUF"); //$NON-NLS-1$ JavaCore.setOptions(options); String name = NamingConventions.removeVariablePrefixAndSuffix( NamingConventions.VK_CONSTANT_FIELD, this.project, "PRE_ONE_NAMESUF"); //$NON-NLS-1$ assumeEquals( "oneName", //$NON-NLS-1$ name); } Since the method then does not only remove pre-/suffixes any more, I'd rename it to getBaseName(..). Issues (c), (d), and (g) make the new API incompatible with the old one. I think you should leave the old implementation for the existing API, deprecate it, and maybe add a little tweak to make it still work even when the project does not contain the old pre-/suffix options but only the new pattern-based options. Then you are free to include all fixes in the new API.
Created attachment 115175 [details] Proposed fix This is an update of the previous patch which take into account most of the remarks from Markus (a, b, c, d, e, f, h). I did not change options for prefixes and suffixes (g) because i think it is a separate feature request that is not required to fix this feature request. I entered a separate bug for this feature (bug 250991). The modified API are /** * Suggests names for a variable. The name is computed from a base name and possible prefixes or suffixes are added. * * <p> * The base name is used to compute the variable name. * Some different kinds of base name are possible and each kind is associated to a different heuristic to compute variable names.<br> * The heuristic depends also of the kind of the variable. Each kind of variable is identified by a constant starting with <code>VK_</code>.<br> * When a prefix and a suffix can be added then all combinations of prefix and suffix are suggested. * If the name is <code>name</code>, the prefix is <code>pre</code> and the suffix is <code>suf</code> then the suggested names will be * <code>prenamesuf</code>, <code>prename</code>, <code>namesuf</code> and <code>name</code>.<br> * <br> * The different kinds of base name are: * <ul> * <li>{@link #BK_NAME}: the whole base name is considered to compute the variable names. A prefix and a suffix can be added.<br> * There is an 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> * <li>{@link #VK_CONSTANT_FIELD} :<br> * In this case all letters of the name will be converted to upper case and words will be separated by an underscore (<code>"_"</code>).<br> * If the base name is <code>SimpleName</code> then the suggested name will be <code>SIMPLE_NAME</code>.</li> * </ul></li> * <li>{@link #BK_TYPE_NAME}: all the words of the base name are considered to compute the variable names. A prefix and a suffix can be added to these names.<br> * There is an 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 a variable name will contain some words of the base name and the first character will be converted to lower case.<br> * If the type is <code>TypeName</code> then the suggested names will be <code>typeName</code> and <code>name</code>.</li> * <li>{@link #VK_CONSTANT_FIELD} :<br> * In this case a variable name will contain some words of the base name, all letters of the name will be converted to upper case and segments will be separated by a underscore (<code>"_"</code>).<br> * If the base name is <code>TypeName</code> then the suggested name will be <code>TYPE_NAME</code> and <code>NAME</code>.</li> * </ul></li> * </ul> * Some other kinds could be added in the future. * </p> * <p> * Each variable kind is affected by the following JavaCore options: * <ul> * <li>{@link #VK_PARAMETER}: {@link JavaCore#CODEASSIST_ARGUMENT_PREFIXES} and {@link JavaCore#CODEASSIST_ARGUMENT_SUFFIXES}</li> * <li>{@link #VK_LOCAL}: {@link JavaCore#CODEASSIST_LOCAL_PREFIXES} and {@link JavaCore#CODEASSIST_LOCAL_SUFFIXES}</li> * <li>{@link #VK_INSTANCE_FIELD}: {@link JavaCore#CODEASSIST_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_FIELD_SUFFIXES}</li> * <li>{@link #VK_STATIC_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FIELD_SUFFIXES}</li> * <li>{@link #VK_CONSTANT_FIELD}: {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_PREFIXES} and {@link JavaCore#CODEASSIST_STATIC_FINAL_FIELD_SUFFIXES}</li> * </ul> * </p> * <p> * For a complete description of these configurable options, see <code>getDefaultOptions</code>. * For programmaticaly change these options, see <code>JavaCore#setOptions()</code>. * </p> * <p> * Proposed names are sorted by relevance (best proposal first).<br> * The names are proposed in the following order: * <ol> * <li>Names with prefix and suffix. Longest name are proposed first</li> * <li>Names with prefix. Longest name are proposed first</li> * <li>Names with suffix. Longest name are proposed first</li> * <li>Names without prefix and suffix. Longest name are proposed first</li> * </ol> * </p> * * @param variableKind specifies what type the variable is: {@link #VK_LOCAL}, {@link #VK_PARAMETER}, {@link #VK_STATIC_FIELD}, * {@link #VK_INSTANCE_FIELD} or {@link #VK_CONSTANT_FIELD}. * @param baseNameKind specifies what type the base name is: {@link #BK_NAME} or {@link #BK_TYPE_NAME} * @param baseName name used to compute the suggested names. * @param javaProject project which contains the variable or <code>null</code> to take into account only workspace settings. * @param dim variable dimension (0 if the field is not an array). * @param excluded a list of names which cannot be suggested (already used names). * Can be <code>null</code> if there is no excluded names. * @param evaluateDefault if set, the result is guaranteed to contain at least one result. If not, the result can be an empty array. * @return String[] an array of names. * @see JavaCore#setOptions(java.util.Hashtable) * @see JavaCore#getDefaultOptions() * * @since 3.5 */ public static String[] suggestVariableNames( int variableKind, int baseNameKind, String baseName, IJavaProject javaProject, int dim, String[] excluded, boolean evaluateDefault) /** * Returns a base name which could be used to generate this variable name with {@link #suggestVariableNames(int, int, String, IJavaProject, int, String[], boolean)}. * <p> * e.g.<br> * If the variable is a {@link #VK_LOCAL} and the variable name is <code>variableName</code> then the base name will be <code>VariableName</code>.<br> * If the variable is a {@link #VK_CONSTANT_FIELD} and the variable name is <code>VARIABLE_NAME</code> then the base name will be <code>VariableName</code>.<br> * </p> * * @param variableKind specifies what type the variable is: {@link #VK_LOCAL}, {@link #VK_PARAMETER}, {@link #VK_STATIC_FIELD}, * {@link #VK_INSTANCE_FIELD} or {@link #VK_CONSTANT_FIELD}. * @param variableName a variable name * @param javaProject project which contains the variable or <code>null</code> to take into account only workspace settings. * * @see #suggestVariableNames(int, int, String, IJavaProject, int, String[], boolean) * @since 3.5 */ public static String getBaseName( int variableKind, String variableName, IJavaProject javaProject)
Created attachment 115379 [details] Updated patch Correct some bugs of the previous patch and improve the algorithm to compute names. With this patch the following JDT/UI tests fail but i think that the new behavior is better. IntroduceIndirectionTests#test07() - e1Inner1 is suggested instead of inner1 PromoteTempToFieldTests#test22() - fSortByDefiningTypeAction is suggested instead of sortByDefiningTypeAction UnresolvedMethodsQuickFixTest#testParameterMismatchChangeFieldType() - fCount2 is suggested instead of count and OptionsConfigurationBlockTest#testKeysForOptions()
Created attachment 115780 [details] Patch that not modify the behavior of the old API This patch add the new API but does not modify the behavior of the old API. I will release this patch and the old API will use the behavior of the new API when we will have enough feedback about this API in 3.5M4. With this patch one JDT/UI test fails OptionsConfigurationBlockTest#testKeysForOptions() because the patch add new JavaCore options. I will attach a patch to modify this JDT/UI test.
Created attachment 115781 [details] Patch for JDT/UI tests
Released for 3.5M3. Tests added NamingConventionTests#testGetBaseName001() -> testGetBaseName004() NamingConventionTests#testSuggestFieldName021() -> testSuggestFieldName038() CompletionTests#testCompletionFieldName3() -> testCompletionFieldName7() Daniel released a fix to pass test testKeysForOptions()
The old API should use the new behavior, i entered bug 251693 to track this.
Verified for 3.5M3 using I20081026-2000
*** Bug 166464 has been marked as a duplicate of this bug. ***
