-
Notifications
You must be signed in to change notification settings - Fork 137
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
Remove compiler support for JLS source/target < 1.8 #2551
Conversation
@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
Outdated
Show resolved
Hide resolved
...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties
Outdated
Show resolved
Hide resolved
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 |
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. |
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. |
+1 IMHO we will be serving no one by leaving in half baked unmaintained, half tested support for ancient versions. |
64d35b4
to
39897e9
Compare
f68dc72
to
e8bbe58
Compare
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 |
bd36976
to
9b9de5e
Compare
@laeubi : could it be, that this block is executed in a random order, or order not related to the definition order in the eclipse.jdt.core/org.eclipse.jdt.core.tests.model/pom.xml Lines 36 to 40 in 92c4a1d
I assume that the |
28819e2
to
5709a3b
Compare
OK, not running Still, |
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 |
What I'm wondering: 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. |
In theory (with unlimited man power available) yes. Contributions are welcome. |
I just guessed from the error message
that
and I didn't know that replacing an
with
requires "unlimited man power" but it seems the compiler code is more complex than I assumed here... |
@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. |
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". |
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
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]>
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