Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 247703

Summary: Removing internal superclass does not report error for removed methods
Product: [Eclipse Project] PDE Reporter: Darin Wright <darin.eclipse>
Component: API ToolsAssignee: Olivier Thomann <Olivier_Thomann>
Status: RESOLVED INVALID QA Contact:
Severity: normal    
Priority: P3    
Version: 3.5   
Target Milestone: 3.5 M3   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed fix + regression tests
none
Proposed fix + regression tests none

Description Darin Wright CLA 2008-09-17 14:33:27 EDT 
If I change the superclass of a class of an API class from a non-API (with a mehtod) class to Object, ot if I remove a public method on the non-API super class, no errors are reported on the public subclass.

Tests that should be fixed when this bug is addressed:

ClassCompatibilityInternalTests.testRemoveInternalMethod*()
ClassCompatibilityHierarchyTests.testRemoveIntSuperClassWithMethod*()
Comment 1 Darin Wright CLA 2008-09-17 14:49:15 EDT 
Clarification: Removing a method on an internal superclass is found as a problem in a full  build, but not incremental.
Comment 2 Darin Wright CLA 2008-09-17 15:28:49 EDT 
So the following tests need updating with this bug:

- ClassCompatibilityInternalTests.testRemoveInternalMethodI()
> This is a dup of bug 243809.

And 

ClassCompatibilityHierarchyTests.testRemoveIntSuperClassWithMethod*()
Comment 3 Olivier Thomann CLA 2008-09-17 15:47:31 EDT 
Created attachment 112814 [details]
Proposed fix + regression tests

To make sure I don't lose these changes here is a patch.
Removed/method errors should be converted to new flag that would then report a better error message.
Comment 4 Olivier Thomann CLA 2008-09-17 22:36:33 EDT 
The patch is not good enough. I need to check if the methods have been moved in the hierarchy before reporting a method removal.
Comment 5 Olivier Thomann CLA 2008-10-17 12:49:25 EDT 
Created attachment 115424 [details]
Proposed fix + regression tests

Darin, could you please run all the api plugin test suite?
Thanks.
Comment 6 Darin Wright CLA 2008-10-17 13:47:10 EDT 
Tests are all green.
Comment 7 Olivier Thomann CLA 2008-10-17 13:58:14 EDT 
Fixed and released in HEAD.
Darin, please verify.
Comment 8 Darin Wright CLA 2008-10-20 15:10:32 EDT 
I found that if an internal superclass is removed that defines a constructor that the subclass does not, it is reported as removed. However, we should ignore constructors.

Updated ClassFileComparator. Added regression test:

ClassCompatibilityHierarchyTests.testRemoveInternalSuperClassWithConstructorI/F()

Please verify, Olivier.
Comment 9 Olivier Thomann CLA 2008-10-21 16:04:28 EDT 
This will be reverted and I'll close it as INVALID.
See bug 251606 for details.
Comment 10 Olivier Thomann CLA 2008-10-21 16:04:43 EDT 
Closing as INVALID.