Bug 99399 - [1.5][assist] Code assist propose final classes in methods type parameter extends clause
Summary: [1.5][assist] Code assist propose final classes in methods type parameter ext...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P4 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-10 12:18 EDT by Frederic Fusier CLA
Modified: 2009-03-10 07:34 EDT (History)
1 user (show)

See Also:


Attachments
Proposed fix + tests (29.00 KB, patch)
2009-02-26 07:48 EST, Srikanth Sankaran CLA
no flags Details | Diff
Revised patch (28.90 KB, patch)
2009-03-02 07:25 EST, Srikanth Sankaran CLA
no flags Details | Diff
Revised patch (30.03 KB, patch)
2009-03-03 06:16 EST, Srikanth Sankaran CLA
david_audel: iplog+
david_audel: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2005-06-10 12:18:24 EDT
Using RC2 candidate I20050610-0010, while verifying bug 85384.

'|' mark position of code completion in following test cases.

1) public class X<T extends Str|
   Code assist does not propose String as it is a final class => correct

2) public class X<T extends Object> {
	void <U extends Str|
   }
   In this case, code assist propose String although it should not
Comment 1 David Audel CLA 2005-06-10 13:09:55 EDT
i cannot reproduce 1) and currently there is no special support for final
classes in codeassist.

But that's real bug.
final classes should not be proposed.

Similar test case found by Frederic:
class X extends Str|
Comment 2 Frederic Fusier CLA 2005-06-10 13:14:35 EDT
comment 0 test case 1) was definitely wrong...

String is proposed but not on top of the list

In fact, I think I typed:
public class X<T extends St<|>
and didn't see String as the list as a little bit long...
Comment 3 Srikanth Sankaran CLA 2009-02-25 04:59:35 EST
(In reply to comment #1)
> i cannot reproduce 1) and currently there is no special support for final
> classes in codeassist.
> But that's real bug.
> final classes should not be proposed.

Since <T extends SomeFinalClass> is not a null set, but the singleton set SomeFinalClass, it may not be totally wrong to suggest final
classes in these cases ? 

Personally, I think we should NOT propose final classes, but just checking.
 
> Similar test case found by Frederic:
> class X extends Str|

There is no uncertainty here: we should not propose final classes here.

I have a patch for both, that is under test.
Comment 4 David Audel CLA 2009-02-25 05:46:30 EST
(In reply to comment #3)
> Since <T extends SomeFinalClass> is not a null set, but the singleton set
> SomeFinalClass, it may not be totally wrong to suggest final
> classes in these cases ? 
> 
> Personally, I think we should NOT propose final classes, but just checking.

I agree. Even if it is not totally wrong, propose SomeFinalClass is useless.
If a user want to use SomeFinalClass he should write
class X {
  void foo(SomeFinalClass p) {}
}
and not 
class X<T extends SomeFinalClass> {
  void foo(T p) {}
}
Comment 5 Srikanth Sankaran CLA 2009-02-26 07:48:38 EST
Created attachment 126839 [details]
Proposed fix + tests


    We don't propose final classes anymore in ''extends'' contexts
(type declarations, typeparameters, wildcards etc).
Comment 6 Srikanth Sankaran CLA 2009-03-02 07:25:34 EST
Created attachment 127156 [details]
Revised patch


   Earlier patch had a couple of problems:

   - There was a serious bug due to misunderstanding of "expected types".
   - The rejection of final classes in extends contexts is handled differently
now - along the lines of rejecting interfaces where classes are expected.
Comment 7 Srikanth Sankaran CLA 2009-03-03 06:16:37 EST
Created attachment 127304 [details]
Revised patch


   Improved patch : Moved some computations out of the critical path.
Comment 8 David Audel CLA 2009-03-04 06:52:32 EST
Comment on attachment 127304 [details]
Revised patch

This patch is OK for me
Comment 9 David Audel CLA 2009-03-04 06:58:18 EST
Released for 3.5M6.
Comment 10 Frederic Fusier CLA 2009-03-10 07:34:43 EDT
Verified for 3.5M6 using I20090310-0100.