Bug 99399

Summary: [1.5][assist] Code assist propose final classes in methods type parameter extends clause
Product: [Eclipse Project] JDT Reporter: Frederic Fusier <frederic_fusier>
Component: CoreAssignee: Srikanth Sankaran <srikanth_sankaran>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P4 CC: david_audel
Version: 3.1   
Target Milestone: 3.5 M6   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed fix + tests
none
Revised patch
none
Revised patch david_audel: iplog+, david_audel: review+

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.