Bug 255345

Summary:

Problems in new NamingConventions APIs

Product:

[Eclipse Project] JDT

Reporter:

Markus Keller <markus.kell.r>

Component:

Core

Assignee:

David Audel <david_audel>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

david_audel, jerome_lanneluc, srikanth_sankaran

Version:

3.5

Target Milestone:

3.5 M4

Hardware:

OS:

Windows XP

Whiteboard:

Bug Depends on:

Bug Blocks:

251670

Attachments:

Description	Flags
Javadoc patch for NamingConventions	none
Proposed fix	none

Description Markus Keller

2008-11-14 08:00:31 EST

I20081112-1200

While adopting the new NamingConventions APIs from bug 38111, I ran into a few issues:

- VK_CONSTANT_FIELD should be called VK_STATIC_FINAL_FIELD (we should continue reserving the term 'constant' for compile-time constants)

- suggestSetter/GetterName(..) methods should also consider CODEASSIST_STATIC_FINAL_FIELD_*FIXES constants and use getBaseName(..) to convert e.g. 'K_CONST_ANT' into 'getConstAnt' (with static final prefix "K_")

- suggestVariableNames(CONSTANT_FIELD, BK_TYPE_NAME, "int", ..) returns "i", but I would expect "I" or "INT" (our old implementation returned "INT").
  - For baseName "A", it should return "A", not "a" (problem with single letters)

- getBaseName(..): Javadoc should tell that this method also strips pre- and suffixes, depending on the variable kind and the settings in the given project

- see other Javadoc corrections in the attached patch (typos and missing references to new constants)

In the fix for bug 251670, I added StubUtility.NAMING_CONVENTIONS_BUGS which should shield JDT/UI from these problems (except for the VK_CONSTANT_FIELD rename).

Comment 1 David Audel

2008-11-17 05:21:46 EST

(In reply to comment #0)
> - see other Javadoc corrections in the attached patch (typos and missing
> references to new constants)

It seems that you forgot to attach the patch

Comment 2 Markus Keller

2008-11-17 06:27:51 EST

Created attachment 118040 [details]
Javadoc patch for NamingConventions

> It seems that you forgot to attach the patch
Oops, sorry about that.

Comment 3 Markus Keller

2008-11-18 09:50:18 EST

I just realized we had a test failure in CodeCompletionTest#testGetterCompletion1() due to a subtle difference between NamingConventions#getBaseName(..) and the now-deprecated removePrefixAndSuffixFor*Name(..) methods.

In the old days, the base name for e.g. field "fWriter" with prefix "f" was "writer". Now, getBaseName() returns "Writer" (with capital first letter).

I've put a workaround for this into StubUtility, but I think getBaseName(..) should return uncapitalized names, since it returns a *variable* name, and those are traditionally uncapitalized. Together with 10.c) from bug 38111, this would completely spare clients from having to care about casing of names.

Comment 4 David Audel

2008-11-20 04:31:26 EST

Created attachment 118341 [details]
Proposed fix

This patch contains the following modifications:
- VK_STATIC_FINAL_FIELD is added to replace VK_CONSTANT_FIELD. I will remove VK_CONSTANT_FIELD when you will use VK_STATIC_FINAL_FIELD.
	VK_CONSTANT_FIELD was shorter but VK_STATIC_FINAL_FIELD is closer to the name of the associated options CODEASSIST_STATIC_FINAL_FIELD_*FIXES
- suggestVariableNames() returns capitalized type name for base type (int -> INT).
- suggestVariableNames() returns capitalized name when the name contains only one letter (a -> A).
- Javadoc of getBaseName() is improved.
- getBaseName() returns name starting with a lowercase.

I also corrected typos and missing references to new constants (thanks for the patch)

I did not modify suggestSetter/GetterName. I will fix them with the fix for bug 251693

Comment 5 David Audel

2008-11-20 04:35:25 EST

Released for 3.5M4.

Tests added
  NamingConventions#testSuggestFieldName039() -> testSuggestFieldName040()

Tests updated
  NamingConventions#testGetBaseName001() -> testGetBaseName004()
  NamingConventions#testSuggestFieldName021() -> testSuggestFieldName022()
  NamingConventions#testSuggestFieldName025() -> testSuggestFieldName028()

Comment 6 Markus Keller

2008-11-20 10:56:38 EST

Thanks, I've removed the references to VK_CONSTANT_FIELD and the workarounds in JDT/UI.

Comment 7 Srikanth Sankaran

2008-12-09 04:24:57 EST

Verified for 3.5M4 using I20081208-1800 build