Comment 1 Markus Keller 2006-08-23 12:40:11 EDT

Removing "throws JavaModelException" would not break binary compatibility, according to JLS3, 13.4.21.

However, since it breaks source compatibility, the change should be announced on the jdt-core list and added to the migration guide.

Comment 2 Dani Megert 2006-09-06 04:47:59 EDT

+1

Comment 3 Dani Megert 2007-02-07 09:52:45 EST

And the Javadoc should mention that this is a handle-only method.

Comment 4 Mike Wilson 2007-02-23 09:39:25 EST

To be clear, are you saying that the method *can't* throw the exception, so it should be removed?

Comment 5 Dani Megert 2007-02-23 09:42:47 EST

Yes, it can't. And clients aren't allowed to implement IClassFile on their own.

Comment 6 Mike Wilson 2007-02-23 09:47:41 EST

+1

Comment 7 Frederic Fusier 2007-02-23 10:49:39 EST

Created attachment 59666 [details]
Proposed patch

Comment 8 Frederic Fusier 2007-02-23 11:47:28 EST

Created attachment 59676 [details]
New proposed patch

Sorry, I posted the previous patch too quickly...

Comment 9 Philipe Mulet 2007-02-23 12:50:26 EST

The source compatibility issue is a bit annoying... is it worth the pain ? or simply nice to have (I suspect we also have some predicates affected with the same issue e.g. IJavaElement#isStructureKnown()).

Comment 10 Markus Keller 2007-02-23 13:16:46 EST

IJavaElement#isStructureKnown() is different, since it is not a handle-only method (that the return type is boolean does not matter, otherwise you could also justify an exists() guard for all other non-handle methods).

It's up to the JDT/Core team to decide whether there are other hidden handle-only methods -- that's just the one I stumbled upon. It's not essential (the workaround is just an empty try-catch handler), but it would streamline some call sites.

Comment 11 Philipe Mulet 2007-02-28 06:14:42 EST

I was looking for more instances of slight inconsistencies where predicate where throwing exception, but I think we cleaned most of them along the way, and simply missed this one.

Therefore, +1.

Comment 12 Markus Keller 2007-02-28 07:07:38 EST

Affected clients in the SDK outside of JDT/Core and JDT/UI are:

org.eclipse.ant.internal.ui.datatransfer.ExportUtil.computeScope(Object)
org.eclipse.jdt.apt.core.internal.env.BaseProcessorEnv.getPackage(String)
org.eclipse.jdt.internal.debug.ui.TypeNameResolver.getType(IJavaElement)

/org.eclipse.jdt.doc.isv/porting/3.3/incompatibilities.html (does not yet exist)

Comment 13 Frederic Fusier 2007-02-28 12:03:25 EST

Created attachment 59998 [details]
Proposed 3.3 porting addition

Not sure whether the 3.2 porting guide should still be in 3.3 or not?

Comment 14 Markus Keller 2007-02-28 13:48:52 EST

> Not sure whether the 3.2 porting guide should still be in 3.3 or not?

The 3.2 porting guide is still relevant, so it should stay (cf. org.eclipse.platform.doc.isv/porting)

Typo: "In Eclipse 3.2, the method IClassFile.getType() thrown a ..."
=> should be "threw a"

I would also add that this is only a source incompatibility. Compiled class files still work fine.

Comment 15 Frederic Fusier 2007-03-02 07:04:03 EST

Created attachment 60151 [details]
New proposed 3.3 porting addition

Comment 16 Frederic Fusier 2007-03-02 07:09:36 EST

Released for 3.3 M6 in HEAD stream.

Walter,
I've also released the necessary APT change in org.eclipse.jdt.apt project.

Darin, Markus,
Do not forget to release your corresponding changes today, thanks

Comment 17 Markus Keller 2007-03-02 09:34:02 EST

Fixed JDT/UI references in HEAD.
In some cases, I even simplified the code to ITypeRoot.findPrimaryType() (no branching for compilation units and class files any more).

Comment 18 Darin Wright 2007-03-02 22:42:16 EST

Fixed debug reference

Comment 19 Olivier Thomann 2007-03-20 09:13:16 EDT

Verified for 3.3 M6 using build I20070320-0010