Bug 185694 - [introduce parameter object] Introduce Parameter Object substitutes wrong class
Summary: [introduce parameter object] Introduce Parameter Object substitutes wrong class
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 RC2   Edit
Assignee: Karsten Becker CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-05-06 12:01 EDT by Missing name Mising name CLA
Modified: 2007-06-06 12:27 EDT (History)
2 users (show)

See Also:
martinae: review+
markus.kell.r: review+


Attachments
Some refactoring + better import of added Parameter (42.97 KB, patch)
2007-05-21 05:47 EDT, Karsten Becker CLA
no flags Details | Diff
Changes for Comment #3 (49.28 KB, patch)
2007-05-23 09:02 EDT, Karsten Becker CLA
no flags Details | Diff
Updated Patch for default package tests (52.13 KB, patch)
2007-05-23 14:04 EDT, Karsten Becker CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Missing name Mising name CLA 2007-05-06 12:01:15 EDT
3.3M7 I20070503-1400

public class A {
	public static void main(String[] args) {
		method(5, 6);
	}

	public static void method(int x, int y) {
		System.out.println(x + ", " + y);
	}
}

Extract a parameter object class Point in a separate CU on x any y and name it point.  The result is:

public class A {
	public static void main(String[] args) {
		method(new Point(5, 6));
	}

	public static void method(java.awt.Point point) {
		System.out.println(point.x + ", " + point.y);
	}
}

The method call no longer matches the signature, which now references java.awt.Point.
Comment 1 Karsten Becker CLA 2007-05-21 05:47:35 EDT
Created attachment 67952 [details]
Some refactoring + better import of added Parameter
Comment 2 Karsten Becker CLA 2007-05-21 05:48:25 EDT
Please review
Comment 3 Markus Keller CLA 2007-05-23 06:27:45 EDT
Javadoc in IDefaultValueAdvisor: don't write "returns" after @return

ParameterObjectFactory:
- Format new code (spaces around binary expressions, ...)
- cuRewrite.getRoot().getPackage() is null in default package => NPE. Also check the rest of the method for problems with default package. I would just use ((IPackageFragment) cuRewrite.getCu().getParent()).getElementName()
- Why do you need localContext? Can't you just call super.findInContext(..) again?

Please add a test with a class in the default package.

Rest looks OK.
Comment 4 Karsten Becker CLA 2007-05-23 09:02:37 EDT
Created attachment 68327 [details]
Changes for Comment #3

The subclassing of the context was not necessary. It was a result from testing with a screwed up workspace and not reproducable.
Comment 5 Martin Aeschlimann CLA 2007-05-23 10:04:52 EDT
patch is good
Comment 6 Karsten Becker CLA 2007-05-23 10:19:53 EDT
Please review the latest attachment
Comment 7 Karsten Becker CLA 2007-05-23 14:04:42 EDT
Created attachment 68399 [details]
Updated Patch for default package tests

java.awt.Point was not in RT.jar of RefactoringTestSetup thus resulting in a wrong green.
Comment 8 Markus Keller CLA 2007-05-24 06:35:31 EDT
(In reply to comment #7)
> Created an attachment (id=68399) [details]
> Updated Patch for default package tests

In addition to test updates, this patch adds a change in ParameterInfo, which is good for now, but should be fixed later. Add this comment to ParameterInfo.setNewTypeBinding(..) when releasing:

//TODO: TypeContextChecker should not resolve such a parameter at all.
// That would also make IProblemVerifier obsolete.

Martin, do you approve the change in ParameterInfo?
Comment 9 Martin Aeschlimann CLA 2007-05-24 06:40:28 EDT
approved, please release.
Comment 10 Markus Keller CLA 2007-05-24 06:52:59 EDT
Released to HEAD.
Comment 11 Benno Baumgartner CLA 2007-05-29 06:37:12 EDT
verified in I20070527-0010