First batch of review feedback

openrewrite · Jan 21, 2025 · 7a5dafc · 7a5dafc
1 parent f7de8fd
commit 7a5dafc
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 23 deletions.
diff --git a/...te-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java b/...te-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java
@@ -791,7 +791,7 @@ public J visitDeconstructionPattern(DeconstructionPatternTree node, Space fmt) {
         return new J.DeconstructionPattern(randomId(),
                 fmt,
                 Markers.EMPTY,
-                convert(node.getDeconstructor(), t -> sourceBefore("(")),
+                convert(node.getDeconstructor()),
                 JContainer.build(convertAll(node.getNestedPatterns(), commaDelim, t -> sourceBefore(")"))),
                 type);
     }

diff --git a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java
@@ -291,8 +291,8 @@ private JavaType.FullyQualified classType(Type.ClassType classType, String signa
             if (sym.members_field != null) {
                 for (Symbol elem : sym.members_field.getSymbols()) {
                     if (elem instanceof Symbol.VarSymbol &&
-                            (elem.flags_field & (Flags.SYNTHETIC | Flags.BRIDGE | Flags.HYPOTHETICAL |
-                                    Flags.GENERATEDCONSTR | Flags.ANONCONSTR)) == 0) {
+                        (elem.flags_field & (Flags.SYNTHETIC | Flags.BRIDGE | Flags.HYPOTHETICAL |
+                                             Flags.GENERATEDCONSTR | Flags.ANONCONSTR)) == 0) {
                         if (fqn.equals("java.lang.String") && elem.name.toString().equals("serialPersistentFields")) {
                             // there is a "serialPersistentFields" member within the String class which is used in normal Java
                             // serialization to customize how the String field is serialized. This field is tripping up Jackson
@@ -305,7 +305,7 @@ private JavaType.FullyQualified classType(Type.ClassType classType, String signa
                         }
                         fields.add(variableType(elem, clazz));
                     } else if (elem instanceof Symbol.MethodSymbol &&
-                            (elem.flags_field & (Flags.SYNTHETIC | Flags.BRIDGE | Flags.HYPOTHETICAL | Flags.ANONCONSTR)) == 0) {
+                               (elem.flags_field & (Flags.SYNTHETIC | Flags.BRIDGE | Flags.HYPOTHETICAL | Flags.ANONCONSTR)) == 0) {
                         if (methods == null) {
                             methods = new ArrayList<>();
                         }
@@ -426,7 +426,7 @@ public JavaType.Primitive primitive(TypeTag tag) {
     }
 
     private JavaType.@Nullable Variable variableType(@Nullable Symbol symbol,
-            JavaType.@Nullable FullyQualified owner) {
+                                                     JavaType.@Nullable FullyQualified owner) {
         if (!(symbol instanceof Symbol.VarSymbol)) {
             return null;
         }

diff --git a/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/RecordPatternMatchingTest.java b/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/RecordPatternMatchingTest.java
@@ -17,9 +17,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.openrewrite.java.MinimumJava21;
-import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
-import org.openrewrite.test.TypeValidation;
 
 import static org.openrewrite.java.Assertions.java;
 

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaPrinter.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaPrinter.java
@@ -735,8 +735,8 @@ public J visitInstanceOf(InstanceOf instanceOf, PrintOutputCapture<P> p) {
     @Override
     public J visitDeconstructionPattern(DeconstructionPattern deconstructionPattern, PrintOutputCapture<P> p) {
         beforeSyntax(deconstructionPattern, Space.Location.DECONSTRUCTOR_PREFIX, p);
-        visitRightPadded(deconstructionPattern.getDeconstructor(), JRightPadded.Location.DECONSTRUCTOR, p);
-        visitContainer("(", deconstructionPattern.getNested(), JContainer.Location.DECONSTRUCTOR, ",", ")", p);
+        visitAndCast(deconstructionPattern.getDeconstructor(), p);
+        visitContainer("(", deconstructionPattern.getPadding().getNested(), JContainer.Location.DECONSTRUCTOR, ",", ")", p);
         afterSyntax(deconstructionPattern, p);
         return deconstructionPattern;
     }

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java
@@ -749,8 +749,8 @@ public J visitDeconstructionPattern(J.DeconstructionPattern deconstructionPatter
         J.DeconstructionPattern d = deconstructionPattern;
         d = d.withPrefix(visitSpace(d.getPrefix(), Space.Location.DECONSTRUCTOR_PREFIX, p));
         d = d.withMarkers(visitMarkers(d.getMarkers(), p));
-        d = d.withDeconstructor(visitRightPadded(d.getDeconstructor(), JRightPadded.Location.DECONSTRUCTOR, p));
-        d = d.withNested(visitContainer(d.getNested(), JContainer.Location.DECONSTRUCTOR, p));
+        d = d.withDeconstructor(visitAndCast(d.getDeconstructor(), p));
+        d = d.getPadding().withNested(visitContainer(d.getPadding().getNested(), JContainer.Location.DECONSTRUCTOR, p));
         d = d.withType(visitType(d.getType(), p));
         return d;
 

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java b/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java
@@ -816,6 +816,25 @@ public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, J j) {
             return instanceOf;
         }
 
+        @Override
+        public J.DeconstructionPattern visitDeconstructionPattern(J.DeconstructionPattern deconstructionPattern, J j) {
+            if (isEqual.get()) {
+                if (!(j instanceof J.DeconstructionPattern)) {
+                    isEqual.set(false);
+                    return deconstructionPattern;
+                }
+
+                J.DeconstructionPattern compareTo = (J.DeconstructionPattern) j;
+                if (!TypeUtils.isOfType(deconstructionPattern.getType(), compareTo.getType())) {
+                    isEqual.set(false);
+                    return deconstructionPattern;
+                }
+                visit(deconstructionPattern.getDeconstructor(), compareTo.getDeconstructor());
+                this.visitList(deconstructionPattern.getNested(), compareTo.getNested());
+            }
+            return deconstructionPattern;
+        }
+
         @Override
         public J.Label visitLabel(J.Label label, J j) {
             if (isEqual.get()) {

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java
@@ -3073,12 +3073,18 @@ final class DeconstructionPattern implements J, Pattern, TypedTree {
 
         @With
         @Getter
-        JRightPadded<Expression> deconstructor;
+        Expression deconstructor;
 
-        @With
-        @Getter
         JContainer<Pattern> nested;
 
+        public List<Pattern> getNested() {
+            return nested.getElements();
+        }
+
+        public DeconstructionPattern withNested(List<Pattern> nested) {
+            return getPadding().withNested(JContainer.withElements(this.nested, nested));
+        }
+
         @Getter
         @With
         JavaType type;
@@ -3098,18 +3104,25 @@ public CoordinateBuilder.DeconstructionPattern getCoordinates() {
             return new CoordinateBuilder.DeconstructionPattern(this);
         }
 
+        public Padding getPadding() {
+            Padding p;
+            if (this.padding == null) {
+                p = new Padding(this);
+                this.padding = new WeakReference<>(p);
+            } else {
+                p = this.padding.get();
+                if (p == null || p.t != this) {
+                    p = new Padding(this);
+                    this.padding = new WeakReference<>(p);
+                }
+            }
+            return p;
+        }
+
         @RequiredArgsConstructor
         public static class Padding {
             private final DeconstructionPattern t;
 
-            public JRightPadded<Expression> getDeconstructor() {
-                return t.deconstructor;
-            }
-
-            public DeconstructionPattern withDeconstructor(JRightPadded<Expression> deconstructor) {
-                return t.deconstructor == deconstructor ? t : new DeconstructionPattern(t.id, t.prefix, t.markers, deconstructor, t.nested, t.type);
-            }
-
             public JContainer<Pattern> getNested() {
                 return t.nested;
             }

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java
@@ -234,7 +234,7 @@ public static Space format(String formatting, int beginIndex, int toIndex) {
 
     @SuppressWarnings("ConstantConditions")
     public static <J2 extends J> @Nullable List<JRightPadded<J2>> formatLastSuffix(@Nullable List<JRightPadded<J2>> trees,
-                                                                         Space suffix) {
+                                                                                   Space suffix) {
         if (trees == null) {
             return null;
         }