Fix reserveMethodNames to reserve each supplied name#36912
Open
junhyeong9812 wants to merge 1 commit into
Open
Fix reserveMethodNames to reserve each supplied name#36912junhyeong9812 wants to merge 1 commit into
junhyeong9812 wants to merge 1 commit into
Conversation
GeneratedClass.reserveMethodNames(String...) passed the entire varargs array to MethodName.of() inside the per-name loop instead of the current element. Since MethodName.of(String...) joins all parts into a single camel-case name, reserving two or more names (for example "apply" and "test") produced "applyTest", and the per-element check Assert.state(generatedName.equals(reservedMethodName)) failed with an IllegalStateException. Single-name calls worked only by accident. Reserve each supplied name individually by passing the loop variable. Signed-off-by: junhyeong9812 <pickjog@gmail.com>
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
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.
Overview
GeneratedClass.reserveMethodNames(String...)reserves a set of method names so thatgenerated methods will not collide with them.
Problem
Inside the per-name loop, the whole varargs array was passed to
MethodName.of(...)insteadof the current element:
MethodName.of(String...)joins all parts into a single camel-case name, soreserveMethodNames("apply", "test")produced"applyTest", and the per-element checkAssert.state("applyTest".equals("apply"))failed withIllegalStateException. Single-namecalls worked only by accident (a one-element array round-trips to the same name). The only
in-tree caller reserves a single name, so the multi-name path was never exercised.
Fix
Pass the loop variable so that each supplied name is reserved individually. A regression test
in
GeneratedClassTestsreserves two names.Note on intent
This fix assumes
reserveMethodNamesis meant to reserve each supplied name individually.The per-element assertion
Assert.state(generatedName.equals(reservedMethodName))and thejavadoc ("a set of reserved method names") both indicate this. If the intent was instead to
reserve a single combined name, the loop and the per-element assertion would be inconsistent
and a different fix would be appropriate — happy to adjust if I have misread the intent.