diff --git a/src/main/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilder.java b/src/main/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilder.java index 84ed040..04566cc 100644 --- a/src/main/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilder.java +++ b/src/main/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilder.java @@ -184,6 +184,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { if (visited != block) { doAfterVisit(new UpdateSerializationInclusionConfiguration().getVisitor()); doAfterVisit(new UpdateAutoDetectVisibilityConfiguration().getVisitor()); + doAfterVisit(coalesceRebuildAssignments()); } return visited; } @@ -276,11 +277,14 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { } } - if (!(mi.getSelect() instanceof J.Identifier)) { + Expression select = mi.getSelect(); + if (!(select instanceof J.Identifier) && + !(select instanceof J.FieldAccess && + ((J.FieldAccess) select).getTarget() instanceof J.Identifier)) { return mi; } - String matchedMapper = matchingMapperType(mi.getSelect().getType()); + String matchedMapper = matchingMapperType(select.getType()); if (matchedMapper == null) { return mi; } @@ -290,6 +294,13 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return mi; } + if (isTopLevelStatement(getCursor()) && isReassignableReceiver(select, getCursor())) { + J rebuilt = rewriteAsRebuildAssignment(mi, select, matchedMapper, mapping, ctx); + if (rebuilt != null) { + return rebuilt; + } + } + // Not eligible for builder migration - add a TODO comment String simpleMapperName = matchedMapper.substring(matchedMapper.lastIndexOf('.') + 1); String commentText = String.format( @@ -696,10 +707,210 @@ private J applyBuilderTemplate(String mapperFqn, List setter } return chained; } + + private @Nullable J rewriteAsRebuildAssignment(J.MethodInvocation mi, Expression select, + String mapperFqn, SetterToBuilderMapping mapping, + ExecutionContext ctx) { + String builderName = mapping.builderName; + if (mapping == SetterToBuilderMapping.SET_DEFAULT_PROPERTY_INCLUSION && + mi.getArguments().size() == 1 && + !(mi.getArguments().get(0) instanceof J.Empty) && + TypeUtils.isAssignableTo("com.fasterxml.jackson.annotation.JsonInclude$Include", + mi.getArguments().get(0).getType())) { + builderName = "serializationInclusion"; + } + StringBuilder templateCode = new StringBuilder(); + templateCode.append("#{any(").append(mapperFqn).append(")} = #{any(") + .append(mapperFqn).append(")}.rebuild()\n.").append(builderName).append("("); + List templateArgs = new ArrayList<>(); + templateArgs.add(select); + templateArgs.add(select); + boolean first = true; + for (Expression arg : mi.getArguments()) { + if (arg instanceof J.Empty) { + continue; + } + if (!first) { + templateCode.append(", "); + } + first = false; + templateCode.append("#{any()}"); + templateArgs.add(arg); + } + templateCode.append(")\n.build()"); + + maybeAddImport(mapperFqn); + maybeAddImport(JSON_INCLUDE); + + JavaParser.Builder parser = JavaParser.fromJavaVersion() + .classpathFromResources(ctx, "jackson-annotations-2", "jackson-core-2", "jackson-databind-2") + .dependsOn(mapperStub(mapperFqn, emptyList())); + + return JavaTemplate.builder(templateCode.toString()) + .imports(mapperFqn, JSON_INCLUDE) + .javaParser(parser) + .build() + .apply(getCursor(), mi.getCoordinates().replace(), templateArgs.toArray()); + } } ); } + private static boolean isTopLevelStatement(Cursor cursor) { + return cursor.getParentTreeCursor().getValue() instanceof J.Block; + } + + private static boolean isReassignableReceiver(Expression select, Cursor cursor) { + if (select instanceof J.Identifier) { + return isReassignableLocal((J.Identifier) select, cursor); + } + if (select instanceof J.FieldAccess) { + J.FieldAccess fa = (J.FieldAccess) select; + if (!(fa.getTarget() instanceof J.Identifier)) { + return false; + } + JavaType.Variable v = fa.getName().getFieldType(); + return v != null && !v.hasFlags(Flag.Final); + } + return false; + } + + private static boolean isReassignableLocal(J.Identifier ident, Cursor cursor) { + JavaType.Variable fieldType = ident.getFieldType(); + if (fieldType != null && fieldType.hasFlags(Flag.Final)) { + return false; + } + String name = ident.getSimpleName(); + Cursor c = cursor; + while (c != null) { + Object v = c.getValue(); + if (v instanceof J.Block) { + for (Statement stmt : ((J.Block) v).getStatements()) { + J.VariableDeclarations vd = extractVariableDeclarations(stmt); + if (vd == null) { + continue; + } + for (J.VariableDeclarations.NamedVariable nv : vd.getVariables()) { + if (name.equals(nv.getName().getSimpleName()) && + TypeUtils.isOfType(nv.getName().getType(), ident.getType())) { + return !vd.hasModifier(J.Modifier.Type.Final); + } + } + } + } + if (v instanceof J.MethodDeclaration) { + for (Statement p : ((J.MethodDeclaration) v).getParameters()) { + if (p instanceof J.VariableDeclarations) { + for (J.VariableDeclarations.NamedVariable nv : ((J.VariableDeclarations) p).getVariables()) { + if (name.equals(nv.getName().getSimpleName())) { + return false; + } + } + } + } + return false; + } + c = c.getParent(); + } + return false; + } + + private static JavaIsoVisitor coalesceRebuildAssignments() { + return new JavaIsoVisitor() { + @Override + public J.Block visitBlock(J.Block block, ExecutionContext ctx) { + J.Block b = super.visitBlock(block, ctx); + List stmts = b.getStatements(); + if (stmts.size() < 2) { + return b; + } + List merged = new ArrayList<>(stmts.size()); + boolean changed = false; + int i = 0; + while (i < stmts.size()) { + RebuildParts head = tryParseRebuildAssignment(stmts.get(i)); + if (head == null) { + merged.add(stmts.get(i)); + i++; + continue; + } + List allChainCalls = new ArrayList<>(head.chainCalls); + int j = i + 1; + while (j < stmts.size()) { + RebuildParts next = tryParseRebuildAssignment(stmts.get(j)); + if (next == null || !SemanticallyEqual.areEqual(head.lhs, next.lhs)) { + break; + } + allChainCalls.addAll(next.chainCalls); + j++; + } + if (j == i + 1) { + merged.add(stmts.get(i)); + i++; + continue; + } + Statement coalesced = rebuildCoalescedAssignment(head, allChainCalls); + merged.add(coalesced); + changed = true; + i = j; + } + return changed ? b.withStatements(merged) : b; + } + }; + } + + private static class RebuildParts { + final J.Assignment assignment; + final Expression lhs; + final J.MethodInvocation buildCall; + final J.MethodInvocation rebuildCall; + final List chainCalls; + + RebuildParts(J.Assignment assignment, Expression lhs, J.MethodInvocation buildCall, + J.MethodInvocation rebuildCall, List chainCalls) { + this.assignment = assignment; + this.lhs = lhs; + this.buildCall = buildCall; + this.rebuildCall = rebuildCall; + this.chainCalls = chainCalls; + } + } + + private static @Nullable RebuildParts tryParseRebuildAssignment(Statement stmt) { + if (!(stmt instanceof J.Assignment)) { + return null; + } + J.Assignment assignment = (J.Assignment) stmt; + if (!(assignment.getAssignment() instanceof J.MethodInvocation)) { + return null; + } + J.MethodInvocation buildCall = (J.MethodInvocation) assignment.getAssignment(); + if (!"build".equals(buildCall.getName().getSimpleName())) { + return null; + } + List chainCalls = new ArrayList<>(); + Expression current = buildCall.getSelect(); + while (current instanceof J.MethodInvocation) { + J.MethodInvocation mi = (J.MethodInvocation) current; + if ("rebuild".equals(mi.getName().getSimpleName())) { + Collections.reverse(chainCalls); + return new RebuildParts(assignment, assignment.getVariable(), buildCall, mi, chainCalls); + } + chainCalls.add(mi); + current = mi.getSelect(); + } + return null; + } + + private static Statement rebuildCoalescedAssignment(RebuildParts head, List allChainCalls) { + Expression newSelect = head.rebuildCall; + for (J.MethodInvocation chainCall : allChainCalls) { + newSelect = chainCall.withSelect(newSelect); + } + J.MethodInvocation newBuild = head.buildCall.withSelect(newSelect); + return head.assignment.withAssignment(newBuild); + } + /** * Returns the FQN of the mapper type matched by the given new class, or null if none match. */ @@ -822,6 +1033,7 @@ private static String mapperStub(String mapperFqn, List unkn .append(" extends com.fasterxml.jackson.databind.ObjectMapper {\n"); sb.append(" public ").append(simpleName).append("() {}\n"); sb.append(" public static Builder builder() { return null; }\n"); + sb.append(" public Builder rebuild() { return null; }\n"); sb.append(" public static class Builder extends ") .append("com.fasterxml.jackson.databind.cfg.MapperBuilder<") .append(simpleName).append(", Builder> {\n"); diff --git a/src/test/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilderTest.java b/src/test/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilderTest.java index 1aefb0b..dd9d90f 100644 --- a/src/test/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilderTest.java +++ b/src/test/java/org/openrewrite/java/jackson/MigrateMapperSettersToBuilderTest.java @@ -682,6 +682,203 @@ JsonMapper create() { } } + @Nested + class RewriteAsRebuild { + + @Test + void trailingSetterAfterGapOnLocal() { + rewriteRun( + java( + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + JsonMapper create() { + JsonMapper mapper = new JsonMapper(); + mapper.disable(SerializationFeature.INDENT_OUTPUT); + System.out.println(mapper); + mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); + return mapper; + } + } + """, + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + JsonMapper create() { + JsonMapper mapper = JsonMapper.builder() + .disable(SerializationFeature.INDENT_OUTPUT) + .build(); + System.out.println(mapper); + mapper = mapper.rebuild() + .enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + return mapper; + } + } + """ + ) + ); + } + + @Test + void trailingSettersCoalesceIntoOneRebuildChain() { + rewriteRun( + java( + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + JsonMapper create() { + JsonMapper mapper = new JsonMapper(); + mapper.disable(SerializationFeature.INDENT_OUTPUT); + System.out.println(mapper); + mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); + mapper.disable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES); + return mapper; + } + } + """, + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + JsonMapper create() { + JsonMapper mapper = JsonMapper.builder() + .disable(SerializationFeature.INDENT_OUTPUT) + .build(); + System.out.println(mapper); + mapper = mapper.rebuild() + .enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .disable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES) + .build(); + return mapper; + } + } + """ + ) + ); + } + + @Test + void thisFieldSetterCoalescesIntoRebuild() { + rewriteRun( + java( + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + private JsonMapper mapper = new JsonMapper(); + + void configure() { + this.mapper.disable(SerializationFeature.INDENT_OUTPUT); + this.mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); + } + } + """, + """ + import com.fasterxml.jackson.databind.DeserializationFeature; + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + private JsonMapper mapper = new JsonMapper(); + + void configure() { + this.mapper = this.mapper.rebuild() + .disable(SerializationFeature.INDENT_OUTPUT) + .enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + } + } + """ + ) + ); + } + + @Test + void qualifiedFieldSetterRewritten() { + rewriteRun( + java( + """ + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + static class Holder { + JsonMapper mapper = new JsonMapper(); + } + + void configure(Holder holder) { + holder.mapper.disable(SerializationFeature.INDENT_OUTPUT); + } + } + """, + """ + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + static class Holder { + JsonMapper mapper = new JsonMapper(); + } + + void configure(Holder holder) { + holder.mapper = holder.mapper.rebuild() + .disable(SerializationFeature.INDENT_OUTPUT) + .build(); + } + } + """ + ) + ); + } + + @Test + void finalFieldStillGetsTodoComment() { + rewriteRun( + java( + """ + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + private final JsonMapper mapper = new JsonMapper(); + + void configure() { + this.mapper.disable(SerializationFeature.INDENT_OUTPUT); + } + } + """, + """ + import com.fasterxml.jackson.databind.SerializationFeature; + import com.fasterxml.jackson.databind.json.JsonMapper; + + class A { + private final JsonMapper mapper = new JsonMapper(); + + void configure() { + // TODO disable could not be folded to the builder of JsonMapper. Use mapper.rebuild().disable(...).build() or move to the mapper's instantiation site. + this.mapper.disable(SerializationFeature.INDENT_OUTPUT); + } + } + """ + ) + ); + } + } + @Nested class FluentChain {