Skip to content
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

StringIsEmpty with Refaster test fails #66

Closed
Uriziel47 opened this issue Nov 23, 2024 · 3 comments · Fixed by openrewrite/rewrite-templating#117
Closed

StringIsEmpty with Refaster test fails #66

Uriziel47 opened this issue Nov 23, 2024 · 3 comments · Fixed by openrewrite/rewrite-templating#117

Comments

@Uriziel47
Copy link

Good Morning,

I just wanted to get started on openrewrite. So I decided to check this Repository first and get my hands dirty.
Unfortunately I got stucked in the first try using Refaster.

So after writing the Refaster recipe of StringIsEmpty. I didn't get the testcase, StringIsEmpty::standardizeStringIsEmpty to pass. Either the case of string.equals("") or string.length == 0 case is wrong.

So after inspecting the generated class. I realized, that the first @BeforeTemplate annotation decides which visitor method will be used for all @BeforeTemplate annotated methods.

In case string.equals("") the visitor method visitMethodInvocation is been used. And in case of string.length == 0 the visitor method visitBinary is been used.

So I am just curious, if I misunderstood the documentation, used the wrong version, or is this a bug, which I did not find in the issues.

For clarification I created a branch in my learning repository: link to repo.
In there the discussed variants with the testcases.

  • StringIsEmptyCheckEqualsFirst
  • StringIsEmptyCheckLengthFirst
  • StringIsEmptyWithSubclasses (splitted equals and length into refaster subclasses)
  • StringisEmptyVisitorRecipe (The recipe I thought it would be generated)

Thanks for your answers in advance.

@timtebeek
Copy link
Contributor

Hi @Uriziel47 ; thanks for the detailed report! This is indeed a bug, that we'll track against the refaster template processor:

Let's continue the conversation there! Any fix & release would also resolve that issue here, without a need for changes in this project beyond bumping the version.

@Uriziel47
Copy link
Author

Hi @timtebeek, thank you very much for the fast feedback and fix!

@timtebeek
Copy link
Contributor

You're welcome; your report made it easy to pick up. You'll have noticed I also released a new version of rewrite-templating and bumped the versions here. We'll do a full new release this Wednesday. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants