Bug 270437 - [assist] Completion proposal leads to cycle detected error
Summary: [assist] Completion proposal leads to cycle detected error
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M1   Edit
Assignee: Pradeep Balachandran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-30 07:57 EDT by Srikanth Sankaran CLA
Modified: 2009-08-04 06:45 EDT (History)
2 users (show)

See Also:


Attachments
Code plugin patch - v-0.5 (1.74 KB, patch)
2009-05-15 11:48 EDT, Pradeep Balachandran CLA
no flags Details | Diff
Tests plugin patch - v-0.5 (2.64 KB, patch)
2009-05-15 11:49 EDT, Pradeep Balachandran CLA
no flags Details | Diff
Code plugin patch - v-0.6 (1.76 KB, patch)
2009-07-20 12:25 EDT, Pradeep Balachandran CLA
no flags Details | Diff
Tests plugin patch - v-0.6 (3.64 KB, patch)
2009-07-20 12:27 EDT, Pradeep Balachandran CLA
no flags Details | Diff
Code plugin patch - v-0.7 (1.76 KB, patch)
2009-07-21 06:56 EDT, Pradeep Balachandran CLA
srikanth_sankaran: iplog+
srikanth_sankaran: review+
Details | Diff
Tests plugin patch - v-0.7 (3.62 KB, patch)
2009-07-21 06:57 EDT, Pradeep Balachandran CLA
srikanth_sankaran: iplog+
srikanth_sankaran: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Srikanth Sankaran CLA 2009-03-30 07:57:54 EDT
Version: 3.5.0
Build id: I20090313-0100

1. Test case: 
public class TestClass extends TestClass.Inn|{
  public class Inner {}
}

2. At the '|' symbol do CTRL+Space

3. 'TestClass.Inn' gets completed to TestClass.Inner - 
which is wrong since it leads to an error:

"Cycle detected: the type TestClass cannot extend/implement
itself or one of its own member types"
Comment 1 Pradeep Balachandran CLA 2009-05-15 11:48:58 EDT
Created attachment 136016 [details]
Code plugin patch - v-0.5

Patch for org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
Comment 2 Pradeep Balachandran CLA 2009-05-15 11:49:58 EDT
Created attachment 136017 [details]
Tests plugin patch - v-0.5

Patch for org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests.java
Comment 3 Pradeep Balachandran CLA 2009-05-15 11:58:19 EDT
The same issue is applicable to interfaces as well. That is, inner/member interfaces are offered as completion choices for implements too.

The attached code patch fixes both the issues - the originally reported extends issue and the one I noticed about implements as well.

The attached test patch has two tests to verify the two problems addressed by this code patch.
Comment 4 David Audel CLA 2009-06-09 05:22:30 EDT
The patch filter member types of the completed type but not members of members

eg.

class X extends X.MyClass1.MyClass| { // do ctrl+space at |
	public class MyClass1 {
		public class MyClass2 {
		}
	}
}


MyClass2 should not be proposed
Comment 5 Pradeep Balachandran CLA 2009-07-20 12:25:33 EDT
Created attachment 142042 [details]
Code plugin patch - v-0.6

Updated the code patch to recursively add member types as forbidden.
Comment 6 Pradeep Balachandran CLA 2009-07-20 12:27:55 EDT
Created attachment 142043 [details]
Tests plugin patch - v-0.6

Add a test to catch the issue pointed out by David where nested member types were identified for completion. This patch contains three tests in all including the two tests added previously.
Comment 7 Pradeep Balachandran CLA 2009-07-20 12:31:21 EDT
(In reply to comment #4)
> The patch filter member types of the completed type but not members of members

Yes, Srikanth too pointed out during his review. The newer patch takes care of this. It was just a matter of making the process recursive. Added an additional JUnit test case as well.
Comment 8 Srikanth Sankaran CLA 2009-07-21 04:54:03 EDT
(In reply to comment #7)
> (In reply to comment #4)
> > The patch filter member types of the completed type but not members of members
> Yes, Srikanth too pointed out during his review. The newer patch takes care of
> this. It was just a matter of making the process recursive. Added an additional
> JUnit test case as well.

The fix and tests look reasonable. Could you please regenerate
the patch after syncronizing with HEAD and repost ? Thanks.

Comment 9 Pradeep Balachandran CLA 2009-07-21 06:56:23 EDT
Created attachment 142113 [details]
Code plugin patch - v-0.7

Patch to CompletionEngine.java in org.eclipse.jdt.core plugin after synchronizing with HEAD.
Comment 10 Pradeep Balachandran CLA 2009-07-21 06:57:37 EDT
Created attachment 142114 [details]
Tests plugin patch - v-0.7

Patch to CompletionTests.java in org.eclipse.jdt.core.tests.model plugin after synchronizing with HEAD.
Comment 11 Srikanth Sankaran CLA 2009-07-22 01:40:17 EDT
Released in HEAD for 3.6M1
Comment 12 Frederic Fusier CLA 2009-08-04 06:45:36 EDT
Verified for 3.6M1 using build 20090802-2000.