-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed ChangeType for stackoverflow #3929
Conversation
rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java
Outdated
Show resolved
Hide resolved
Thanks for the proposed fix @zacscoding ! Seems like this is specific to Kotlin, so I'm doing a quick tag of @knutwannheden in case he'd like to solve this differently given the context above. |
Thanks @timtebeek , I look forward to the problem being resolved :) |
@zacscoding As you are saying a unit test like the following passes without issues. I wonder what the problem is when run differently. @Test
void companion() {
rewriteRun(
spec -> spec.recipe(new ChangeType("a.ApiModelProperty", "a.Schema", true)),
kotlin(
"""
package a
annotation class ApiModelProperty
""",
SourceSpec::skip
),
kotlin(
"""
import a.ApiModelProperty
class MyClass {
@ApiModelProperty
private val code: String? = null
companion object {
const val MY_CODE = "001"
}
}
""",
"""
import a.Schema
class MyClass {
@Schema
private val code: String? = null
companion object {
const val MY_CODE = "001"
}
}
"""
)
);
} |
@knutwannheden Sorry for the late response. I just tried to execute rewriteRun command with our recipe jar like |
@knutwannheden @timtebeek |
I don't see this PR as harmful at any rate, even if we can't fully reproduce. If it solves @zacscoding issue, I think it should be merged without further ado. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions could not be made:
- rewrite-gradle/src/main/groovy/RewriteSettings.groovy
- lines 31-36
What's changed?
Check if
classType
is the same asclassType.getOwningClass()
in ChangeType::getTopLevelClassName()What's your motivation?
Fixed
ChangeType
recipe to prevent stack over flow.Any additional context
I used
ChangeType
recipe to change@ApiModelProperty
annotation to@Schema
and below is my sample kotlin code.Above sample code is working well in the unit test code, but stack over flow error occurs if i run rewrite recipes.
More details, in the first call,
classType
isMyClass$Companion
andclassType.getOwningClass()
isMyClass
.In the recursive calls,
classType
andclassType.getOwningClass()
is the MyClass repeatedly.