Remove reflective validation from ReplaceStringLiteralWithConstant#6825
Closed
Remove reflective validation from ReplaceStringLiteralWithConstant#6825
Conversation
The validate() method used Class.forName() to reflectively verify that the target constant exists when literalValue was not provided. This fails when the target class is not on the recipe classpath, which is the common case (e.g. org.springframework.http.MediaType is not a dependency of rewrite-spring), producing errors like: fullyQualifiedConstantName was 'org.springframework.http.MediaType.TEXT_HTML_VALUE' but it No class for specified name was found. The reflective fallback in getLiteralValue() already handles this gracefully by returning null, causing getVisitor() to return a noop. Validation should not reject the recipe for a condition that the runtime already handles.
timtebeek
deleted the
remove-reflective-validation-from-replace-string-literal
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's Changed
ReplaceStringLiteralWithConstant.validate()usedClass.forName()to reflectively verify the target constant whenliteralValuewas not provided. This fails when the target class isn't on the recipe classpath, producing validation errors like:This is the common case —
rewrite-springdoesn't depend onspring-web, soorg.springframework.http.MediaTypeandorg.springframework.http.HttpHeadersare never on the recipe classpath.The runtime already handles this gracefully:
getLiteralValue()catchesClassNotFoundExceptionand returnsnull, andgetVisitor()returns a noop. Validation should not reject the recipe for a condition the runtime handles.Fix
Remove the reflective validation block from
validate(). Keep only thefullyQualifiedConstantNameblank check. The reflective fallback ingetLiteralValue()is retained as a best-effort mechanism for when the class happens to be on the classpath.Test plan
ReplaceStringLiteralWithConstantTesttests pass (11 tests)