Java 17 instanceof pattern matching for modules #82341

astefan · 2022-01-07T17:08:24Z

Switch to Java 17 instanceof pattern matching for folders build-conventions through modules.

…ntions through modules

elasticmachine · 2022-01-07T23:02:32Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

henningandersen

LGTM, except a few smaller things, no need for another round.

Thanks for improving this!

henningandersen · 2022-01-08T09:10:37Z

build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

@@ -1209,8 +1209,7 @@ private void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path>
                }
            });
        } catch (UncheckedIOException e) {
-            if (e.getCause() instanceof NoSuchFileException) {
-                NoSuchFileException cause = (NoSuchFileException) e.getCause();
+            if (e.getCause()instanceof NoSuchFileException cause) {


nit: missing space

Suggested change

if (e.getCause()instanceof NoSuchFileException cause) {

if (e.getCause() instanceof NoSuchFileException cause) {

@henningandersen unfortunately, that whitespace missing there is the result of spotless, because of a bug. https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
Even if it was fixed on the Eclipse side, we need to wait for Spotless to upgrade. I think diffplug/spotless#1046 this is the issue we are waiting for to be fixed.

henningandersen · 2022-01-08T09:12:31Z

libs/x-content/src/main/java/org/elasticsearch/xcontent/support/MapXContentParser.java

@@ -58,8 +58,8 @@ public MapXContentParser(

    @Override
    protected boolean doBooleanValue() throws IOException {
-        if (iterator != null && iterator.currentValue() instanceof Boolean) {
-            return (Boolean) iterator.currentValue();
+        if (iterator != null && iterator.currentValue()instanceof Boolean aBoolean) {


missing space

Suggested change

if (iterator != null && iterator.currentValue()instanceof Boolean aBoolean) {

if (iterator != null && iterator.currentValue() instanceof Boolean aBoolean) {

henningandersen · 2022-01-08T09:12:58Z

libs/x-content/src/main/java/org/elasticsearch/xcontent/support/MapXContentParser.java

@@ -216,8 +216,8 @@ public NumberType numberType() throws IOException {

    @Override
    public byte[] binaryValue() throws IOException {
-        if (iterator != null && iterator.currentValue() instanceof byte[]) {
-            return (byte[]) iterator.currentValue();
+        if (iterator != null && iterator.currentValue()instanceof byte[] bytes) {


missing space:

Suggested change

if (iterator != null && iterator.currentValue()instanceof byte[] bytes) {

if (iterator != null && iterator.currentValue() instanceof byte[] bytes) {

henningandersen · 2022-01-08T09:27:01Z

.../src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java

@@ -621,22 +621,19 @@ public void visitStringConcatenation(StringConcatenationNode irStringConcatenati
            int j = i;
            irRightNode.visit(this, (e) -> irStringConcatenationNode.getArgumentNodes().set(j + 1, e));

-            if (irLeftNode instanceof ConstantNode && irRightNode instanceof ConstantNode) {
-                ConstantNode irConstantNode = (ConstantNode) irLeftNode;
+            if (irLeftNode instanceof ConstantNode irConstantNode && irRightNode instanceof ConstantNode) {


This seems slightly inconsistently used here, in that the cast to ConstantNode seems unnecessary. I would prefer to either cast/pattern match both left and right nodes or none of them.

Yeah, I didn't particularly like this block of code changes either. Seems too compact and the code too similar between the conditional branches to make it easy to read. I've reverted the changes. Even if the instanceof pattern matching is not used, the readability of the code is slightly better this way.

henningandersen · 2022-01-08T09:28:45Z

.../src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java

                irConstantNode.attachDecoration(
                    new IRDConstant(
                        "" + irConstantNode.getDecorationValue(IRDConstant.class) + irRightNode.getDecorationValue(IRDConstant.class)
                    )
                );
                irConstantNode.attachDecoration(new IRDExpressionType(String.class));
                irStringConcatenationNode.getArgumentNodes().remove(i + 1);
-            } else if (irLeftNode instanceof NullNode && irRightNode instanceof ConstantNode) {
-                ConstantNode irConstantNode = (ConstantNode) irRightNode;
+            } else if (irLeftNode instanceof NullNode && irRightNode instanceof ConstantNode irConstantNode) {


Same as above, seems simpler to just use irRightNode in the block? Also easier to read.

henningandersen · 2022-01-08T09:29:00Z

.../src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java

                irConstantNode.attachDecoration(new IRDConstant("" + null + irRightNode.getDecorationValue(IRDConstant.class)));
                irConstantNode.attachDecoration(new IRDExpressionType(String.class));
                irStringConcatenationNode.getArgumentNodes().remove(i);
-            } else if (irLeftNode instanceof ConstantNode && irRightNode instanceof NullNode) {
-                ConstantNode irConstantNode = (ConstantNode) irLeftNode;
+            } else if (irLeftNode instanceof ConstantNode irConstantNode && irRightNode instanceof NullNode) {


Same as above.

elasticsearchmachine · 2022-01-10T08:44:07Z

💚 Backport successful

Status	Branch	Result
✅	8.0

Switch to Java 17 instanceof pattern matching for folders build-conventions through modules

Switch to Java 17 instanceof pattern matching for folders build-conve…

0767c89

…ntions through modules

elasticsearchmachine added the v8.1.0 label Jan 7, 2022

astefan added v8.0.0 >non-issue >refactoring labels Jan 7, 2022

Spotless

1cc6ba0

astefan added the :Core/Infra/Core Core issues without another label label Jan 7, 2022

astefan requested a review from henningandersen January 7, 2022 23:02

astefan marked this pull request as ready for review January 7, 2022 23:02

elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 7, 2022

astefan mentioned this pull request Jan 7, 2022

Java 17 features in Elasticsearch code base #81820

Open

19 tasks

astefan added the auto-backport-and-merge label Jan 7, 2022

henningandersen approved these changes Jan 8, 2022

View reviewed changes

Address reviews

c11c3ee

astefan merged commit 35a79bc into elastic:master Jan 10, 2022

astefan deleted the java17/instanceof/modules branch January 10, 2022 08:43

astefan mentioned this pull request Jan 10, 2022

[8.0] Java 17 instanceof pattern matching for modules (#82341) #82347

Merged

astefan added a commit to astefan/elasticsearch that referenced this pull request Jan 10, 2022

Java 17 instanceof pattern matching for modules (elastic#82341)

dab8600

Switch to Java 17 instanceof pattern matching for folders build-conventions through modules

elasticsearchmachine pushed a commit that referenced this pull request Jan 10, 2022

Java 17 instanceof pattern matching for modules (#82341) (#82347)

ed1448b

Switch to Java 17 instanceof pattern matching for folders build-conventions through modules

pugnascotia added v8.0.0-rc2 and removed v8.0.0 labels Feb 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java 17 instanceof pattern matching for modules #82341

Java 17 instanceof pattern matching for modules #82341

astefan commented Jan 7, 2022

elasticmachine commented Jan 7, 2022

henningandersen left a comment

henningandersen Jan 8, 2022

astefan Jan 10, 2022

henningandersen Jan 8, 2022

henningandersen Jan 8, 2022

henningandersen Jan 8, 2022

astefan Jan 10, 2022

henningandersen Jan 8, 2022

henningandersen Jan 8, 2022

elasticsearchmachine commented Jan 10, 2022

	if (e.getCause()instanceof NoSuchFileException cause) {
	if (e.getCause() instanceof NoSuchFileException cause) {

	if (iterator != null && iterator.currentValue()instanceof Boolean aBoolean) {
	if (iterator != null && iterator.currentValue() instanceof Boolean aBoolean) {

Java 17 instanceof pattern matching for modules #82341

Java 17 instanceof pattern matching for modules #82341

Conversation

astefan commented Jan 7, 2022

elasticmachine commented Jan 7, 2022

henningandersen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticsearchmachine commented Jan 10, 2022

💚 Backport successful