Remove compiler support for JLS source/target < 1.8 #2551

iloveeclipse · 2024-06-11T16:44:07Z

This is a draft, many tests have to be removed or updated as they test compliance with non supported JLS versions.

I've tried to keep all tests enabled under 1.8 (which I could understand so far) even if they were marked for older version.

Fixes #2536

TODO's

See #2761

iloveeclipse · 2024-06-11T16:47:17Z

@stephan-herrmann, @srikanth-sankaran, @mpalat, @jarthana : please check this PR, even if it is a draft, because I want know if the approach taken makes sense for you or you see much easier / better way to obsolete older versions support.

Note that many tests are failing because they test something valid for Java < 1.8 only. I will check them and disable some I'm sure are, for the rest I will probably need your help.

But first check if the direction is the right one.

...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties

srikanth-sankaran · 2024-06-12T10:51:28Z

But first check if the direction is the right one.

I quickly glanced through and the over all approach looks OK, tomorrow I will actually replicate the PR locally and play around with it and share feedback

iloveeclipse · 2024-06-12T11:09:57Z

My idea is to "cut" old versions in compiler level first (I left the version constants there, only removed from tests), so do a "small enough" change now and look at the bigger cleanup in the second (third etc) steps, cleaning up both code in compiler and JDT core, and also upwards in JDT UI and PDE (which tests currently even reuse some removed constants from JDT tests).

At any time my aim is to have no failing tests of course, which would mean that some (obsoleted) tests could be still present if they run fine on Java 8+ and some still useful tests might be disabled temporarily till it is clear whether they should be properly fixed or removed.

Feel free to push any change to the branch here if you see a need for that.

jarthana · 2024-06-12T11:20:31Z

@stephan-herrmann, @srikanth-sankaran, @mpalat, @jarthana : please check this PR, even if it is a draft, because I want know if the approach taken makes sense for you or you see much easier / better way to obsolete older versions support.

Note that many tests are failing because they test something valid for Java < 1.8 only. I will check them and disable some I'm sure are, for the rest I will probably need your help.

I don't mean to add more work, but it will be nice if we can identify all such tests and move them to separate suites so that they can be disabled altogether. If anyone interested, they can always test just those suites for < 1.8.

iloveeclipse · 2024-06-12T11:26:41Z

I don't mean to add more work, but it will be nice if we can identify all such tests and move them to separate suites so that they can be disabled altogether. If anyone interested, they can always test just those suites for < 1.8.

With the changes here merged, there will be no more possibility to test with Java < 1.8, compiler will simply reject anything below 8.

srikanth-sankaran · 2024-06-12T11:58:45Z

With the changes here merged, there will be no more possibility to test with Java < 1.8, compiler will simply reject anything below 8.

+1

IMHO we will be serving no one by leaving in half baked unmaintained, half tested support for ancient versions.

stephan-herrmann · 2024-06-21T15:56:38Z

Sorry, I can't find the time to give valuable input right now, but I can at least try to play smart-ass: commit comment keeps saying "Test adoption" but I don't think you want to adopt any tests. Is "adaptation" what you are doing? ;-P

iloveeclipse · 2024-06-24T15:59:38Z

@laeubi : could it be, that this block is executed in a random order, or order not related to the definition order in the pom.xml if tests run in tycho:

eclipse.jdt.core/org.eclipse.jdt.core.tests.model/pom.xml

Lines 36 to 40 in 92c4a1d

    
           <includes> 
        
           	<include>org/eclipse/jdt/core/tests/model/AllJavaModelTestsTracing.class</include> 
        
           	<include>org/eclipse/jdt/core/tests/dom/RunAllTestsTracing.class</include> 
        
           	<include>org/eclipse/jdt/core/tests/RunFormatterTests.class</include> 
        
           </includes>

I assume that the CompletionTests failures I see in Jenkins depend on some previous testsuite that messes up with the environment (JavaOptions and the like), because I can't reproduce the fails locally.
I've tried to change order, but it seem to have no effect, AllJavaModelTestsTracing seem to be always executed after RunAllTestsTracing.

iloveeclipse · 2024-06-24T18:03:48Z

OK, not running RunFormatterTests seem to cause lot of test fails (~2000) , which mean, many other model tests are not self-contained and require something that is set by RunFormatterTests in the environment...
See https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/PR-2551/21/display/redirect .

Still, CompletionTests fail. So next attempt is to disable everything except AllJavaModelTestsTracing...

iloveeclipse · 2024-07-25T04:58:02Z

🤞 I'll be near a radio tomorrow to hear the news when this explodes 😎

OK, so let the fun begin.

Build failed because we have bundles in the SDK that are still compiled against 1.6 :-(

https://ci.eclipse.org/releng/job/Builds/job/I-build-4.33/80/console

iloveeclipse · 2024-07-25T05:45:08Z

See eclipse-platform/eclipse.platform.releng.aggregator#2197

jarthana · 2024-07-25T05:52:14Z

  * using source from project `Converter`, `Converter15`, however, selects the AST level from the project.
    @srikanth-sankaran , @jarthana  does it make any sense have such tests here? Shouldn't tests always sit in a test class corresponding to the used `Converter*` project, e.g., `ASTConverterAST3Test`?

Yes, ideally. I have seen similar issues in many other places. We load projects from everywhere for convenience and set compliance or api level as required. and that's why we often run into issues in this area, for e.g. #2743

laeubi · 2024-07-25T06:35:24Z

🤞 I'll be near a radio tomorrow to hear the news when this explodes 😎

OK, so let the fun begin.

Build failed because we have bundles in the SDK that are still compiled against 1.6 :-(

https://ci.eclipse.org/releng/job/Builds/job/I-build-4.33/80/console

What I'm wondering:
Could it maybe just be made that ECJ emits a WARNING (or even ERROR) but do not abort compilation and simply choose the lowest supported java level?

This would maybe also allow a quickfix to update the project settings in the IDE and CI builds will make the user aware but do not require to change the settings immediately.

iloveeclipse · 2024-07-25T06:41:51Z

Could it maybe just be made that ECJ emits a WARNING (or even ERROR) but do not abort compilation and simply choose the lowest supported java level?

In theory (with unlimited man power available) yes. Contributions are welcome.

laeubi · 2024-07-25T06:47:12Z

In theory (with unlimited man power available) yes. Contributions are welcome.

I just guessed from the error message

CompilerException: target level should be in '1.8','9'...'22' (or '8.0'..'22.0'): 1.6

that

we know the minimum (=1.8)
know the current value (=1.6)

and I didn't know that replacing an

if (x < y) {
 throw error
}

with

if (x < y) {
 log (error)
 x = y
}

requires "unlimited man power" but it seems the compiler code is more complex than I assumed here...

iloveeclipse · 2024-07-25T06:53:27Z

@laeubi: please if you really want some feature to be implemented, create a ticket and find someone who has either free time or money to work on whatever you want. I personally have neither interest, nor free time or money to work on the requested feature. Maybe someone else has, it is open source project.

laeubi · 2024-07-25T06:58:54Z

It was just a suggestion that seems easy to implement (for someone who is already deep inside the topic) to help users of ECJ/JDT in the transition phase especially as it already has shown to require additional efforts for platform itself.

But if it is not a priority of the project its fine, anyways it is exaggerated to claim every suggestion requires "unlimited man power" where it was actually meant as "I don't find it useful or plan to work on this".

jarthana · 2024-07-25T07:03:50Z

requires "unlimited man power" but it seems the compiler code is more complex than I assumed here...

It probably is. This particular issue is proving to be much more complex and time consuming that I imagined. Not that anyone thought it was going to be easy. So, I guess it is easier to get this to a decent stage and let us look at everything around a little later. You can already see Andrey has listed quite a few items for the next iteration.

Found during the review of eclipse-jdt#2551 - CompilerOptions.originalComplianceLevel is obsoleted and can be removed + code in TypeConverter using it should be cleaned up. - CompilerOptions.originalSourceLevel is obsoleted and can be removed + code in JDT that uses it should be cleaned up Fixes eclipse-jdt#2760

* Remove obsoleted code in CompilerOptions Found during the review of #2551 - CompilerOptions.originalComplianceLevel is obsoleted and can be removed + code in TypeConverter using it should be cleaned up. - CompilerOptions.originalSourceLevel is obsoleted and can be removed + code in JDT that uses it should be cleaned up Fixes #2760 * Additional commit to address test failures --------- Co-authored-by: Srikanth Sankaran <[email protected]>

srikanth-sankaran reviewed Jun 12, 2024

View reviewed changes

...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties Outdated Show resolved Hide resolved

srikanth-sankaran reviewed Jun 12, 2024

View reviewed changes

...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties Outdated Show resolved Hide resolved

iloveeclipse force-pushed the issue_2536 branch 4 times, most recently from 64d35b4 to 39897e9 Compare June 17, 2024 09:59

iloveeclipse mentioned this pull request Jun 17, 2024

jclMin22.jar contains classes compiled against Java 21 #2580

Closed

iloveeclipse force-pushed the issue_2536 branch 9 times, most recently from f68dc72 to e8bbe58 Compare June 21, 2024 15:46

iloveeclipse force-pushed the issue_2536 branch 3 times, most recently from bd36976 to 9b9de5e Compare June 24, 2024 10:03

iloveeclipse force-pushed the issue_2536 branch 2 times, most recently from 28819e2 to 5709a3b Compare June 24, 2024 17:12

iloveeclipse mentioned this pull request Jul 25, 2024

4.33 I-Build: I20240724-1800 - BUILD FAILED eclipse-platform/eclipse.platform.releng.aggregator#2197

Closed

HannesWell mentioned this pull request Jul 25, 2024

Move api.tools.annotations to Java-1.8 eclipse-pde/eclipse.pde#1349

Merged

iloveeclipse deleted the issue_2536 branch July 25, 2024 13:29

iloveeclipse mentioned this pull request Jul 25, 2024

4 new jdt.ui + jdt.ui.refactoring fails since I20240722-0320 eclipse-jdt/eclipse.jdt.ui#1526

Open

This was referenced Jul 25, 2024

Move API-Tools test resources to JavaSE-1.8 eclipse-pde/eclipse.pde#1350

Merged

[PDE-Build] Use 1.8 as default javac target/source-level in scripts eclipse-pde/eclipse.pde#1351

Merged

iloveeclipse mentioned this pull request Jul 26, 2024

BatchCompilerTest.test248_jar_ref_in_jar fails on Windows #2766

Closed

rgrunber mentioned this pull request Jul 31, 2024

Changelog for 1.33.0. redhat-developer/vscode-java#3728

Merged

stephan-herrmann mentioned this pull request Aug 2, 2024

5 jdt.ui and jdt.refactoring tests fail since I20240801-1800 eclipse-jdt/eclipse.jdt.ui#1565

Closed

iloveeclipse mentioned this pull request Sep 5, 2024

Remove obsoleted code in CompilerOptions #2924

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove compiler support for JLS source/target < 1.8 #2551

Remove compiler support for JLS source/target < 1.8 #2551

iloveeclipse commented Jun 11, 2024 •

edited

Loading

iloveeclipse commented Jun 11, 2024

srikanth-sankaran commented Jun 12, 2024

iloveeclipse commented Jun 12, 2024

jarthana commented Jun 12, 2024

iloveeclipse commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

stephan-herrmann commented Jun 21, 2024

iloveeclipse commented Jun 24, 2024

iloveeclipse commented Jun 24, 2024

iloveeclipse commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

jarthana commented Jul 25, 2024

laeubi commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

laeubi commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

laeubi commented Jul 25, 2024

jarthana commented Jul 25, 2024

Remove compiler support for JLS source/target < 1.8 #2551

Remove compiler support for JLS source/target < 1.8 #2551

Conversation

iloveeclipse commented Jun 11, 2024 • edited Loading

iloveeclipse commented Jun 11, 2024

srikanth-sankaran commented Jun 12, 2024

iloveeclipse commented Jun 12, 2024

jarthana commented Jun 12, 2024

iloveeclipse commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

stephan-herrmann commented Jun 21, 2024

iloveeclipse commented Jun 24, 2024

iloveeclipse commented Jun 24, 2024

iloveeclipse commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

jarthana commented Jul 25, 2024

laeubi commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

laeubi commented Jul 25, 2024

iloveeclipse commented Jul 25, 2024

laeubi commented Jul 25, 2024

jarthana commented Jul 25, 2024

iloveeclipse commented Jun 11, 2024 •

edited

Loading