Community
Participate
Working Groups
I would like to update IAdaptable to return the actual type of the requested object. Suggest change public <T> T getAdapter(Class<T> adapter);
The above change would require to use Java 1.5 or Java 1.6. https://git.eclipse.org/r/31870 This will create Type Safety and Raw Type warnings, I'm happy to help with solving them, if the change gets accepted.
(In reply to Lars Vogel from comment #1) > The above change would require to use Java 1.5 or Java 1.6. > > https://git.eclipse.org/r/31870 > > This will create Type Safety and Raw Type warnings, I'm happy to help with > solving them, if the change gets accepted. I don't mind moving up. The framework is already at Java 6. But I think we should clean up the warnings as well.
Created attachment 250005 [details] Fix for raw types and unchecked type conversion warnings The patch is a complement for Lars' https://git.eclipse.org/r/#/c/31870/. It generifies rt.equinox.bundles and fixes all warnings related to use of raw types and unchecked type conversion. Unfortunately, I was not able to push the patch to Gerrit since I'm getting "ssh://sprigogin@git.eclipse.org/gitroot/equinox/rt.equinox.bundles.git: error occurred during unpacking on the remote end: unpack-objects abnormal exit"
(In reply to Sergey Prigogin from comment #3) The fix for use of raw types and unchecked type conversion is in Gerrit at https://git.eclipse.org/r/#/c/39800/ I kept it separate from https://git.eclipse.org/r/31870 but I can merge the two patches if it is preferable.
(In reply to Thomas Watson from comment #2) > I don't mind moving up. The framework is already at Java 6. But I think we > should clean up the warnings as well. Tom can you review the cleanup from Sergey? https://git.eclipse.org/r/#/c/31870/ - Update e.common to Java 1.6 https://git.eclipse.org/r/#/c/39800/ - Clean up of the warnings I don't think Sergey or I can merge both requests, you must be committer to do this, but if you (Tom) prefer I don't mind if https://git.eclipse.org/r/#/c/31870/ gets abondanded and included in Sergeys change.
A combined fix is now in https://git.eclipse.org/r/#/c/31870/ Please review.
Tom, I reset the target from M5 to M6, please adjust again if that is unrealistic.
The changes look good to me, but I would like signoff from Dani and Markus since they will understand the impact to the eclipse platform and other consumers more. I suspect the changes in the org.eclipse.core.runtime package will cause many new generic warnings for bundles in the Eclipse project and other projects. Dani, are you OK with this change?
(In reply to Thomas Watson from comment #8) > I suspect the changes in the org.eclipse.core.runtime > package will cause many new generic warnings for bundles in the Eclipse > project and other projects. The sooner this change is submitted, the sooner maintainers of the dependent code can start making their code type safe and fix the related warnings while doing that.
(In reply to Thomas Watson from comment #8) > The changes look good to me, but I would like signoff from Dani and Markus > since they will understand the impact to the eclipse platform and other > consumers more. I suspect the changes in the org.eclipse.core.runtime > package will cause many new generic warnings for bundles in the Eclipse > project and other projects. > > Dani, are you OK with this change? Yes, see bug 442022 comment 7.
Yes, we should do this right ASAP. There will compile errors or warnings in other projects, but the unnecessary casts can easily be removed (run the "Source > Clean Up...", or apply Quick Fix in Problems view and then select all similar problems). Is there a reason for not going straight to 1.7? Remaining problems I saw in https://git.eclipse.org/r/#/c/31870/4/ : - update Javadocs of IAdaptable, IAdapterManager, PlatformObject: - remove casts, use new signatures [This is just a matter of changing the preferences and the running the "Missing Code" Clean Ups to add the annotations. Probably easiest to be done by Tom in a quick follow-up commit.] - update Project Properties > Java Compiler > Errors/Warnings: - enable warnings for missing @Override/@Deprecated annotations - disable "Ignore unavoidable generic type problems due to raw APIs" - disable "Treat above errors like fatal compile errors (make compiled code not executable)" -- nothing new, but that was just a bad idea
(In reply to Markus Keller from comment #11) > Yes, we should do this right ASAP. There will compile errors or warnings in > other projects, but the unnecessary casts can easily be removed (run the > "Source > Clean Up...", or apply Quick Fix in Problems view and then select > all similar problems). > > Is there a reason for not going straight to 1.7? No reason except fear that moving to 1.7 would be met with even more resistance than moving to 1.6. Do you prefer that we move directly to 1.7? > Remaining problems I saw in https://git.eclipse.org/r/#/c/31870/4/ : > > - update Javadocs of IAdaptable, IAdapterManager, PlatformObject: > - remove casts, use new signatures > > [This is just a matter of changing the preferences and the running the > "Missing Code" Clean Ups to add the annotations. Probably easiest to be done > by Tom in a quick follow-up commit.] > - update Project Properties > Java Compiler > Errors/Warnings: > - enable warnings for missing @Override/@Deprecated annotations > - disable "Ignore unavoidable generic type problems due to raw APIs" > - disable "Treat above errors like fatal compile errors (make compiled > code not executable)" -- nothing new, but that was just a bad idea
(In reply to Sergey Prigogin from comment #12) > (In reply to Markus Keller from comment #11) > > Yes, we should do this right ASAP. There will compile errors or warnings in > > other projects, but the unnecessary casts can easily be removed (run the > > "Source > Clean Up...", or apply Quick Fix in Problems view and then select > > all similar problems). > > > > Is there a reason for not going straight to 1.7? > > No reason except fear that moving to 1.7 would be met with even more > resistance than moving to 1.6. Do you prefer that we move directly to 1.7? > Is there a feature in Java 7 that you will be using? I don't want to hold anyone back, but I have not heard of a feature in Java 7 that is being used in this bundle.
(In reply to Thomas Watson from comment #13) > Is there a feature in Java 7 that you will be using? I don't want to hold > anyone back, but I have not heard of a feature in Java 7 that is being used > in this bundle. The diamond operator <> is a nice thing to use.
(In reply to Sergey Prigogin from comment #14) > (In reply to Thomas Watson from comment #13) > > Is there a feature in Java 7 that you will be using? I don't want to hold > > anyone back, but I have not heard of a feature in Java 7 that is being used > > in this bundle. > > The diamond operator <> is a nice thing to use. I have not kept enough track of the RCP bundles in the Eclipse Platform. At one point we still wanted RCP to be able to run on Java 6. I don't really want to make equinox.common be the first to cause RCP not to run on Java 6. But if there are already cases of other Eclipse core RCP bundles that have moved up then go for it.
(In reply to Thomas Watson from comment #15) > I have not kept enough track of the RCP bundles in the Eclipse Platform. At > one point we still wanted RCP to be able to run on Java 6. I don't really > want to make equinox.common be the first to cause RCP not to run on Java 6. > But if there are already cases of other Eclipse core RCP bundles that have > moved up then go for it. We have a few planned to be updated to Java 1.7 in platform.ui in the next future.
(In reply to Lars Vogel from comment #16) > We have a few planned to be updated to Java 1.7 in platform.ui in the next > future. Lars, does this mean post Mars, or the next milestone or ... ;-)
(In reply to Thomas Watson from comment #17) > (In reply to Lars Vogel from comment #16) > > We have a few planned to be updated to Java 1.7 in platform.ui in the next > > future. > > Lars, does this mean post Mars, or the next milestone or ... ;-) M6, if we find the time to do it. I ask Simon to hurry...
(In reply to Lars Vogel from comment #18) > (In reply to Thomas Watson from comment #17) > > Lars, does this mean post Mars, or the next milestone or ... ;-) Add two bugs as references, I know we have more (IIRC Sergey was planning to update some plug-ins) but I could not find the reference fast enough.
(In reply to Lars Vogel from comment #19) > (In reply to Lars Vogel from comment #18) > > (In reply to Thomas Watson from comment #17) > > > Lars, does this mean post Mars, or the next milestone or ... ;-) > > Add two bugs as references, I know we have more (IIRC Sergey was planning to > update some plug-ins) but I could not find the reference fast enough. Bug 459373, as it stands today, is about moving Platform Resources to 1.6, not 1.7. I'm getting push back even on this modest change.
(In reply to Sergey Prigogin from comment #12) > > - update Javadocs of IAdaptable, IAdapterManager, PlatformObject: > > - remove casts, use new signatures JavaDoc comments have been updated in the latest Gerrit patch.
Gerrit change https://git.eclipse.org/r/31870 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=6e1d47679e92123b15795a2245ade8df57cace4e
(In reply to Markus Keller from comment #11) > [This is just a matter of changing the preferences and the running the > "Missing Code" Clean Ups to add the annotations. Probably easiest to be done > by Tom in a quick follow-up commit.] > - update Project Properties > Java Compiler > Errors/Warnings: > - enable warnings for missing @Override/@Deprecated annotations > - disable "Ignore unavoidable generic type problems due to raw APIs" > - disable "Treat above errors like fatal compile errors (make compiled > code not executable)" -- nothing new, but that was just a bad idea I did this as a following commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=f5a8b4ceb8cac9d730598aac4c5f382aed652222 This also included the following: - Increased the package version of org.eclipse.core.runtime - Increased the bundle version - updated copyright dates if needed. - Updated version of equinox.registry and made it depend on the new version of equinox.common I also had to commit the following: http://git.eclipse.org/c/platform/eclipse.platform.runtime.git/commit/?id=86ed554de2a952c60fcf53f6a635f25056142af0 Fallout from changes to the org.eclipse.core.runtime package which is split between equinox.common equinox.registry and core.runtime bundles. Need to increase the versions of all theses bundles and fix the compatibility fragment Now we will see what the fallout is in the nightly build.
Thanks Tom and Sergey.
Things to do for consumers: 1. In MANIFEST.MF, update your Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.11.0,4.0.0)", or org.eclipse.equinox.common;bundle-version="[3.7.0,4.0.0)", 2. If your bundle re-exports one of these bundles, then you also have to make sure the minor version is incremented. 3. Remove unnecessary casts (Clean Up, or Problems view > Quick Fix > Select All) 4. Update implementations of IAdaptable#getAdapter(Class<T>), unless you override another implementation of that method that still uses the old signature. Typical change: Old: public Object getAdapter(Class adapter) { if (ICompilationUnit.class.equals(adapter)) return getCompilationUnit(); return null; } New: @SuppressWarnings("unchecked") public <T> T getAdapter(Class<T> adapter) { if (ICompilationUnit.class.equals(adapter)) return (T) getCompilationUnit(); return null; } 5. Update implementations of IAdapterFactory Hint for 4. & 5.: - Open Type Hierarchy on IAdaptable, etc. - In the view menu, select a working set that contains your projects - In the methods list of the Type Hierarchy view, select the methods, and then click the first toolbar button (Lock View and Show Members in Hierarchy)
Should we send this change out to cross so that other projects can plan for the change?
(In reply to Lars Vogel from comment #26) > Should we send this change out to cross so that other projects can plan for > the change? Yes, I think Tom has this on his list.
I added a short version of this change to our M6 N&N. Feel free to extend it.
(In reply to Dani Megert from comment #27) > (In reply to Lars Vogel from comment #26) > > Should we send this change out to cross so that other projects can plan for > > the change? > > Yes, I think Tom has this on his list. Since I missed it, and Tom was kind enough to point me to it, thought I'd post the link here, to the cross-project list message: https://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg11589.html Thanks Tom (and, all)
(In reply to Markus Keller from comment #25) > New: > @SuppressWarnings("unchecked") > public <T> T getAdapter(Class<T> adapter) { > if (ICompilationUnit.class.equals(adapter)) > return (T) getCompilationUnit(); > return null; > } > For what it's worth, you can avoid the need for a @SuppressWarnings here using Class.cast: if (ICompilationUnit.class.equals(adapter)) return adapter.cast(getCompilationUnit()); // or return ICompilationUnit.class.cast(getCompilationUnit());
