Community
Participate
Working Groups
Build Identifier: 20100218-1602 I've noticed my impl of this interface receives typeHierarchyChanged callbacks even though the hierarchy has not changed. TypeHierarchy.elementChanged receives an event whose toString() is: org.eclipse.jdt.core.ElementChangedEvent[source=Java Model[*]: {CHILDREN} myProject[*]: {CHILDREN | CONTENT} src_0/java[*]: {CHILDREN | CONTENT} com.myproject.some.package[*]: {CONTENT} ResourceDelta(/myProject/src_0/java/com/myproject/some/package/simplefile.txt)[*] ResourceDelta(/myProject/src_0/java/com)[*] ResourceDelta(/myProject/src_0)[*]] where: - myProject is my project name - src_0/java is a source folder - /myProject/src_0/java/com/myproject/some/package/simplefile.txt is a text file (non-Java resource in a package fragment) It seems like TypeHierarchy.isAffectedByPackageFragmentRoot() is returning true incorrectly. In the comments, it mentions for JAR content changes, it will return true if one of the packages match. However, the corresponding logic never checks for a JAR package fragment root, and just returns true if the package matches one from the packageRegion. Seems like a fix is to ensure the package fragment root is a JAR package fragment root. Reproducible: Always Steps to Reproduce: 1. Create a simple plugin that listens for changes on a TypeHierarchy for some type. 2. Debug Eclipse w/ the plugin 3. Create a new java project 4. In the java project, create a subclass of the type whose TypeHierarchy is being listened to 5. In the source folder directory, create a dummy text file, save it 6. You should have seen a callback to your TypeHierarchy listener
I am seeing the problem. I will investigate.
Created attachment 171912 [details] Proposed patch Proposed patch + tests. More tests might be added.
Created attachment 172902 [details] Updated patch Added a new tests in TypeHierarchyNotificationTests.testAddNonJavaToPackageFragmentRoot_b(). However this tests is failing because the code that processes the fine grained changes seems to be broken. Will investigate further and raise a new bug if required.
Created attachment 173286 [details] Patch with updated tests The failing test has been fixed. All tests pass.
Srikanth, would you have some time to review this patch? Thanks!
(In reply to comment #5) > Srikanth, would you have some time to review this patch? Thanks! I think the patch is on the right track, however (1) I don't think the tests are testing the right thing: You can back out the source change and run the new tests and 2/3 pass. This indicates that the tests are not precise enough. (2) Should org.eclipse.jdt.core.tests.model.TypeHierarchyNotificationTests.testBug316654() cleanup the txt file that is being created ? (3) For org.eclipse.jdt.core.tests.model.TypeHierarchyNotificationTests.testBug316654_b() the javadoc says it can throw CoreException, but the method throw spec has Exception in it -- Is this intentional ?
Created attachment 173395 [details] Updated pat Patch incorporates comments (2) and (3). About (1) - Those are regression tests that should pass with and without the fix. Since I didn't find any existing tests for those scenarios, I added them anyway. Test TypeHierarchyNotification#testBug316654_b() is particularly important because it is directly related to the code that is being changed in this patch.
Added additional test in TypeHierarchyNotificationTests.testBug316654_c(), which is similar to testBug316654_b() except that it tests a change to an internal JAR. Released in HEAD for 3.7M1.
Verified for 3.7M1 using build I20100802-1800
