Bug 108115 - [refactoring] Extract Local Variable and Ex. Constant don't find matching invocations of generic methods
Summary: [refactoring] Extract Local Variable and Ex. Constant don't find matching inv...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.1.1   Edit
Assignee: Markus Keller CLA Friend
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-08-26 10:23 EDT by Markus Keller CLA Friend
Modified: 2005-09-02 08:54 EDT (History)
3 users (show)

See Also:

Attachments
Fix (895 bytes, patch)
2005-08-26 10:24 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Regression Tests (4.44 KB, patch)
2005-08-26 10:26 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Better fix (5.68 KB, patch)
2005-08-29 05:13 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Better Tests (5.68 KB, patch)
2005-08-29 05:15 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Better fix 2 (895 bytes, patch)
2005-08-29 05:18 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Better fix 3 (1.35 KB, patch)
2005-08-29 05:47 EDT, Markus Keller CLA Friend 		no flags Details | Diff
Show Obsolete (4) 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 Markus Keller CLA Friend 2005-08-26 10:23:48 EDT 
M20050824-1200

See JDT/Core bug 104293. For that specific instance of the problem, there's an
easy workaround in JdtASTMatcher that we should submit for 3.1.1.
Comment 1 Markus Keller CLA Friend 2005-08-26 10:24:56 EDT 
Created attachment 26527 [details]
Fix
Comment 2 Markus Keller CLA Friend 2005-08-26 10:26:05 EDT 
Created attachment 26528 [details]
Regression Tests
Comment 3 Markus Keller CLA Friend 2005-08-26 10:28:29 EDT 
Tobias & Dirk, please review for 3.1.1.
Comment 4 Tobias Widmer CLA Friend 2005-08-26 10:45:22 EDT 
Patch looks good, +1 for 3.1.1
Comment 5 Dirk Baeumer CLA Friend 2005-08-26 12:49:10 EDT 
Using Bindings.equals requires node.resolveBinding() not being null. This wasn't
the case before. Markus, can you please improve the patch.
Comment 6 Markus Keller CLA Friend 2005-08-29 05:13:55 EDT 
Created attachment 26579 [details]
Better fix

Duh, stupid me. Here's a better fix.
Comment 7 Markus Keller CLA Friend 2005-08-29 05:15:55 EDT 
Created attachment 26580 [details]
Better Tests

Tests, including test for comment 5.
Comment 8 Markus Keller CLA Friend 2005-08-29 05:18:22 EDT 
Created attachment 26581 [details]
Better fix 2

Selected wrong file before :-(
Comment 9 Markus Keller CLA Friend 2005-08-29 05:47:46 EDT 
Created attachment 26582 [details]
Better fix 3

3rd try to submit the better fix (it's monday morning;-)
Comment 10 Dirk Baeumer CLA Friend 2005-08-29 06:06:15 EDT 
Patch looks ok to me. 

For 3.2: doesn't it make sense to say that the two nodes are equal if we don't
have bindings for them. I would opt to return false in this case as well.
Comment 11 Markus Keller CLA Friend 2005-08-30 09:20:41 EDT 
Tobias, could you please review the latest fix again?

> For 3.2: doesn't it make sense to say that the two nodes are equal if we don't
> have bindings for them. I would opt to return false in this case as well.

Yes, I agree it would make sense to consider nodes without bindings as equal
(and return _true_ in that case). I'll file a new bug for that and fix it in
HEAD after having tested this fix for a while.
Comment 12 Tobias Widmer CLA Friend 2005-08-30 09:35:51 EDT 
Patch looks ok to me
Comment 13 Markus Keller CLA Friend 2005-08-30 10:02:31 EDT 
Released patches to HEAD and 3.1.1 branch.
The other bug is bug 108354.
Comment 14 Tobias Widmer CLA Friend 2005-09-02 08:54:42 EDT 
Verified using M20050831-1200 + ZRH plugin export