Bugzilla – Full Text Bug Listing
| Summary: | ClassCastException while copying a .class file to wrong source package | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Rozhkov Igor <irozhkov2007> | ||||||
| Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, minhyuk, satyam.kandula | ||||||
| Version: | 3.7 | Flags: | amj87.iitr:
review+
|
||||||
| Target Milestone: | 3.8 M7 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Rozhkov Igor
Igor, is it possible to attach a code snippet that causes this to happen? Can you say anything about how to reproduce this problem? Jay, can you please investigate? Thanks! I watched it when editing the different parts of the code, and when you press Ctrl-1. Now this problem has disappeared. Since we don't have reproducible steps and now the problem no longer occurs, closing the bug. (In reply to comment #4) > Since we don't have reproducible steps and now the problem no longer occurs, > closing the bug. Its not too hard to construct the steps: 1. Launch a runtime workbench in debug mode 2. Create a source folder, for which the output folder is the same as the source folder itself. Create two classes A and B. In A, have this void foo() { B b = new B(); } 3. Now open the directory in the file system corresponding to the src folder. You will see A.java, A.class, B.java and B.class. 4. Delete ONLY B.java 5. Go back to Eclipse runtime workbench and try hovering over B in the B b = new B(). Try other things like selection or quick assist. 6. Look at the error log in the base Eclipse instance. You will see the exception from comment 0. Obviously, the assumption in NameLookup.seekTypesInSourcePackage line 1090 is flawed. The 'cu' array is obtained by getting all children of the source package, however those children can also be class files. An instanceof check, IMO, should suffice. (In reply to comment #5) > Its not too hard to construct the steps: Thanks for the steps, Ayush. But is something amiss? Because I can't reproduce it even with the steps. I tried with auto-build off, with 'B' being closed, problems view being closed etc. In debug mode, at some point, I only see only one CU in the array and I also see an error in the editor about the unresolved type 'B'. (In reply to comment #6) > (In reply to comment #5) > > Its not too hard to construct the steps: > > Thanks for the steps, Ayush. But is something amiss? Because I can't reproduce > it even with the steps. I tried with auto-build off, with 'B' being closed, > problems view being closed etc. In debug mode, at some point, I only see only > one CU in the array and I also see an error in the editor about the unresolved > type 'B'. Oops, those were the wrong steps: I didn't notice that some other experiment triggered the CCE, and not this one. Steps: 1) In any package 'p' in a project, add some sample A.class file in the 'p' folder in the filesystem or in the package explorer, and refresh the package. 2) In any .java file in the package add a line A a = new A(); like this class Sample { A a = new A(); } Now the CCE should occur in whatever Eclipse instance you're running. I tried this a couple of times and could consistently reproduce. Here are the correct steps:
1. Create project 'P', with one source folder 'src' with default output folder and two packages namely 'p' and 'q'.
2. Create two classes A and B in packages p and q respectively with following content:
package p;
public class A {
B b = new B();
}
package q;
public class B {
}
3. Create a temporary folder, say 'temp' within the same project.
4. Open the Navigator view and copy the file /P/bin/q/B.class to /P/temp
5. Delete /P/src/q/B.java from the Navigator view
6. Copy the B.class from the temporary location to /P/src/p. Note that the B actually belonged to package 'q' originally, but we are copying it to 'p'.
The errors appears in the error log view in a few seconds. If they don't, try reopening A.java in the Java editor and try selecting 'B'.
Created attachment 213754 [details]
Proposed fix
The fix is to ignore binary files when we are looking for source elements. A new test has been added. I will update after running all existing tests.
Minor point: there's a commented out import statement in NameLookup. While the fix looks good, I'm wondering if the getType() for ITypeRoot is still required, since currently getType exists both in ICompilationUnit and IClassFile and does the same thing - return the top level type? Maybe we should open an api request for 3.9. (In reply to comment #10) > I'm wondering if the getType() for ITypeRoot is still required, > since currently getType exists both in ICompilationUnit and IClassFile and does > the same thing - return the top level type? Maybe we should open an api request > for 3.9. Actually from an IClassFile.getType() there's no guarantee of obtaining a top-level type since inner classes have their own class files, while ICU.getType() will always return top level type. So clients will anyway have to handle the result differently. Hence, doesn't make a strong case for ITypeRoot.getType(). I also checked for all other usages of getType(..) and they are protected by proper checks. (In reply to comment #11) > Actually from an IClassFile.getType() there's no guarantee of obtaining a > top-level type since inner classes have their own class files, while > ICU.getType() will always return top level type. So clients will anyway have to > handle the result differently. Hence, doesn't make a strong case for > ITypeRoot.getType(). > > I also checked for all other usages of getType(..) and they are protected by > proper checks. That's right. Ideally we don't want to add the ClassFile to the package's children list at all since it doesn't belong to that package. But such a fix would be more complicated and might even have side effects. Hence, I chose the simple fix to just ignore it. Created attachment 213785 [details]
Updated patch
Removed the commented out import statement.
Ayush, please review this patch.
Looks good. Released the fix here: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=c71a2dd3276b1c054fbb64a586af2db8d142bd3f Verified for 3.8M7 using build I20120429-1800 *** Bug 393745 has been marked as a duplicate of this bug. *** |