Bug 161412 - org.eclipse.jdt.internal.core.NamedMember#getFullyQualifiedParameterizedName probably boggus
Summary: org.eclipse.jdt.internal.core.NamedMember#getFullyQualifiedParameterizedName ...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 M3   Edit
Assignee: Jerome Lanneluc CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-18 10:10 EDT by Olivier Thomann CLA
Modified: 2007-09-07 05:01 EDT (History)
2 users (show)

See Also:

Attachments
Proposed fix (795 bytes, patch)
2006-10-24 13:03 EDT, Olivier Thomann CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Thomann CLA 2006-10-18 10:10:42 EDT 
In HEAD, the line:

		for (int i = 0; i < length; i++) {
			String typeArgument = typeArguments[i];
			typeArgument.replace('/', '.'); <== HAS NO EFFECT
			buffer.append(Signature.toString(typeArgument));
			if (i < length-1)
				buffer.append(',');
		}

This is probably a bug and should be replaced by:
			typeArgument = typeArgument.replace('/', '.');

This also needs regression tests.
Comment 1 Philipe Mulet CLA 2006-10-18 11:26:16 EDT 
Found by findbugs
http://findbugs.blogspot.com/2006/09/how-do-you-fix-obvious-bug.html
Comment 2 Olivier Thomann CLA 2006-10-24 10:33:21 EDT 
The type argument doesn't contain '/' anymore. The call:
String[] typeArguments = new BindingKey(uniqueKey).getTypeArguments();
returns a string[] where '/' have been replaced with '.'.
So no need to do it again.

Jérôme,

Could you please double-check that I didn't miss anything?

org.eclipse.jdt.core.tests.model.ResolveTests_1_5#test0083 seems to be a regression test for this case.

Thanks.
Comment 3 Olivier Thomann CLA 2006-10-24 13:03:41 EDT 
Created attachment 52610 [details]
Proposed fix
Comment 4 Olivier Thomann CLA 2006-10-24 13:05:15 EDT 
Released for 3.3M3.
All tests passed including existing ResolveTests_1_5#test0083.

Jérôme, just confirm that '/' cannot be inside typeArguments. The call typeArgument.replace('/', '.') is a noop.
Comment 5 Jerome Lanneluc CLA 2006-10-25 04:37:58 EDT 
The spec of BindingKey#getTypeArguments() doesn't specify whether the signature is dot separated or slash separated. However the current implemation of BindingKey#getTypeArguments() always returns a dot separated signature. Since we control both implementations and since we have a regression test, the change is ok.
Comment 6 Frederic Fusier CLA 2007-09-07 05:01:29 EDT 
Verified code using History...