Skip to content

Java 21: Pattern Matching + StringTemplate JEP-430 + Unnamed JEP-445#935

Open
npeters wants to merge 1 commit into
palantir:developfrom
npeters:feature/java-21
Open

Java 21: Pattern Matching + StringTemplate JEP-430 + Unnamed JEP-445#935
npeters wants to merge 1 commit into
palantir:developfrom
npeters:feature/java-21

Conversation

@npeters

@npeters npeters commented Oct 3, 2023

Copy link
Copy Markdown

Before this PR

May error on parsing Java 21 file.

After this PR

Support of different JEP of Java 21 and preview

  • JEP-430 StringTemplates
  • JEP-440 Record Patterns
  • JEP-441 Pattern Matching switch (with guard)
  • JEP-443 Unamed Pattern

Possible downsides?

Build

  • I added a independent gradle module to build all Java 21 code but the gradle do not support to build groovy code with java 21: it should be first compile with java 17 and you can compile the project with java 21 😞
  • I added a hack on palantir-java-format gradle script to copy all the java 21 class file on the jar. 😞
  • Need to add --enable-preview with Java 21.

Code

  • "StringTemplates" was really hard to support: I did some change on JavaInput.java and visitStringTemplate. The complexity of this part is really to high.
  • I copy and past "visitCase" from Java14 file and the guard param.

@palantirtech

Copy link
Copy Markdown
Member

Thanks for your interest in palantir/palantir-java-format, @npeters! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@changelog-app

changelog-app Bot commented Oct 3, 2023

Copy link
Copy Markdown

Generate changelog in changelog/@unreleased

What do the change types mean?
  • feature: A new feature of the service.
  • improvement: An incremental improvement in the functionality or operation of the service.
  • fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
  • break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
    and external consumers of the service's API (e.g. customer-written software or integrations).
  • deprecation: Advertises the intention to remove service functionality without any change to the
    operation of the service itself.
  • manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
    performing database surgery, ...) at the time of upgrade for it to succeed.
  • migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?
  • ❗The break and manual task changelog types will result in a major release!
  • 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
  • ✨ All others will result in a minor version release.

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Java 21: Pattern Matching + StringTemplate JEP-430 + Unnamed JEP-445

Check the box to generate changelog(s)

  • Generate changelog entry

@npeters

npeters commented Oct 3, 2023

Copy link
Copy Markdown
Author

The build is so painful... I push all the jar / idea-plugin https://github.com/npeters/palantir-java-format/releases/tag/java21-draft.

@npeters

npeters commented Oct 3, 2023

Copy link
Copy Markdown
Author

link to #934 #933 #931

@kittech0

Copy link
Copy Markdown

great job, now gotta wait for approval

@koppor koppor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to use palantir-java-format and saw this PR. I read through and commented on code changes appearing unusual to me. Feel free to use or to ignore - I am not affiliated with the project, just some Java project lead.

Comment thread build.gradle
Comment on lines -48 to -51
sourceCompatibility = 11
targetCompatibility = 11

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep as is - and remove the "duplicate" settings in the sub projects.

Comment on lines +11 to +12
java{
sourceCompatibility = 11
targetCompatibility = 11
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed (because configured globally)

Comment thread idea-plugin/build.gradle Outdated
Comment on lines +54 to +55
java{
sourceCompatibility = 11
targetCompatibility = 11
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed (because configured globally)

Comment on lines +5 to +9
java{
sourceCompatibility = 11
targetCompatibility = 11
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed (because configured globally)

Comment thread palantir-java-format-spi/build.gradle Outdated
java{
sourceCompatibility = 11
targetCompatibility = 11
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed (because configured globally)

if (stringFragmentKind) {
stringFragmentEndPos = t.endPos();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove empty line


/** Skips over extra semi-colons at the top-level, or in a class member declaration lists. */
protected void dropEmptyDeclarations() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove empty line

@@ -1,4 +1,4 @@
import a.A;;
import a.A;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the test input changed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

";;" is not valid on java 21.
"Disallow Extra Semicolons Between "import" Statements"
https://www.oracle.com/java/technologies/javase/21-relnote-issues.html#JDK-8027682

I was not able to disable the test only for Java21

@@ -1,4 +1,4 @@
package foo;;
package foo;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the test input changed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

";;" is not valid on java 21.
"Disallow Extra Semicolons Between "import" Statements"
https://www.oracle.com/java/technologies/javase/21-relnote-issues.html#JDK-8027682

I was not able to disable the test only for Java21

Void r = scan(node.getDeconstructor(), unused);
token("(");

// r = scanAndReduceVoid(, unused, r);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this comment mean?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove it

@talios

talios commented Nov 5, 2023

Copy link
Copy Markdown

Has there been any progress on this PR? Would love to see a new release supporting Java 21.

@CRogers

CRogers commented Nov 17, 2023

Copy link
Copy Markdown
Contributor

Thanks for submitting this PR - although we're unlikely to be able review it/actually support Java 21 in palantir-java-format by the end of the year. There's a significant amount of other work we need to do on our internal repos (currently ongoing) before we can start using Java 21 source features and these take priority. Unfortunately, we are quite resource constrained at the moment.

Until then, you may be able to use something like jitpack (https://jitpack.io/) if it is allowed by your organisation to use this commit hash until we are in a place to review/merging/supporting it.

@koppor

koppor commented Nov 20, 2023

Copy link
Copy Markdown
Contributor

The build is so painful... I push all the jar / idea-plugin npeters/palantir-java-format@java21-draft (release).

Is there also a chance for the gradle plugin? I also hit some issues and would like to test if the modification works with https://github.com/JabRef/jabref/blob/39569344d4051ac958e4fc5edda89866d0706498/src/main/java/org/jabref/gui/fieldeditors/identifier/IdentifierEditor.java#L63. (which is issue #952)



switch (something){
case Integer i when i > 10-> System.out.println(i.toString());

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the #952 should be fix and the test is here

@npeters

npeters commented Nov 20, 2023

Copy link
Copy Markdown
Author

yes, the PR should manage the #952. It is part of 'JEP-441 Pattern Matching switch (with guard)' .
There are already a test on the guard: sample is I961.

@wb459

wb459 commented Feb 21, 2024

Copy link
Copy Markdown

@CRogers Is there any news regarding when the team will have the capacity to review this PR?

@bmarwell

bmarwell commented Mar 3, 2024

Copy link
Copy Markdown

Thanks for submitting this PR - although we're unlikely to be able review it/actually support Java 21 in palantir-java-format by the end of the year.

End of WHICH year? 😄
Jokes aside – can you nominate more maintainers? A few ASF projects like maven use palantir and we would like to have property formatted source code. Maybe one of them could get access to palantir PRs and releases?

@talios

talios commented Apr 7, 2024

Copy link
Copy Markdown

In relation to this - StringTemplates as we currently no them are 100% dead and will not be in JDK 23, even under --enable-preview: https://mail.openjdk.org/pipermail/amber-spec-experts/2024-April/004106.html

@anbonifacio

Copy link
Copy Markdown

Maybe a smaller scope would be easier to review? What about limiting the support to the new pattern matching features like case null, defaut ->?

@koppor

koppor commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

As far as I understand the whole setting, the whole endeavor is blocked by the release of version 2.0 of the IntelliJ Gradle Plugin, because of the fix JetBrains/intellij-platform-gradle-plugin#1494 is needed.

(Source: #997)

@npeters

npeters commented Apr 27, 2024

Copy link
Copy Markdown
Author

@koppor How can I assist you?
This PR seems too complex to merge and I understand.
Perhaps a better approach would be as follows:

  1. Create one pull request with a Gradle configuration build that includes multi-compilation tasks and multi-test task, each targeting a specific JDK version.
  2. Submit another pull request containing the Java code and associated tests

@koppor If you agree with this plan. I can try to do it.

@npeters

npeters commented Apr 27, 2024

Copy link
Copy Markdown
Author

@talios thank you for the point. We need to create a InputAstVisitor class for each java version.

@xhyrom

xhyrom commented May 12, 2024

Copy link
Copy Markdown

How it's looking?

@RealYusufIsmail

Copy link
Copy Markdown

Yo, any updates in regard to the following changes.

@siepkes

siepkes commented Oct 18, 2024

Copy link
Copy Markdown

Since String templates was removed from JDK 23 (see: Update on String Templates (JEP 459) it should probably be removed from this PR?

@npeters

npeters commented Nov 2, 2024

Copy link
Copy Markdown
Author

Currently I am using this PR on my project and it works fine: so the java code work correctly.
I think the build is the problem: the second project and the hack to copy the class to jar are really bad.

Gradle has been updated and it is possible to support Java 21.

I completely rewrite the system:

  1. I split the code for java 21 and java 21 preview,
  2. I create sourceset for java21 and java21preview
  3. the jar is now correctly built.

@npeters

npeters commented Jan 8, 2025

Copy link
Copy Markdown
Author

Hi @koppor.
can you please have a look on PR.

@koppor

koppor commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

@npeters I am not the maintianer of this project. It seems that there is some internal development going oin (according to #952 (comment)).

@koppor

koppor commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Related PR: #1211

@heruan

heruan commented Sep 2, 2025

Copy link
Copy Markdown

Is this PR still active? Any plans to merge it? It's been a while tools like SonarQube suggest using unnamed variables like _, but Palantir Java Format fails on those.

@bmarwell

bmarwell commented Sep 2, 2025

Copy link
Copy Markdown

I created jdtfmt, since this project seems not much alive. I contacted the authors without luck.

jdtfmt is based on the eclipse formatter, so it works the same on the command line. You can also use the configuration with spotless + eclipse jdt.

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.