Add Apache Commons StringUtils refaster template replacements #271

AlekSimpson · 2023-08-11T22:03:57Z

What's changed?

Refaster rule recipes to replace Apache Commons StringUtils methods with JDK internals where possible.

What's your motivation?

Fixes #267

Anyone you would like to review specifically?

@timtebeek

Checklist

I've added unit tests to cover both positive and negative cases
I've added the license header to any new files through ./gradlew licenseFormat
I've used the IntelliJ auto-formatter on affected files
I've updated the documentation (if applicable)

knutwannheden

I have added a first set of review comments.

src/main/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtils.java

…/ApacheCommonsStringUtils.java Co-authored-by: Tim te Beek <tim@moderne.io>

AlekSimpson · 2023-08-14T15:15:01Z

Ok the latest commit has all the changes made based off of the feedback given.

AlekSimpson · 2023-08-14T15:43:13Z

I am still seeing errors when I run the tests. It tells me that an enclosing instance that contains org.openrewrite.java.migrate.apache.commons.lang.ApacheCommonsStringUtilsRecipes.DefaultStringRecipe is required. I'm not to sure what that error is saying to be honest.

knutwannheden · 2023-08-14T15:51:36Z

I am still seeing errors when I run the tests. It tells me that an enclosing instance that contains org.openrewrite.java.migrate.apache.commons.lang.ApacheCommonsStringUtilsRecipes.DefaultStringRecipe is required. I'm not to sure what that error is saying to be honest.

You probably need to declare the Refaster template classes as static.

AlekSimpson · 2023-08-14T16:02:33Z

I have made them all static. I think the issue could be because I have the a local version of rewrite-templating to get some fixes earlier so I could make some progress while I waited for the fixes to make it to the main branch.

knutwannheden · 2023-08-14T16:18:06Z

Meanwhile quite a few fixes have been integrated. I suggest you try with the latest integration build.

...test/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtilsTest.java

timtebeek · 2023-08-17T16:01:18Z

We need a merge of this one to fix the parentheses cases:

Add doAfterVisit(UnnecessaryParentheses) to generated recipes rewrite-templating#22

timtebeek · 2023-08-17T16:30:41Z

...test/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtilsTest.java

+
+              class Foo {
+                  void test(String s) {
+                      String test = StringUtils.strip(bar()).toString();


@knutwannheden A more interesting failure: here I would not want to make a change when the input is a method call itself; I suspect that's harder to fix quickly; would this need changes to JavaTemplateSemanticallyEqual.matchesTemplate either in the matcher, or in the TemplateMatchResult? Curious to hear your thoughts, as I have no experience there yet.

A perhaps better troublesome case is when people pass in iterator.next(); then you definitely don't want that called twice.

Logged as

When before template parameter is used twice in after template, do not call method invocations such as iterator.next() twice rewrite-templating#27

…mplates

build.gradle.kts

timtebeek and others added 14 commits August 10, 2023 21:09

Add Apache Commons StringUtils refaster template replacements

dc71bce

Add missing import

b462f08

Fix import in test

6909bea

Fix import in test

0c612c6

Rename recipe since package already includes Apache Commons

7568acc

Show _once_ that static imports work too

6d2bb7b

Format fix

129aa9a

initial commit

156c6f5

updated tests

9519cb0

added a couple more tests and recipe

c392897

more tests and recipes

54199e1

more recipes, all moved into one class

be89223

updated license headers

a1bb548

small fix

1c77e3b

knutwannheden reviewed Aug 11, 2023

View reviewed changes

timtebeek self-requested a review August 11, 2023 22:15

timtebeek changed the base branch from main to apache_commons_string_utils_refaster_templates August 11, 2023 22:30

timtebeek reviewed Aug 11, 2023

View reviewed changes

src/main/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtils.java Outdated Show resolved Hide resolved

timtebeek mentioned this pull request Aug 14, 2023

Continue work on AutoTemplate openrewrite/rewrite-templating#2

Closed

Update src/main/java/org/openrewrite/java/migrate/apache/commons/lang…

b8c597a

…/ApacheCommonsStringUtils.java Co-authored-by: Tim te Beek <tim@moderne.io>

timtebeek assigned AlekSimpson Aug 14, 2023

timtebeek added the recipe Recipe requested label Aug 14, 2023

made changes from PR feedback

803afad

used regionMatches() for EndsWithIgnoreCase

b4b4e67

timtebeek marked this pull request as draft August 14, 2023 16:20

Verify there's no change yet in additional cases

9137626

timtebeek reviewed Aug 17, 2023

View reviewed changes

...test/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtilsTest.java Outdated Show resolved Hide resolved

timtebeek reviewed Aug 17, 2023

View reviewed changes

...test/java/org/openrewrite/java/migrate/apache/commons/lang/ApacheCommonsStringUtilsTest.java Outdated Show resolved Hide resolved

timtebeek mentioned this pull request Aug 17, 2023

Add doAfterVisit(UnnecessaryParentheses) to generated recipes openrewrite/rewrite-templating#22

Merged

timtebeek added 2 commits August 17, 2023 17:39

Add parentheses around replacement templates that need it

f5b7cef

Test that input methods are not called twice

169d3ac

timtebeek reviewed Aug 17, 2023

View reviewed changes

timtebeek mentioned this pull request Aug 17, 2023

Generate uses type and uses method openrewrite/rewrite-templating#24

Merged

Swap case should also use parentheses

1963661

timtebeek mentioned this pull request Aug 18, 2023

Limit doAfterVisit(UnnecessaryParenthesesVisitor) to afterTemplate lstTypes that need it openrewrite/rewrite-templating#25

Closed

2 tasks

Fix rules Abbreviate through EqualsIgnoreCase

eed020f

timtebeek mentioned this pull request Aug 18, 2023

When before template parameter is used twice in after template, do not call method invocations such as iterator.next() twice openrewrite/rewrite-templating#27

Closed

timtebeek added 3 commits August 18, 2023 12:49

Disable inputMethodsNotCalledTwice; tracked separately

2f1abc4

Check Equals through Lowercase

0cc52fc

Fix RemoveEnd

db257b9

timtebeek mentioned this pull request Aug 18, 2023

Simplify boolean expressions after a replacement openrewrite/rewrite-templating#28

Closed

timtebeek added 3 commits August 18, 2023 14:04

Fix Replace and Split

5d985dd

Disable with note broken recipes

f2ced41

Disable Uncapitalize

78d9007

timtebeek marked this pull request as ready for review August 18, 2023 13:30

Merge branch 'main' into alek/apache_commons_string_utils_refaster_te…

d8bfb50

…mplates

timtebeek reviewed Aug 18, 2023

View reviewed changes

build.gradle.kts Outdated Show resolved Hide resolved

Update build.gradle.kts

44d1d81

timtebeek merged commit 89b00e6 into main Aug 18, 2023

timtebeek deleted the alek/apache_commons_string_utils_refaster_templates branch August 18, 2023 17:01

timtebeek mentioned this pull request Aug 18, 2023

StringUtils.defaultString -> Objects.toString #267

Closed

AlekSimpson mentioned this pull request Aug 18, 2023

Apache Commons File Utils Migrations #274

Merged

4 tasks

This was referenced Aug 18, 2023

Simplify null and empty string comparisons openrewrite/rewrite-static-analysis#154

Closed

Simplify boolean expressions comparing with null and isEmpty openrewrite/rewrite#3489

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Apache Commons StringUtils refaster template replacements #271

Add Apache Commons StringUtils refaster template replacements #271

AlekSimpson commented Aug 11, 2023 •

edited by timtebeek

Loading

knutwannheden left a comment

AlekSimpson commented Aug 14, 2023

AlekSimpson commented Aug 14, 2023

knutwannheden commented Aug 14, 2023

AlekSimpson commented Aug 14, 2023

knutwannheden commented Aug 14, 2023

timtebeek commented Aug 17, 2023

timtebeek Aug 17, 2023

timtebeek Aug 18, 2023

timtebeek Aug 18, 2023

Add Apache Commons StringUtils refaster template replacements #271

Add Apache Commons StringUtils refaster template replacements #271

Conversation

AlekSimpson commented Aug 11, 2023 • edited by timtebeek Loading

What's changed?

What's your motivation?

Anyone you would like to review specifically?

Checklist

knutwannheden left a comment

Choose a reason for hiding this comment

AlekSimpson commented Aug 14, 2023

AlekSimpson commented Aug 14, 2023

knutwannheden commented Aug 14, 2023

AlekSimpson commented Aug 14, 2023

knutwannheden commented Aug 14, 2023

timtebeek commented Aug 17, 2023

timtebeek Aug 17, 2023

Choose a reason for hiding this comment

timtebeek Aug 18, 2023

Choose a reason for hiding this comment

timtebeek Aug 18, 2023

Choose a reason for hiding this comment

AlekSimpson commented Aug 11, 2023 •

edited by timtebeek

Loading