Community
Participate
Working Groups
From bug 520651 comment 4 Previously, each IClassFile had an IType as its child, which, however, for the case of module-info.class is wrong. First location found: org.eclipse.jdt.internal.ui.javaeditor.JavaEditorBreadcrumb.getInput(IJavaElement) where it says: element= ((IClassFile) element).getType(); would be better to say: element= ((IClassFile) element).getChildren()[0]; (which could answer the IModuleDescription for module-info.class)
New Gerrit change created: https://git.eclipse.org/r/102688
(In reply to Eclipse Genie from comment #1) > New Gerrit change created: https://git.eclipse.org/r/102688 Tested with https://git.eclipse.org/r/#/c/102681/ (for JDT/Core's bug 520651) applied: With JDT/UI HEAD: module-info.class has an empty bread crumb. With https://git.eclipse.org/r/102688 breadbrumb for module-info.class works.
New Gerrit change created: https://git.eclipse.org/r/103130
(In reply to Eclipse Genie from comment #3) > New Gerrit change created: https://git.eclipse.org/r/103130 With latest pending changes in bug 520651, a lot of things are broken in JDT/UI. For this change I searched uses of IClassFile in cast expressions and references to IJavaElement.CLASS_FILE. In a full scan of JDT/UI I tried to adjust all necessary locations, and smoke testing the IDE handling of module-info.class seemed OK. At least this exercise gives us the option to explicitely include/exclude module-info.class from certain functionality. I still left a few TODOs in the patch. See bug 520651 for background discussion, in particular whether or not an additional interface IGenericClassFile would be good to add.
(In reply to Eclipse Genie from comment #3) > New Gerrit change created: https://git.eclipse.org/r/103130 Patch set #2 roughly corresponds to bug 520651 comment 22 ff. In particular the removal of IJavaElement.MODULAR_CLASS_FILE reduces the patch size on client side ( now +219,-174 vs. previously +267,-169 )
(In reply to Stephan Herrmann from comment #5) > (In reply to Eclipse Genie from comment #3) > > New Gerrit change created: https://git.eclipse.org/r/103130 > > Patch set #2 roughly corresponds to bug 520651 comment 22 ff. > In particular the removal of IJavaElement.MODULAR_CLASS_FILE reduces the > patch size on client side ( now +219,-174 vs. previously +267,-169 ) Update as of patch set #3: With latest from bug 520651 (https://git.eclipse.org/r/#/c/103202/4 ) necessary changes are down to +108,-77, i.e. less than half of my first attempt :) I smoke tested use of module-info.class in several regards and reported all my findings in separate bugs. I didn't find any new breakage caused by my change.
Thanks for the patch, Stephan. Writing down some issues and thoughts here. Please have a look at Gerrit for other comments. - Cannot see the source of module-info.class in the editor, e.g. from java.base. - The attached example from bug 520690 doesn't work even after attaching the source. Mostly related to the above issue. - Open "Java Browsing" perspective. Come back to "Java" perspective. Open a module-info.class file. We get this in Error log: java.lang.UnsupportedOperationException: IClassFile#getType() cannot be used on an IModularClassFile at org.eclipse.jdt.internal.core.ModularClassFile.getType(ModularClassFile.java:117) at org.eclipse.jdt.internal.ui.browsing.TypesView.findElementToSelect(TypesView.java:138) - With some other action after the above step, I think "Link with editor" in Java perspective, I got this: java.lang.UnsupportedOperationException: IClassFile#getType() cannot be used on an IModularClassFile at org.eclipse.jdt.internal.core.ModularClassFile.getType(ModularClassFile.java:117) at org.eclipse.jdt.internal.ui.browsing.MembersView.findInputForJavaElement(MembersView.java:262) - In Java browsing perspective, select a src folder > default package > it shows the source module in Types view. But it doesn't show it for a binary module. It should be consistent. - Changes in JavaBrowsingContentProvider.java should be revisited. - package-info.class files should also be smoke tested. - In general, it would be good to check the revision history of the code being changed to see if we are not breaking or missing anything for ordinary (including package-info.class) or modular class files. - We still have usages of the deprecated #getType method, which can also be removed.
Thanks, Noopur. Before going into details: - I intentionally left Java Browsing mostly untouched because it doesn't look like that perspective could easily handle module-info with just a few changes. None of the panels is suitable for showing modules. Still it shouldn't cause exceptions downstream ... Any recommendations how to deal with that perspective? - Some of the functionality may only work with all pending JDT/Core patches. You should get them all nicely piled up if you fetch from https://git.eclipse.org/r/103261 In particular showing sources for module-info.class has been a topic of bug 521058 and bug 520656 Regarding remaining calls to deprecated #getType(): I'd propose to defer them to a follow-up bug, so we can get more field testing as soon as possible. The change is more disruptive than you'd normally want at this point during the game, but the current API is wrong and should never be published as is, so we are a bit between the rock and a hard place.
(In reply to Stephan Herrmann from comment #8) > Any recommendations how to deal with that perspective? As you mentioned, module-info doesn't fit in any of the views in Java Browsing perspective. But since this is a Java element, we should show it somewhere in that perspective. IModuleDescription extends IMember so it may go in Members view, but that will need a selection in Types and Packages view. Currently, it shows module in Types view when default package is selected from the Packages view. We can keep the same behavior if it involves less work. Or, we can show it in Packages view along with other packages. This is how it is presented in Package explorer also.
(In reply to Noopur Gupta from comment #9) > (In reply to Stephan Herrmann from comment #8) > > Any recommendations how to deal with that perspective? > > As you mentioned, module-info doesn't fit in any of the views in Java > Browsing perspective. But since this is a Java element, we should show it > somewhere in that perspective. > > IModuleDescription extends IMember so it may go in Members view, but that > will need a selection in Types and Packages view. > > Currently, it shows module in Types view when default package is selected > from the Packages view. We can keep the same behavior if it involves less > work. > > Or, we can show it in Packages view along with other packages. This is how > it is presented in Package explorer also. OK, to be continued in bug 521133 :) BTW, I favor the last option (whereas implementing IMember is not really a strong statement in this case).
(In reply to Noopur Gupta from comment #7) > Writing down some issues and thoughts here. Please have a look at Gerrit for > other comments. done. > - Cannot see the source of module-info.class in the editor, e.g. from > java.base. > > - The attached example from bug 520690 doesn't work even after attaching the > source. Mostly related to the above issue. As mentioned, with latest from JDT/Core as of bug 520656 / https://git.eclipse.org/r/103261 all this works for me. > - Open "Java Browsing" perspective. Come back to "Java" perspective. Open a > module-info.class file. We get this in Error log: > java.lang.UnsupportedOperationException: IClassFile#getType() cannot be used > on an IModularClassFile I already get it when I got to "Java Browsing" perspective. I will protect those particular calls, but this needs to be continued in bug 521133 > - package-info.class files should also be smoke tested. I just tested a bit, and it didn't smoke :) Latest in gerrit also adopted the new IPF#getAllClassFiles() (bug 520651 comment 38) in jeview, and explicitly leaves AbstractCodeCreationOperation at calling the now deprecated #getClassFiles(), because I believe this should also migrate to #getAllClassFiles() : Import from a Jar will have to create module-info.java from module-info.class, too, right?
Gerrit change https://git.eclipse.org/r/103130 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=34d3b0539d021603c5af79d8e27707119125d4c0
(In reply to Eclipse Genie from comment #12) > Gerrit change https://git.eclipse.org/r/103130 was merged to [BETA_JAVA9]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=34d3b0539d021603c5af79d8e27707119125d4c0 > Released - in order to match the released changes from JDT/Core. Summarizing follow-up work: - minor code clean up in bug 521139 (not urgent) - improve ClassFileDocumentProvider and ClassFileEditorInputFactory via bug 521141 - improve the Java Browsing perspective via bug 521133 - decide in AbstractCodeCreationOperation to - either extend for handling module-info - or change getClassFiles() to getOrdinaryClassFiles() to avoid deprecation (No bug yet but we have the deprecation warning to remind us) thanks, and let me know if I missed anything.
