rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Fixed ChangeType for stackoverflow

Open zacscoding opened this issue 1 year ago • 5 comments

What's changed?

Check if classType is the same as classType.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.

class MyClass {
  @ApiModelProperty("description")
  private val code: String? = null

  companion object {
    const val MY_CODE = "001"
  }
}

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 is MyClass$Companion and classType.getOwningClass() is MyClass. In the recursive calls, classType and classType.getOwningClass() is the MyClass repeatedly.

zacscoding avatar Jan 17 '24 13:01 zacscoding

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.

timtebeek avatar Jan 17 '24 13:01 timtebeek

Thanks @timtebeek , I look forward to the problem being resolved :)

zacscoding avatar Jan 17 '24 13:01 zacscoding

@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 avatar Jan 20 '24 19:01 knutwannheden

@knutwannheden Sorry for the late response. I just tried to execute rewriteRun command with our recipe jar like gradlew --init-script recipe.gradle rewriteRun :(

zacscoding avatar Jan 24 '24 15:01 zacscoding

@knutwannheden @timtebeek
Could you please review this PR again? I think this PR is appropriate as defensive code as well.

zacscoding avatar Jun 13 '24 08:06 zacscoding

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.

jkschneider avatar Jul 24 '24 03:07 jkschneider