Skip to content

Simplify Ternary expressions [#203]#231

Merged
timtebeek merged 7 commits intoopenrewrite:mainfrom
ammachado:simplify-ternary
Jan 8, 2024
Merged

Simplify Ternary expressions [#203]#231
timtebeek merged 7 commits intoopenrewrite:mainfrom
ammachado:simplify-ternary

Conversation

@ammachado
Copy link
Copy Markdown
Contributor

What's changed?

Simplify ternary expressions (closes #203)

What's your motivation?

Explore rewrite-templating

Anything in particular you'd like reviewers to focus on?

No

Anyone you would like to review specifically?

No one

Have you considered any alternatives or workarounds?

No

Any additional context

Requires changes provided by openrewrite/rewrite#3861

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@timtebeek
Copy link
Copy Markdown
Member

Thanks for starting this one! I've added two new cases to the tests to check handling on negation. Without the parentheses in the after template we could end up only negating the first boolean in a binary, changing the logic. I'm not yet pleased with the left over parentheses, but think we might need to fix that in rewrite-templating first.

@timtebeek
Copy link
Copy Markdown
Member

After fixes in rewrite-templating and rewrite it appears we now need a fix in rewrite-kotlin before we can get this one through. Thanks for your patience! :)

Copy link
Copy Markdown
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help in seeing this through! Quite some fixes and enhancements that will help other cases as well, so very useful adding this here.

Comment thread src/main/java/org/openrewrite/staticanalysis/SimplifyTernary.java Outdated
Comment thread src/test/java/org/openrewrite/staticanalysis/SimplifyTernaryTest.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Cover ? true : false

2 participants