Bug 247703 - Removing internal superclass does not report error for removed methods
Summary: Removing internal superclass does not report error for removed methods
Status: RESOLVED INVALID
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M3   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-17 14:33 EDT by Darin Wright CLA
Modified: 2008-10-21 16:04 EDT (History)
0 users

See Also:

Attachments
Proposed fix + regression tests (12.63 KB, patch)
2008-09-17 15:47 EDT, Olivier Thomann CLA 		no flags Details | Diff
Proposed fix + regression tests (19.70 KB, patch)
2008-10-17 12:49 EDT, Olivier Thomann CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.