Bugzilla – Full Text Bug Listing
| Summary: | [1.7][code assist] CompletionContext.getExpectedTypesKeys() returns wrong type | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P1 | CC: | Olivier_Thomann, satyam.kandula | ||||||||
| Version: | 3.7 | ||||||||||
| Target Milestone: | 3.7.1 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
Created attachment 199691 [details]
proposed fix
This patch makes sure the expected type is correct for variable declarations or return statements. The LHS type parameters are used.
Note that diamond inference is not possible here because we find the completion node before even parsing the complete allocation expression and the arguments in it. So taking the type parameters from LHS or from expected return type is the best we can do.
Dani, let me know if this patch works for you.Thanks!
Did you comment checkCancel() on purpose ? (In reply to comment #2) > Did you comment checkCancel() on purpose ? That was for testing. I still have to add the tests, so I'll uncomment it out in the final patch (In reply to comment #3) > That was for testing. I still have to add the tests, so I'll uncomment it out > in the final patch ok. just checking :-) Created attachment 199819 [details]
proposed fix v2.0 + regression tests
Dani, would you expect a signature - [L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;] for List<String> a3= new ArrayList<| ,
or a signature - [[L, j, a, v, a, /, u, t, i, l, /, L, i, s, t, <, L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;, >, ;]] ?
This patch gives the latter expected type signature, while the above patch gives the former signature. Looking at the UI code, I realised this is the way UI expects proposals in this case.
(Patch contains some commented out code which i'll remove once Dani approves that this is the intended behaviour).
(In reply to comment #5) > Created attachment 199819 [details] [diff] > proposed fix v2.0 + regression tests > > Dani, would you expect a signature - [L, j, a, v, a, /, l, a, n, g, /, S, t, r, > i, n, g, ;] for List<String> a3= new ArrayList<| , > or a signature - [[L, j, a, v, a, /, u, t, i, l, /, L, i, s, t, <, L, j, a, v, > a, /, l, a, n, g, /, S, t, r, i, n, g, ;, >, ;]] ? The latter. The patch works for me. Sorry for the late response. (In reply to comment #6) > The latter. > > The patch works for me. Sorry for the late response. Yeah i guessed this would be the case looking at the UI code. However I had a doubt because the UI code currently does not handle the following: X<String, String> x = new X<String, | > This will be easier for the UI to handle if we give [L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;] as the signature. With the patch in comment 5 however,the UI will have to parse the signature [[L, j, a, v, a, /, u, t, i, l, /, L, i, s, t, <, L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;, L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g,; >, ;]] to identify the second type arg as String. Let me know what you think. (In reply to comment #7) > (In reply to comment #6) > > The latter. > > > > The patch works for me. Sorry for the late response. > > Yeah i guessed this would be the case looking at the UI code. However I had a > doubt because the UI code currently does not handle the following: > > X<String, String> x = new X<String, | > > > This will be easier for the UI to handle if we give [L, j, a, v, a, /, l, a, n, > g, /, S, t, r, i, n, g, ;] as the signature. With the patch in comment 5 > however,the UI will have to parse the signature [[L, j, a, v, a, /, u, t, i, l, > /, L, i, s, t, <, L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;, L, j, > a, v, a, /, l, a, n, g, /, S, t, r, i, n, g,; >, ;]] > > to identify the second type arg as String. > Let me know what you think. Yes, for that case but it would not easily allow to insert "String,String" in this case: X<String, String> x = new X<|> which is more common. NOTE: In order to test it you need the latest JDT UI code from HEAD. Created attachment 200436 [details]
released patch (fix v2.0 + more tests)
Added more tests and polished the fix.
Released in BETA_JAVA7 branch Verified for 3.8M1 using build I20110729-1200 |
BETA_JAVA7. When invoking content assist at | List<String> a3= new ArrayList| and I call CompletionContext.getExpectedTypesKeys() I get the correct result: [[L, j, a, v, a, /, u, t, i, l, /, L, i, s, t, <, L, j, a, v, a, /, l, a, n, g, /, S, t, r, i, n, g, ;, >, ;]] But when I do the same here: List<String> a3= new ArrayList<| I get: [[L, j, a, v, a, /, l, a, n, g, /, O, b, j, e, c, t, ;]] This is something we need to fix for 3.7.1 as otherwise people can't easily insert the inferred type after we inserted the diamond.