Community
Participate
Working Groups
A "module-info.class" ClassFile contains "module-info" BinaryType as its child, even though it is not a type.
When I make ClassFile answer the IModuleDescription as its only child, then org.eclipse.jdt.internal.ui.javaeditor.JavaEditorBreadcrumb.getInput(IJavaElement) will answer null, which eventually causes java.lang.NullPointerException at org.eclipse.jdt.internal.core.SourceRefElement.hasChildren(SourceRefElement.java:232) at org.eclipse.jdt.ui.StandardJavaElementContentProvider.hasChildren(StandardJavaElementContentProvider.java:235) at org.eclipse.jdt.internal.ui.javaeditor.JavaEditorBreadcrumb$JavaEditorBreadcrumbContentProvider.hasChildren(JavaEditorBreadcrumb.java:386) @Noopur, what do you suggest for coordinating changes in Core and UI? OK to book it on the same (this) bug? It seems the following should do the trick, regardless of what we do in Core: if (element instanceof IClassFile) element= ((IClassFile)element).getChildren()[0];
New Gerrit change created: https://git.eclipse.org/r/102681
(In reply to Eclipse Genie from comment #2) > New Gerrit change created: https://git.eclipse.org/r/102681 Main change: ClassFile.buildStructure() now has disjoint branches for type & module (only one can apply). BinaryModule (which now is a child of ClassFile) bogusly assumed to find its children in the ClassFileInfo (leading to infinite recursive trees) - hardcoded now that modules have no children - ensure that all infos have children & binaryChildren initialized (was: NPE in various places) More locations may expect each ClassFile to contain a BinaryType, I found: - org.eclipse.jdt.core.dom.ASTParser.internalCreateAST(IProgressMonitor) Not sure, how to find all remaining locations making this assumption.
(In reply to Stephan Herrmann from comment #1) > When I make ClassFile answer the IModuleDescription as its only child... The Javadoc of IClassFile says "A class file has a single child of type IType". Will IClassFile.getType() still return a type? That may be confusing. > @Noopur, what do you suggest for coordinating changes in Core and UI? OK to > book it on the same (this) bug? It seems the following should do the trick, > regardless of what we do in Core: > > if (element instanceof IClassFile) > element= ((IClassFile)element).getChildren()[0]; Not sure if the fix from jdt.core will need updates at other locations also in the clients (jdt.ui, jdt.debug etc.). It would be better to have a separate bug for JDT UI.
(In reply to Noopur Gupta from comment #4) > (In reply to Stephan Herrmann from comment #1) > > When I make ClassFile answer the IModuleDescription as its only child... > > The Javadoc of IClassFile says "A class file has a single child of type > IType". Will IClassFile.getType() still return a type? That may be confusing. Legally, Java 9 is a breaking change here. Pragmatically I would just update the javadoc ("either IType or IModuleDescription"). Do we have other options? - Create an IModularClassFile as a third option below ITypeRoot? That would break the contract of ITypeRoot (either ICompilationUnit or IClassFile). - Also the status-quo is a breaking change as it provides an IType that is not a type (this being both illegal and ugly). @Dani? > > @Noopur, what do you suggest for coordinating changes in Core and UI? OK to > > book it on the same (this) bug? It seems the following should do the trick, > > regardless of what we do in Core: > > > > if (element instanceof IClassFile) > > element= ((IClassFile)element).getChildren()[0]; > > Not sure if the fix from jdt.core will need updates at other locations also > in the clients (jdt.ui, jdt.debug etc.). It would be better to have a > separate bug for JDT UI. here you go: bug 520691
(In reply to Stephan Herrmann from comment #5) > here you go: bug 520691 Thanks! Based on the decision in jdt.core, it would require checking all invocations of IClassFile.getType() in jdt.ui and handling them appropriately, if required. This may impact other clients of jdt.core also e.g. jdt.debug and the external clients as well.
(In reply to Noopur Gupta from comment #6) > Based on the decision in jdt.core, it would require checking all invocations > of IClassFile.getType() in jdt.ui and handling them appropriately, if > required. > > This may impact other clients of jdt.core also e.g. jdt.debug and the > external clients as well. Ergo s.t. for the README / migration guide, right? (unless s.o. finds the magical wand to maintain compatibility). Clients could also be stupid enough to approximate IClassFile.getType() with (IType)IClassFile.getChildren()[0] just saying...
Searching for references to IClassFile.getType() in a JDT (Core + UI) workspace yields 83 results (not counting the literally hundreds of occurrences in tests!). Inspecting (and adjusting) them all is not really a fun task. The least we could do is, have getType() on a module-info.class answer an IType that doesn't exist(). But that would probably just change throwing NPE into throwing JavaModelException("does not exist"). This also raises the question, whether it's OK to let getType() answer a type that's not present in getChildren(). We could also try to reduce the flood of problems, by preventing those ClassFile instances from spilling into client code in the first place. Thinking aloud: what would happen if IPackageFragment would *not* list module-info.class among its children, if separate API would need to be used instead to find them? Interestingly, the source case (ICompilationUnit) doesn't seem to have this problem, since ICompilationUnit.getTypes() can already return an empty array prior to Java 9.
(In reply to Stephan Herrmann from comment #8) > We could also try to reduce the flood of problems, by preventing those > ClassFile instances from spilling into client code in the first place. > Thinking aloud: what would happen if IPackageFragment would *not* list > module-info.class among its children, if separate API would need to be used > instead to find them? Anyone have an opinion about adding IPackageFragment#getModularClassFile() and let #getClassFiles() *not* answer these? Right now this is the best I can think of to reduce the risk of breaking clients. Also, how should ClassFile("module-info.class") react to getType()? Anwser null or throw? Or should we rather break the contract of ITypeRoot and introduce IModularClassFile to be safe once a cast to IClassFile was successful? I believe this needs fixing sooner rather than later. I'm willing to do the changes, but I feel I need a +1 from one of you guys.
(In reply to Stephan Herrmann from comment #5) > Legally, Java 9 is a breaking change here. > Or should we rather break the contract of ITypeRoot and introduce > IModularClassFile to be safe once a cast to IClassFile was successful? +1 to this. Bug 490103 is another example where we had no option but to break the contract. We are introducing a new classpath entry kind.
(In reply to Jay Arthanareeswaran from comment #10) > (In reply to Stephan Herrmann from comment #5) > > Legally, Java 9 is a breaking change here. > > > Or should we rather break the contract of ITypeRoot and introduce > > IModularClassFile to be safe once a cast to IClassFile was successful? > > +1 to this. Thanks, will do.
Preparing for the actual change: While trying to wrap my head around all representations of modules in JDT/Core I found some irregularities. I'll put a few fixes for those into a separate preparatory commit. I'm looking at representations in these 5 areas: (A) o.e.j.internal.compiler.classfmt.* (B) o.e.j.internal.compiler.env.* (C) subclasses of org.eclipse.jdt.internal.core.JavaElementInfo (D) subclasses of org.eclipse.jdt.internal.core.JavaElement (E) subtypes of org.eclipse.jdt.core.IJavaElement The initial surprise is the sheer proliferation of new types relating to modules, several of which having identical simple names. (A): Slightly odd, but tolerable: ClassFileReader represents both the class file and its type (if present). By contrast, classfmt.ModuleInfo is obtained as a separate thing via the ClassFileReader (which in that case does not represent a type). (B): While IGenericType has subtypes IBinaryType & ISourceType, env.IModule has no such distinction. => I propose at least a new env.IBinaryModule as the type by which Java model will see classfmt.ModuleInfo, similar to env.IBinaryType representing classfmt.ClassFileReader. The new interface remains empty, it's just a marker, so at the other end of the spectrum we have a clue of what's behind, without leaking type classfmt.ModuleInfo. A corresponding env.ISourceModule could be added later, if desired. With this placebo fix (and my confusion resolved/reduced) also bug 517037 should be easier to address. (C) org.eclipse.jdt.internal.core.ModuleInfo was introduced via a commit containing the word "crude" ;p and I'm afraid I have to agree in this particular aspect. This class oddly connects three worlds: env.IModule, o.e.j.internal.core.SourceTypeElementInfo and ..ast.ModuleDeclaration (from which it is initialized). I found no justification for extending SourceTypeElementInfo, and the single use of this type (in the builder!) can easily be replaced by the similar batch.BasicModule, still a slight breach of encapsulation, but ... => I propose to remove this class and replace the single use as mentioned. At this point, Ctrl+Shift+T "ModuleInfo" starts to make sense :) org.eclipse.jdt.internal.core.ModuleDescriptionInfo: I would have expected this to be named ModuleDescriptionElementInfo (to clearly distinguish from classfmt.*Info), but no big deal. (D) It's not clear to me, why org.eclipse.jdt.internal.core.AbstractModule uses ModuleDescriptionInfo, which I'd expect to happen only in SourceModule. BinaryModule should use only the new env.IBinaryModule (plus the parent ClassFileInfo), and AbstractModule should be agnostic to this choice. I haven't yet changed anything in AbstractModule. BinaryModule should not have any children. Nothing changed yet. (E) mostly looks good to me :) Some question still: Looking at ISourceType vs IBinaryType (public API) one of the differences seems to be the source view (possibly using unresolved simple names) vs. the resolved binary view, right? Should IModuleDescription support the same distinction for all references to types (service provide/use)? (I just checked: yes, "provides" can leverage an import and thus use a simple name - do we have any tests for this?). And why do we say "IModuleDescription"? I could not find such terminology in JLS. API doc of java.lang.module speaks of "module descriptors". Is that what we mean?
Another irregularity: model's ClassFile can construct its contained main type as a pure handle simply using the known filename to derive the type name. By contrast, ModularClassFile cannot provide an IModuleDescription as a pure handle operation, unless the client provides the module name. I'm working around this by letting ModularClassFile#getModule() call openWhenClosed() if needed. Should I add #getModule(String) to have a pure-handle method? Another option would be to change BinaryModule such that it is opened when the module name is retrieved (rather than have the module name as a ctor argument), but that would be against the customary use of NamedElement.
New Gerrit change created: https://git.eclipse.org/r/103100
New Gerrit change created: https://git.eclipse.org/r/103101
(In reply to Eclipse Genie from comment #14) > New Gerrit change created: https://git.eclipse.org/r/103100 Preparation, see comment 12
(In reply to Eclipse Genie from comment #15) > New Gerrit change created: https://git.eclipse.org/r/103101 Main change: Split ClassFile into 3: - ClassFile: only those parts for ordinary class files, i.e., containing an IType indeed, no handling of module-info - ModularClassFile: those parts that handle module-info.class, no handling of contained ITypes - AbstractClassFile for common parts shared among the two If you compare each of these 3 against the original ClassFile.java, you should be able to see which parts were modified vs. just moved (suppress white space changes to view real differences). Only ModularClassFile has real differences: - handling of fileName is replaced by a constant "module-info.class" (ctor and some others) - buildStructure is a cleaned-up extract from the original mixed implementation - findPrimaryType() always returns null (legal per javadoc) - getJarBinaryModuleInfo() (extracted from getJarBinaryTypeInfo()) is still slightly crippled due to missing module support around the new index - no support for external annotations (only @NNBD applicable, which we don't support in .eea) - getWorkingCopy() not yet implemented here, may need new class ModularClassFileWorkingCopy. Searching for the right place to introduce getModule() I struggled with ICompilationUnit not being able to guarantee non-null return, as IModularClassFile does. Still I propose a generalized ITypeRoot.getModule() which spells out that for the general casethis may return null. Only IModularClassFile strengthens the contract. So clients wishing to obtain an IModuleDescription have two options: - check only for ITypeRoot and then check the result for null - check for IModularClassFile and rely on non-null result MatchLocator.reportMatching(ModuleDeclaration..) is a witness that clients need no changes if they already check for null module. Follow-up changes in IPackageFragment etc.: - document that IPF#getClassFile(String) will never answer the class file for module-info.class, raise IllegalArgumentException if this attempted. - add new IPF#getModularClassFile() - similar doc change for IPF#getClassFiles(), and class javadoc of IPF - similar doc change for JavaCore#createClassFileFrom(), no variant for module-info, yet (FIXME?) - ensure that children of *PackageFragment indeed contain module-info.class (if present) - NameLookup.seekTypesInBinaryPackage() exemplifies a cast that now needs protection by instanceof (and then calling getType() is safe) - internally, PackageFragment#getAnyClassFile(..) can be used if you don't care about type/module distinction Another follow-up is in ASTParser from the previous https://git.eclipse.org/r/102681 (adjusted) Let BinaryModule.getChildren() always answer NO_ELEMENTS (see comment 12 (D)). Open issues: - SourceMapper doesn't seem to support module-info, see getType() but no getModule() - we probably need JEM_MODULAR_CLASSFILE, not yet added
New Gerrit change created: https://git.eclipse.org/r/103120
(In reply to Eclipse Genie from comment #18) > New Gerrit change created: https://git.eclipse.org/r/103120 Working on follow-up changes in JDT/UI I realized that all over the place we rely on the implication: see IJavaElement.CLASS_FILE => safe to cast to IClassFile. This is actually part of the contract. When IModularClassFile shares IJavaElement.CLASS_FILE this will break the contract and create gazillions of CCE. Hence the new change (on top of the previous two) introduces IJavaElement.MODULAR_CLASS_FILE and sprinkles this into many places that previously only checked CLASS_FILE. Thinking about migration strategies, these are the first things to check - all casts to IClassFile It is no longer safe to infer (x instanceof ITypeRoot && !(x instanceof ICompilationUnit)) => safe to cast to IClassFile, must also consider IModularClassFile - all references to IJavaElement.CLASS_FILE - if module-info.class should be handled the same way, simply add one more case label - otherwise check if not matching any case label would cause an exception In my pending patch for JDT/UI there's a number of locations, where I need a common type for IClassFile and IModularClassFile. So far it works OK to use ITypeRoot (and document, that ICompilationUnit is handled separatedly, if so). For clients it would be cleaner to have one more type above them two, say IGenericClassFile There's actually one method to be placed in this type byte[] getBytes() // :) I'm not sure if the additional API carries its own weight. Anyone?
(In reply to Stephan Herrmann from comment #12) > And why do we say "IModuleDescription"? I could not find such terminology in > JLS. API doc of java.lang.module speaks of "module descriptors". Is that > what we mean? Just got from holidays, and lot of comments to read through :) I will just answer this part. As a I mentioned in bug 486011, comment #1, I thought it is better to see "module" as two different things: 1. What the module-info contains. 2. Module as a container element, between IJavaProject and IPackageFragmentRoot. Per this IModuleDescription had to be different from IModule.
More observations: With the proposed change IModuleDescription#getClassFile() is broken, as it cannot return an IModularClassFile. Would need a new method #getModularClassFile() (unless we cut the subtyping between IModuleDescription and IMember - in which case we would be free to name that new method "getClassFile" just with new signature). Also needed: ToolFactory.createDefaultModularClassFileReader(IModularClassFile, int) which then would be a step towards ModularClassFileWorkingCopy. I already have a FIXME requesting JavaCore.createModularClassFileFrom(IFile)
After this has caused ripples (here and in bug 520691) which run deeper than we could possibly like at this point in the game: ====================================================== Here's another idea: ====================================================== --- Pure additions: --- interface IModularClassFile extends IClassFile IModuleDescription getModule() interface IOrdinaryClassFile extends IClassFile IType getType() in IPackageFragment: IModularClassfile getModularClassFile() IOrdinaryClassFile getOrdinaryClassFile(String) IOrdinaryClassFile[] getOrdinaryClassFiles() --- And then we just say in IClassFile: --- @Deprecated IType getType() and really work through the process to eventually *remove* that method. Until then it may throw an exception if invoked on a modular class file. (Too bad we can't yet say "@Deprecated(forRemoval)" :) ) --- Behind the scenes I already have the corresponding implementation classes, just the class->ifc relation would be changed to: AbstractClassFile implements IClassFile ClassFile extends AbstractClassFile implements IOrdinaryClassFile ModularClassFile extends AbstractClassFile implements IModularClassFile All uses of IClassFile can basically remain, unless calling getType(). So far all looks nicer to me. Of course the devil is in the details, next: What about IJavaElement.CLASS_FILE? If we keep the contract that this ID allows casting to IClassfile, then some clients would need a real instanceof-check on top, before they can ask getType() or getModule(). We could offer IJavaElement IClassFile#getMainElement() to ease this transition. In this case no additional constant for modular class files would be introduced in IJavaElement. Wouldn't the deprecation approach take most of the stress from the migration task?
New Gerrit change created: https://git.eclipse.org/r/103202
(In reply to Eclipse Genie from comment #23) > New Gerrit change created: https://git.eclipse.org/r/103202 Plan B as a single commit (on top of only https://git.eclipse.org/r/#/c/103100 )
@Stephan, JDT Debug uses IClassFile.getType() What are the guidelines to make the changes ?
(In reply to Sarika Sinha from comment #25) > @Stephan, > JDT Debug uses IClassFile.getType() > > What are the guidelines to make the changes ? First JDT/Core needs to decide what will be our API going forward. As per my plan B (comment 22) getType() will get deprecated on IClassFile, but for a while it still exists, just it may throw an exception when invoked on a "modular class file". To avoid this, clients should test "classFile instanceof IOrdinaryClassFile" before calling getType(). After a corresponding cast the 'new' IOrdinaryClassFile#getType() can be safely called. There will be use cases, where you *only* want ordinary class files, in which case you can use, e.g., IPackageFragment#getOrdinaryClassFile(String) / #getOrdinaryClassFiles(). In other situations you may need all class files, in which case you may continue to use the old methods returning IClassFile, which will then comprise both kinds. This should be all for getType(), similar considerations apply to #getModule() from IModularClassFile. Additionally, I've seen code that assumes correspondence between the file name and the name of the contained type. Such correspondence obviously doesn't exist for module-info. It would actually help for JDT/Core if you could comment whether this sounds like a viable migration strategy (or ask back if anything is unclear).
I updated the change (patch set #4) to complete my "plan B" from comment 22. With this, necessary follow-up changes in JDT/UI could be much reduced (see bug 520691). It would be great if s.o. could approve the API changes in org.eclipse.jdt.core.I* It would be great to hear an inside opinion (Jay) and at least one client p.o.v (Noopur, Sarika?). I will then write a blurp for a migration guide following the lines of comment 26. The key improvement over the previous attempt was to keep IClassFile as meaning "any class file". As a result we no longer need to introduced IJavaElement.MODULAR_CLASS_FILE (as I did before). The dangerous part is an old method (getType()), which can now throw a new exception, and which for that reason is now deprecated. A new, non-deprecated method has been in introduced in the subtype, where it can be safely called. The subtype (IOrdinaryClassFile) can be accessed either using instanceof and cast, or using new accessors in IPackageFragment. This seems to be the best compromise between compatibility and conceptual correctness (although I know that the model is still not fully symmetric, but that's neither new, nor fully avoidable).
after tests timed out, a successful build can be found as https://hudson.eclipse.org/platform/job/eclipse.jdt.core-Gerrit/3100/ on behalf of bug 521058 (commit sits on top of the commits for this bug).
JDT Debug - Looks ok with the check of instanceof IOrdinaryClassFile. No breakpoints in module-info.java so less problem scope.
(In reply to Stephan Herrmann from comment #27) This approach looks fine and is better than the previous ones. Please have a look at other comments in bug 520691.
Just one point from my side: If we kept the IPackageFragmentRoot() intact (i.e. not to include module-info) along with all other proposed changes, then interested clients can look up getModularClassFile(). That way, less clients will be broken today? Did I miss something amid the discussion?
(In reply to Jay Arthanareeswaran from comment #31) > Just one point from my side: If we kept the IPackageFragmentRoot() intact > (i.e. not to include module-info) I can't follow here: module-info is a child of IPackageFragment, not of IPackageFragmentRoot. Please explain. I think the root cause for breakage can best be described as: IPackageFragment#getClassFiles() rightfully returns "everything *.class", some of which happen to break the contract of IClassFile. Do you propose to silently exclude module-info.class from getClassFile()? If we go down that road then almost nothing would work for that file. For each function touching classfiles in one way or other we'd need to add specific implementation for module-info.class It's kind of deciding between opt-in (by default nothing works) / opt-out (by default everything tries to work) or a mix. My proposal gives both options: use the old methods and then in one particular branch exceptions can be raised. When migrating, clients can select where they want all class files, only ordinary, or only modular ones. If you see a better strategy please elaborate.
(In reply to Stephan Herrmann from comment #32) > (In reply to Jay Arthanareeswaran from comment #31) > > Just one point from my side: If we kept the IPackageFragmentRoot() intact > > (i.e. not to include module-info) > > I can't follow here: module-info is a child of IPackageFragment, not of > IPackageFragmentRoot. > > Please explain. > > I think the root cause for breakage can best be described as: > IPackageFragment#getClassFiles() rightfully returns "everything *.class", > some of which happen to break the contract of IClassFile. > > Do you propose to silently exclude module-info.class from getClassFile()? Sorry, my bad. Of course, I meant IPackageFragment#getClassFiles(). If we just exclude the module-info from this list, we perhaps break the contract that it should return every .class file, but clients who are using that code and those that are not expecting a module class are not broken. That is okay IMO, because even if they get it in the list, they wouldn't know what to do with it, let alone the fact that it is very different from their understanding of a class file. And those who wish to upgrade to the module world can do so with all new APIs. I am just saying that this will be the least troublesome migration path for clients.
So, would your proposal amount to the following rename in IPackageFragment: getClassFiles -> getAllClassFiles getOrdinaryClassFiles -> getClassFiles Plus the new method (unchanged) getModularClassFile() After JDT/Core has been fully adjusted to the change, inside we should do the rename by refactorings so that we don't lose all the work in this bug. I predict this to significantly bloat the change, but if that's purely mechanical, that may be fine. I might also include JDT/UI with changes in bug 520691, so that we don't break the newly fixed stuff there, either.
If we do the rename, we should probably still add getOrdinaryClassFiles() as an alias just for symmetry. Ideally, I would also make the getClassFiles() with restricted semantics @Deprecated, just to alert users that it's semantic has changed in a way. Here deprectation doesn't necessarily imply removal, since there's no danger of exception, just clients may receive a smaller answer than they expect.
(In reply to Stephan Herrmann from comment #35) > If we do the rename, we should probably still add getOrdinaryClassFiles() as > an alias just for symmetry. Yes, just to make sure we are talking about the same thing: I suggest we simply remove this Javadoc from getClassFiles(): Note: the returned list may contain ordinary class files as well as a modular class file (for "module-info.class"). Or simply change this to say, "this list does not include modular class file." I also agree with adding all other newly proposed APIs. I am also open to not having the getAllClassFiles() for now, unless of course there are client that require this.
(In reply to Jay Arthanareeswaran from comment #36) > (In reply to Stephan Herrmann from comment #35) > > If we do the rename, we should probably still add getOrdinaryClassFiles() as > > an alias just for symmetry. > > Yes, just to make sure we are talking about the same thing: > > I suggest we simply remove this Javadoc from getClassFiles(): > > Note: the returned list may contain ordinary class files as well as a > modular class file (for "module-info.class"). > > Or simply change this to say, "this list does not include modular class > file." OK, that's the API perspective. Implementationwise this would imply to change the implementation to invoke getOrdinaryClassFiles() instead of the current code. And all current invocations in JDT/Core (after the change under discussion) need to be changed to call getAllClassFiles() instead. > I also agree with adding all other newly proposed APIs. thanks. > I am also open to not having the getAllClassFiles() for now, unless of > course there are client that require this. I currently can't check but doesn't JDT/Core itself use getClassFiles() in many locations, or does my patch change all of those invocations to getOrdinaryClassFiles()? Same for JDT/UI. I think we have at least two clients already who require this :)
I was wrong, it almost never is correct in JDT/Core nor UI to call IPF#getAllClassFiles(). So, excluding modular class files from getClassFiles() will protect clients from unexpectedly seeing modular class files, without much loss of functionality. I still think we should - add getAllClassFiles(), so clients can make their choice explicit. - deprecate getClassFiles(), so clients are alerted there is a choice to be made Despite the deprecation, I see no need to ever remove that method (in contrast to IClassFile#getType()). With this the migration story will go like this: " IClassFile can now represent two different things, an ordinary class file containing exactly one type, or a modular class file containing exactly one module descriptor. This implies that IClassFile#getType() is now unsafe, for which reason it is deprecated. To safely call getType() clients should make sure they have an IOrdinaryClassFile (new subtype of IClassFile). Class files are typically acquired from the containing package fragment. There is a risk now that clients inadvertently work with an IModularClassFile (representing "module-info.class") and then later call getType(), which causes an exception at runtime. To protect clients from this situation the method IPackageFragment#getClassFiles() has been restricted to only return ordinary class files. This method, however, is deprecated, too, because clients are advised to explicitly choose one of the new methods getAllClassFiles(), getOrdinaryClassFiles() or getModularClassFile() (with corresponding return types IClassFile[], IOrdinaryClassFile[] and IModularClassFile, respectively), in order to make their intent explicit. When requesting a single class file via IPackageFragment#getClassFile(String) clients are free to request an ordinary class file or a modular class file (by passing the string "module-info.class"), but here it is seen as the client's responsibility to decide whether both kinds are OK or not. Still, for explicit requests new methods getOrdinaryClassFile(String) and getModularClassFile() exist (again with most suitable return types). " Patch set 5 ( https://git.eclipse.org/r/#/c/103202/5 ) contains the revised version.
(In reply to Stephan Herrmann from comment #38) > Patch set 5 ( https://git.eclipse.org/r/#/c/103202/5 ) contains the revised > version. Amended in #6 with mentioning the exception that #getType() may now throw.
Gerrit change https://git.eclipse.org/r/103202 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=25071e6ed2edcb8faae73209cbe4aeaceb937d78
Gerrit change https://git.eclipse.org/r/103100 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=27c39f2d52997455d4e4e1030d3682e1263712f5
I made final editorial changes (copyright & javadoc for IOrdinaryClassFile.getType()) and released everything. Thanks for the feedback, and sorry if my responses may have been a bit defensive, suggestions were right on. Also let me know of any issues I may have forgotten or that are detected by field testing (once also the JDT/UI part is released - coming in a minute).
(In reply to Stephan Herrmann from comment #38) Perfect! I agree with everything. What about others? Anybody has reservations/opinions?
Setting keyword "readme" as a reminder. What's the process to assemble the migration guide?
New Gerrit change created: https://git.eclipse.org/r/103790
bug 519444 comment 47 showed us that ModularClassFile.getJarBinaryModuleInfo() was still incomplete. It had a few code blocks copied from ClassFile.getJarBinaryTypeInfo(), where I had hoped these will only be relevant once we integrate modules in the new index. Obviously that hope was wrong. (In reply to Eclipse Genie from comment #45) > New Gerrit change created: https://git.eclipse.org/r/103790 Fills the gap by minimal copies from corresponding code for types. Additionally, I had to make tests a bit smarter: when creating a project with JCL19_LIB (should be renamed to JCL9_LIB), that classpath entry needs to be marked as "module", otherwise compilation will not find module java.base. I think this only affected the JclMin approach, which unlike the real JRE is not a Jrt container.
Gerrit change https://git.eclipse.org/r/103790 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=e5360e04e6ea5683cbe34b165b0e694733f4efde
