Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new API for supported/unsupported Java versions #2606

Merged

Conversation

iloveeclipse
Copy link
Member

@iloveeclipse iloveeclipse commented Jun 19, 2024

  • List<String> getAllJavaProjectVersions() - all Java versions that could be used for Java projects inside Eclipse. The difference to existing getAllVersions() API is that later one knows almost all Java versions ever released and might be used not only in JDT core but also in debugger/PDE area.
  • boolean isSupportedJavaProjectVersion(String version) - differs from existing isSupportedJavaVersion() in the same way as explained above
  • String getFirstSupportedJavaVersion() - similar to existing latestSupportedJavaVersion() and should return minimal "default" version supported by JDT.

API above will be used in JDT UI, Debug (PR's are following), and (most likely) PDE.

Internal API added in batch compiler CompilerOptions:

  • getFirstSupportedJavaVersion()
  • getFirstSupportedJdkLevel()

Bumped minor version segments on both core and compiler, even if compiler "only" provides internal API, it is more convenient to have them in-sync.

Related UI/Debugger changes that use new API are:

See #2536

iloveeclipse added a commit to iloveeclipse/eclipse.jdt.debug that referenced this pull request Jun 19, 2024
- Added API
`org.eclipse.jdt.launching.environments.IExecutionEnvironmentsManager.getSupportedExecutionEnvironments()`
- UI shows only supported Java versions starting from 1.8 in container
configuration

Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#451
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this pull request Jun 19, 2024
- Don't propose compilation targets below 1.8 JLS
- don't show old EE's in wizards

Requires eclipse-jdt/eclipse.jdt.debug#452
Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#1465
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this pull request Jun 19, 2024
- Don't propose compilation targets below 1.8 JLS
- don't show old EE's in wizards

Requires eclipse-jdt/eclipse.jdt.debug#452
Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#1465
@iloveeclipse
Copy link
Member Author

@jarthana, @stephan-herrmann, @srikanth-sankaran : could you please check new API? Does it make sense for you?

Related UI/Debugger changes that use new API are:

@iloveeclipse
Copy link
Member Author

The doubts I have is why to have two "supported" sets of Java versions in the API, which seem to be awkward.

The initial idea that with this PDE (and whoever else) could still use "old" API with all possible Java / EE versions, while JDT UI only sees "new" Java versions.

The negative part of it is extra (kind of duplicated) API and that "whoever else" would need code change if the code wanted to see "fully" supported Java versions.

I'm currently thinking about not adding getAllJavaProjectVersions() and isSupportedJavaProjectVersion(String version) but changing behavior / javadoc of getAllVersions() and isSupportedJavaVersion(String version) to point out that it doesn't supply ALL of {@link JavaCore}{@code #VERSION_*} levels as stated today. This would be breaking change and that is my dilemma.

@iloveeclipse
Copy link
Member Author

Another interesting use case for the new getAllJavaProjectVersions() API would be this issue: eclipse-jdt/eclipse.jdt.ui#1448.

The problem there is that in 4.32 we've introduced JavaCore.VERSION_23 which is not supported by 4.32 at all! So it shouldn't appear in getAllVersions() and should report false in isSupportedJavaVersion(String version) in 4.32.

With the API proposed here we would say: yep, JavaCore.VERSION_23 is known by JDT Core but is not supported for anything visible in UI, debugger etc, all places that involve projects etc.

So assuming JDT Core would continue to add Java versions (for JDT LS) that aren't fully supported yet by JDT UI, the new APi would make sense.

@akurtakov, @mickaelistria : WDYT?

@akurtakov
Copy link
Contributor

Another interesting use case for the new getAllJavaProjectVersions() API would be this issue: eclipse-jdt/eclipse.jdt.ui#1448.

The problem there is that in 4.32 we've introduced JavaCore.VERSION_23 which is not supported by 4.32 at all! So it shouldn't appear in getAllVersions() and should report false in isSupportedJavaVersion(String version) in 4.32.

+1

With the API proposed here we would say: yep, JavaCore.VERSION_23 is known by JDT Core but is not supported for anything visible in UI, debugger etc, all places that involve projects etc.

The reason to have (eclipse-jdt/eclipse.jdt.debug#426 eclipse-jdt/eclipse.jdt.debug#429) is so projects can declare they require newer Java EE for the sake of new APIs being added and not for being able to target this bytecode level. So some visibility in projects is required - a prototype how to handle the case of EE being newer than target level compiler supports is at akurtakov/eclipse.jdt.ui@7a2ac1b.

@mickaelistria
Copy link
Contributor

This is all a corollary of the deep question "what does JDT support Java N mean ?": projects targetting this version can load and be developed, ECJ can compile to this version, all features of this versions are supported in JDT Core (ie DOM), all features of this version are supported in UI...

I think your proposal still assumes that JavaCore is only capable of dealing with what ECJ supports, while in practice things such as rewrite could allow to manipulate some different versions (older or newer) than what ECJ supports.
If we're to allow other parser/compiler than ECJ, then we cannot statically know what versions are supported. In theory, I think JDT should simply state the minimal version it's capable to handle in its AST, and everything else such as "this version is not supported" should be produced by the compiler and only surface as errors on the file. That would dis-couple further from ECJ, allowing AST to evolve ahead of ECJ, reduce maintenance when moving forward as less things would have to change. As for Java versions in the future, I'm even wondering whether it makes sense to have a list of them.

I know my answer is not helping on short-term, it's more food for thought, sorry for highjacking the PR.
So more precisely about the PR, I'm more in a "whatever" opinion at the moment, since the deeper thought presented make that I cannot even define what "supported" mean so I cannot build an opinion about such change.

@iloveeclipse
Copy link
Member Author

In theory, I think JDT should simply state the minimal version it's capable to handle in its AST

AST is not the only part of JDT that needs Java version and actually AST itself has its own set of versions it supports.

and everything else such as "this version is not supported" should be produced by the compiler and only surface as errors on the file.

This is too simplistic view and against IDE concept of guiding / helping user. We have wizards and preference pages that do not need to run compiler to detect that project options are incompatible with supported Java.

If we're to allow other parser/compiler than ECJ, then we cannot statically know what versions are supported.

Not quite correct, we now that as IDE we can't support more as we have coded in various wizards/preference pages etc.

This "other parser/compiler" story is of very disruptive to the static nature of JavaCore and CompilerOptions constants / methods, but so far this part doesn't surface in the "classic" IDE at all. The JavaCore constants/methods were developed with a monolithic Java IDE in mind, now the "Java IDE" can internally be decomposed into ("ecj" vs "alternative compiler") + ("JDT Core AST" vs "alternative AST") + ("JDT UI " vs "Java LS / whatever").

Ideally "what is supported" could be also queried per project and derived from the intersection of particular IDE capabilities and project options like "use alternative compiler".

In ideal world, IDE as a whole should be able to know which Java (JLS, runtime) is supported, in which use case, scope and to which degree. This is required to avoid obviously surprising issues like eclipse-jdt/eclipse.jdt.ui#1448 where left hand doesn't know what the right hand does. So even if we know that there will be Java 24, 25 etc, it doesn't make sense to "allow" these versions to appear anywhere in the UI or preferences because we can't really support them with our current version.

So far we only had one set of options / versions we could support, and they didn't really distinguish between compiler, core, ast, UI, debugger etc. Everyone referenced JavaCore versions and in few cases we had special handling for "special" versions. Therefore my proposal here: let continue assume JavaCore would still serve the "monolithic" IDE purpose as before, and look at this PR from this point of view.

@mickaelistria
Copy link
Contributor

("JDT Core AST" vs "alternative AST")

FWIW, this is not what we encourage with #2560 , we do want (and need) to keep the same AST, the only difference is which parser/compiler is used to create this AST. But we do strongly want to work on the very one JDT Core AST/DOM which is the one the ecosystem has gathered around.

Therefore my proposal here: let continue assume JavaCore would still serve the "monolithic" IDE purpose as before, and look at this PR from this point of view.

I'm totally in favor of that. I just brought there food for thought. While I don't think this PR fixes the more general problem that is arising in JDT with alternative compiler, I understand it's still an improvement over the current state of things. So I have no concern with it being merge; I just have little hope that it will be sufficient on the long run.

@jarthana
Copy link
Member

Note: I am bumping the versions for jdt.core and compiler.batch (in advance) before the merge onto BETA_JAVA23. This is being done via #2615

@jarthana jarthana force-pushed the issue_2536_new_constants branch from 42df6e6 to 7820202 Compare June 20, 2024 08:04
@iloveeclipse
Copy link
Member Author

Note: I am bumping the versions for jdt.core and compiler.batch (in advance) before the merge onto BETA_JAVA23. This is being done via #2615

OK. @jarthana : could you please check comments here and give your opinion? I'm in doubt if that is right way to do or not.

@srikanth-sankaran
Copy link
Contributor

Note: I am bumping the versions for jdt.core and compiler.batch (in advance) before the merge onto BETA_JAVA23. This is being done via #2615

OK. @jarthana : could you please check comments here and give your opinion? I'm in doubt if that is right way to do or not.

@jarthana and I reviewed this together and discussed this in detail and I am capturing our comments.

The doubts I have is why to have two "supported" sets of Java versions in the API, which seem to be awkward.

The present effort with #2536 is only to drop support for compiling Java source files at levels 1.7 and below. It is obviously not the case that all support is dropped for those Java levels 1.7 and below. We must still be able to consume binary projects, class files, jars and such artifacts produced at levels not supported by compiler anymore. Various non-compilation operations (search, hierarchy, model construction etc) should span the scope of these binary artifacts produced at levels not supported for compilation.

As a result, it is best not to modify the semantics and operations of existing API points, but introduce additional new ones that may satisfy the needs of any and all interested parties.

However, the present choice of names can be a source of confusion. Here are suggested alternatives that may be considered:

Rather than JavaCore.getAllJavaProjectVersions() how about JavaCore.getAllJavaVersionsSupportedAtSource() or alternatively JavaCore.getAllJavaSourceVersionsSupportedByCompiler()

Likewise, instead of JavaCore.isSupportedJavaProjectVersion(String version) how about JavaCore.isJavaVersionSupportedAtSource(String version) or alternatively JavaCore.isJavaSourceVersionSupportedByCompiler(String version)

Likewise instead of JavaCore.getFirstSupportedJavaVersion() how about JavaCore.getFirstJavaVersionSupportedAtSource() or JavaCore.getFirstJavaSourceVersionSupportedByCompiler()

@iloveeclipse
Copy link
Member Author

Thanks Srikanth & Jay. I've checked the usage of these methods and I agree that the "compiler" related method names fit better:

JavaCore.getAllJavaSourceVersionsSupportedByCompiler()
JavaCore.isJavaSourceVersionSupportedByCompiler(String version)
JavaCore.getFirstJavaSourceVersionSupportedByCompiler()

Here are callers and it makes sense looking who consumes all that:
image

I will proceed with that for now.

- `List<String> getAllJavaSourceVersionsSupportedByCompiler()` - all
Java versions that could be used for Java projects inside Eclipse. The
difference to existing `getAllVersions()` API is that later one knows
almost all Java versions ever released and might be used not only in JDT
core but also in debugger/PDE area.
- `boolean isJavaSourceVersionSupportedByCompiler(String version)` -
differs from existing `isSupportedJavaVersion()` in the same way as
explained above
- `String getFirstJavaSourceVersionSupportedByCompiler()` - similar to
existing `latestSupportedJavaVersion()` and should return minimal
"default" version supported by JDT.

API above will be used in JDT UI, Debug (PR's are following), and (most
likely) PDE.

**Internal** API added in batch compiler `CompilerOptions`:
- `getFirstSupportedJavaVersion()`
- `getFirstSupportedJdkLevel()`

See eclipse-jdt#2536
@iloveeclipse iloveeclipse force-pushed the issue_2536_new_constants branch from 7820202 to 3fcbbf9 Compare June 26, 2024 15:01
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this pull request Jun 26, 2024
- Don't propose compilation targets below 1.8 JLS
- don't show old EE's in wizards

Requires eclipse-jdt/eclipse.jdt.debug#452
Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#1465
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.debug that referenced this pull request Jun 26, 2024
- Added API
`org.eclipse.jdt.launching.environments.IExecutionEnvironmentsManager.getSupportedExecutionEnvironments()`
- UI shows only supported Java versions starting from 1.8 in container
configuration

Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#451
@iloveeclipse iloveeclipse merged commit 975e7c9 into eclipse-jdt:master Jun 26, 2024
9 checks passed
@iloveeclipse iloveeclipse deleted the issue_2536_new_constants branch June 26, 2024 18:18
iloveeclipse added a commit to eclipse-jdt/eclipse.jdt.debug that referenced this pull request Jun 27, 2024
- Added API
`org.eclipse.jdt.launching.environments.IExecutionEnvironmentsManager.getSupportedExecutionEnvironments()`
- UI shows only supported Java versions starting from 1.8 in container
configuration

Requires eclipse-jdt/eclipse.jdt.core#2606
See #451
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this pull request Jun 27, 2024
- Don't propose compilation targets below 1.8 JLS
- don't show old EE's in wizards

Requires eclipse-jdt/eclipse.jdt.debug#452
Requires eclipse-jdt/eclipse.jdt.core#2606
See eclipse-jdt#1465
iloveeclipse added a commit to eclipse-jdt/eclipse.jdt.ui that referenced this pull request Jun 27, 2024
- Don't propose compilation targets below 1.8 JLS
- don't show old EE's in wizards

Requires eclipse-jdt/eclipse.jdt.debug#452
Requires eclipse-jdt/eclipse.jdt.core#2606
See #1465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants