Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #183165 +++ Based on our previous discussions in bug 183165 and bug 180562, as well as the new API Tooling feature of @noextend on interfaces as per bug 230189, we need to tag all those interfaces as @noextend where we intend to add constants of static ields in the future.
See also this message by Darin Wright on cross-project-issues-dev: http://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg02862.html
I found the following interfaces which define constants: IClassifierConstants IFileConstants IRemoteConnectionHostConstants IRSECoreStatusCodes IRSEDOMConstants IRSEPreferenceNames IRSESystemTypeConstants IRSEUserIdConstants ISubSystemConfigurationCategories ISystemContextMenuConstants ISystemModelChangeEvents ISystemPreferenceChangeEvents ISystemRemoteChangeEvents ISystemResourceChangeEvents RemoteServerLauncherConstants SystemResourceConstants Plus the following ones, which are currently tagged @noimplement but where we do not supply any default implementation that clients could legally extend (such that no client could ever legally implement an extension of the interface anyways): IClassByteStreamHandler IDomainNotifier IProperty IPropertySet IPropertyType IRemoteClassInstance IRSECoreRegistry ISystemMessageObject ISystemProfile ISystemProfileManager ISystemRegistry ISystemRegistryUI ISystemRemotePath In the Eclipse 3.5 (Galileo) timeframe there is the unique chance of adding an @noextend tag to an existing @noimplement API legally without seeing a warning, because of the change of semantics introduced with bug 230189. So, we need to get this fixed now and it is better to restrict our API too much rather than too little, thus maintaining existing semantics. We'll be able to open-up interfaces in the future based on our current markup, but we'll not be able to restrict it in the future. That being said, the following ones need special consideration: They do define some constants, so we might want to extend them ourselves in the future. Thus, we need to add @noextend to keep clients from extending them (although that might eventually make sense): IRSESystemType ISystemResourceSet I also chose to mark the following interfaces from the Terminal as @noextend, in order to convey the semantics that has already been there in 3.0: ITerminalConnector ITerminalControl ITerminalTextData ITerminalTextDataReadOnly ITerminalTextDataSnapshot The changes above are committed with [261486][api][cleanup] Mark @noimplement interfaces as @noextend
I also marked following ones @noextend, because they are not really meant to be extendable API so we better play it safe and don't allow extension: IRSEPersistenceManager ISystemChangeFilterPaneEditPaneSupplier For the remaining occurrences, the following are quite clear that we do *not* want to mark @noextend: IService ISubSystem ISubSystemConfiguration since these are expected to be extended by clients to create special kinds of service or subsystem respectively. But how to treat their descendants, and related interfaces, specifically ISearchService, IFileService, IProcessService, ... IRemoteFileSubSystem, ... IShellServiceSubSystemConfiguration, ... IConnectorService IConnectorServiceManager IRemoteFile the arguments in favor of / or against adding "@noextend" are: (pro) We can add @noextend now to maintain current semantics. We can easily remove it later but we cannot add it later. (con) These interfaces are really meant to be decorated by clients. We do not want to add static constants to these interfaces in the future. Since they are inherently @noimplement, clients can not do much harm by extending them anyways, i.e. it won't hurt us too much. Finally, there is the entire "RSE Model" with interfaces like ISystemFilter* IRSEModelObject ISystemHostPool IRSE*Reference* which I don't know to what extent it is meant to be extendable. Reassigning to Dave D for consideration. Personally, I think I'd rather play it safe and add @noextend everywhere, since we can later remove it where not appropriate. Dave, please decide and resolve this by M6 latest (better by M5) since the API Tooling markup is part of our API.
Dave, FYI: All remaining instances of interfaces which are tagged @noimplement but are NOT tagged @noextend can be found with Search > File Search and the following regular expression: @noimplement.*\r?\n.*(?!noextend) where (?!noextend) is a negative lookahead on the line after @noextend :-)
I added @noextend to the following interfaces ISystemFilter.java ISystemFilterContainer.java ISystemFilterContainerReference.java ISystemFilterPool.java ISystemFilterPoolManager.java ISystemFilterPoolManagerProvider.java ISystemFilterPoolReference.java ISystemFilterPoolReferenceManager.java ISystemFilterPoolReferenceManagerProvider.java ISystemFilterPoolSelectionValidator.java ISystemFilterPoolWrapper.java ISystemFilterPoolWrapperInformation.java ISystemFilterReference.java ISystemFilterStartHere.java ISystemFilterString.java ISystemFilterStringReference.java IHost.java IPropertySetContainer.java IRSEModelObject.java IRSEPersistableContainer.java ISystemContainer.java ISystemContentsType.java ISystemHostPool.java ISystemModifiableContainer.java ISystemResourceSet.java IRSEBasePersistableReferencedObject.java IRSEBasePersistableReferenceManager.java IRSEBasePersistableReferencingObject.java IRSEBaseReferencedObject.java IRSEBaseReferencingObject.java IRSEPersistableReferencedObject.java IRSEPersistableReferencingObject.java IRSEReferencedObject.java IRSEReferencingObject.java IConnectorService.java IConnectorServiceManager.java IContextObject.java I removed @noimplement from the following after adding @noextend: ISystemFilterPoolSelectionValidator.java The following interfaces are still @noimplement and do not have @noextend IFileService IProcessService IRemoteFile ISearchService IShellService ISubSystem ISubSystemConfiguration ITerminalService ITerminalShell DaveM -- you should look at IContextObject and the ones above.
reassigning to DaveM for final work
(In reply to comment #5) Hm... by adding @noextend to IRSEModelObject, API Tooling now marks IUserActionContext and IUserActionModel as illegal extensions of the interface. Is this expected? What is our story for IRSEModelObject? On IUserActionContext, the Javadoc even says "Clients can implement this interface", which apparently was invalid before (since the parent class IRSEModelObject was marked @noimplement before). To me it looks like our APIs are inconsistent, what is our intention with the API? Should clients be allowed to extend or not? In general, allowing to extend / implement means that we cannot evolve the API in the future easily. Is there a need that the user action model actually lives in the IRSEModelObject hierarchy?
In the case of IRSEModelObject, I was trying to make sure that, for the time being, non-RSE component extensions were flagged in error. I like to accept the user actions extensions as legitimate, but there doesn't appear to be a way to do so without triggering the error. There doesn't appear to be a way to tell that a bundle is part of a component that is allowed to extend an interface or class. If this is a problem, I can take the @noextend off IRSEModelObject, but we do have other problems that fall into this kind of category - mostly with respect to @noimplement. Perhaps we should look at those as well?
(In reply to comment #8) > do so without triggering the error. There doesn't appear to be a way to tell > that a bundle is part of a component that is allowed to extend an interface or > class. You can filter-out every API Tooling error or warning, just do a quickfix on the warning and it will offer you to add an "API Problem Filter". For User actions, these would be stored under .settings/ in the user actions project. > If this is a problem, I can take the @noextend off IRSEModelObject, but we do > have other problems that fall into this kind of category What's important to me is that our API Tooling markup reflects what we think is right. In this case, it looks like we don't want anybody to extend the RSE model objects. User actions is an allowable exception since it is considered "internal" to RSE -- that's fine for me. The one thing that does worry me, is that it looks like if IRSEModelObject is @noextend @noimplement then any extender of it (i.e. IUserActionContext) should likely also be @noextend @noimplement or it looks like our design is flawed. We cannot say that nobody should extend our model in general, but then allow or even encourage extensions of the model in the user action case.
(In reply to comment #5) As per the discussion in the committer meeting yesterday, IRSEModelObject is in order since it's better to have the @noextend than not, and we can deal with errors in user actions later by either adding a filter, or doing some refactoring to extract a proper interface to extend. What's still open is the following task assigned to DaveM: > The following interfaces are still @noimplement and do not have @noextend > > IFileService > IProcessService > IRemoteFile > ISearchService > IShellService > ISubSystem > ISubSystemConfiguration > ITerminalService > ITerminalShell > > DaveM -- you should look at IContextObject and the ones above. where I personally believe that the I*Service and ISubSystem* are in order since they are designed to be extended, I'm not sure about the following: IRemoteFile ITerminalShell Dave can you please mark this fixed when appropriate.
I actually think we should mark this fixed and live with the fact that IRemoteFile ITerminalShell are probably not optimal. Adding @noextend in the 3.2 time frame would be a breaking API change which we don't want. It's better to live with what we have. I'm tagging M6 since the fix was back in march.
