Do not throw if ReplaceStringLiteralWithConstant value not found (#4224)

openrewrite · Jun 3, 2024 · cfbb5b0 · cfbb5b0
1 parent fadb443
commit cfbb5b0
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 21 deletions.
diff --git a/...te-java-test/src/test/java/org/openrewrite/java/ReplaceStringLiteralWithConstantTest.java b/...te-java-test/src/test/java/org/openrewrite/java/ReplaceStringLiteralWithConstantTest.java
@@ -17,6 +17,10 @@
 
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.ExecutionContext;
+import org.openrewrite.Tree;
+import org.openrewrite.internal.lang.Nullable;
+import org.openrewrite.java.tree.J;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
@@ -39,13 +43,13 @@ void doNothingIfStringLiteralNotFound() {
         rewriteRun(
           spec -> spec.recipe(new ReplaceStringLiteralWithConstant(EXAMPLE_STRING_CONSTANT, EXAMPLE_STRING_FQN)),
           java(
-                """
-            package org.openrewrite.java;
-                            
-            class Test {
-                String s = "FooBar";
-            }
             """
+              package org.openrewrite.java;
+              
+              class Test {
+                  String s = "FooBar";
+              }
+              """
           )
         );
     }
@@ -57,7 +61,7 @@ void doNotReplaceDefinition() {
           java(
             """
               package org.openrewrite.java;
-                          
+              
               class ReplaceStringLiteralWithConstantTest {
                   static final String EXAMPLE_STRING_CONSTANT = "Hello World!";
               }
@@ -73,14 +77,14 @@ void shouldNotAddImportWhenUnnecessary() {
           java(
             """
               package org.openrewrite.java;
-                              
+              
               class Test {
                   Object o = "Hello World!";
               }
               """,
-            """                
+            """
               package org.openrewrite.java;
-                          
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
@@ -99,9 +103,9 @@ class Test {
                   Object o = "Hello World!";
               }
               """,
-            """                
+            """
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                          
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
@@ -117,7 +121,7 @@ void replaceLiteralWithUserDefinedConstant() {
           java(
             """
               package com.abc;
-                          
+              
               class A {
                   String v = "newValue";
                   private String method() {
@@ -127,9 +131,9 @@ private String method() {
               """,
             """
               package com.abc;
-                          
+              
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                          
+              
               class A {
                   String v = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
                   private String method() {
@@ -153,7 +157,7 @@ class Test {
               """,
             """
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                          
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
@@ -174,7 +178,7 @@ class Test {
               """,
             """
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                          
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
@@ -202,7 +206,7 @@ class Test {
               """,
             """
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                           
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
@@ -231,12 +235,35 @@ class Test {
               """,
             """
               import org.openrewrite.java.ReplaceStringLiteralWithConstantTest;
-                          
+              
               class Test {
                   Object o = ReplaceStringLiteralWithConstantTest.EXAMPLE_STRING_CONSTANT;
               }
               """
           )
         );
     }
+
+    @Test
+    void missingFieldNoError() {
+        rewriteRun(
+          spec -> spec.recipe(RewriteTest.toRecipe(() -> new JavaVisitor<>(){
+              @Override
+              public @Nullable J visit(@Nullable Tree tree, ExecutionContext executionContext) {
+                  // Circumvent validation to match use in rewrite-spring's ReplaceStringLiteralsWithMediaTypeConstants
+                  doAfterVisit(new ReplaceStringLiteralWithConstant(EXAMPLE_STRING_FQN + "_xyz").getVisitor());
+                  return super.visit(tree, executionContext);
+              }
+          })),
+          java(
+            """
+              package org.openrewrite.java;
+              
+              class Test {
+                  Object o = "Hello World!";
+              }
+              """
+          )
+        );
+    }
 }
diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ReplaceStringLiteralWithConstant.java b/rewrite-java/src/main/java/org/openrewrite/java/ReplaceStringLiteralWithConstant.java
@@ -69,12 +69,14 @@ public String getDescription() {
         return "Replace String literal with constant, adding import on class if needed.";
     }
 
+    @Nullable
     public String getLiteralValue() {
         if (this.literalValue == null && this.fullyQualifiedConstantName != null) {
             try {
                 this.literalValue = (String) getConstantValueByFullyQualifiedName(this.fullyQualifiedConstantName);
             } catch (ClassNotFoundException | IllegalAccessException | NoSuchFieldException e) {
-                throw new IllegalArgumentException("Failed to retrieve value from the configured constant", e);
+                // Failed to retrieve unspecified value; field might be missing in this version of the class
+                return null;
             }
         }
         return this.literalValue;
@@ -107,7 +109,8 @@ public Validated validate() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new ReplaceStringLiteralVisitor(getLiteralValue(), getFullyQualifiedConstantName());
+        String value = getLiteralValue();
+        return value == null ? TreeVisitor.noop() : new ReplaceStringLiteralVisitor(value, fullyQualifiedConstantName);
     }
 
     private static class ReplaceStringLiteralVisitor extends JavaVisitor<ExecutionContext> {