From 8c30ca6005a22cab88dd168198895d49e49b5b09 Mon Sep 17 00:00:00 2001 From: lkerford Date: Mon, 21 Oct 2024 16:28:18 -0700 Subject: [PATCH 1/3] Cap the lengths of generate names We could generate some unreasonably long variable names. By default, we will cap variable names at 100 characters but people can change this was desired. We will crop the generated variable name to the nearest "_" if there is one available --- .../ReplaceDuplicateStringLiterals.java | 17 +++++++++++- .../ReplaceDuplicateStringLiteralsTest.java | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java index 1f4138dd75..00a710be8e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java @@ -44,6 +44,12 @@ public class ReplaceDuplicateStringLiterals extends Recipe { @Nullable Boolean includeTestSources; + @Option(displayName = "Maximum length of the generate variable names", + description = "By default this is set to 100 characters", + required = false) + @Nullable + Integer maxVariableLength = 100; + @Override public String getDisplayName() { return "Replace duplicate `String` literals"; @@ -181,7 +187,16 @@ private String transformToVariableName(String valueOfLiteral) { prevIsLower = Character.isLowerCase(c); } } - return VariableNameUtils.normalizeName(newName.toString()); + String newNameString = newName.toString(); + while(newNameString.length() > maxVariableLength){ + int indexOf = newNameString.lastIndexOf("_"); + if(indexOf > -1) { + newNameString = newNameString.substring(0, indexOf); + } else{ + newNameString = newNameString.substring(0, maxVariableLength); + } + } + return VariableNameUtils.normalizeName(newNameString); } }); } diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiteralsTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiteralsTest.java index 63b3f7a8f7..849e7f3773 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiteralsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiteralsTest.java @@ -148,6 +148,32 @@ class A { ); } + @Test + void generatedNameIsVeryLong() { + rewriteRun( + //language=java + java( + """ + package org.foo; + class A { + final String val1 = "ThisIsAnUnreasonablyLongVariableNameItGoesOnAndOnForAVeryLongTimeItMightNeverEndWhoIsToKnowHowLongItWillKeepGoingAndGoing"; + final String val2 = "ThisIsAnUnreasonablyLongVariableNameItGoesOnAndOnForAVeryLongTimeItMightNeverEndWhoIsToKnowHowLongItWillKeepGoingAndGoing"; + final String val3 = "ThisIsAnUnreasonablyLongVariableNameItGoesOnAndOnForAVeryLongTimeItMightNeverEndWhoIsToKnowHowLongItWillKeepGoingAndGoing"; + } + """, + """ + package org.foo; + class A { + private static final String THIS_IS_AN_UNREASONABLY_LONG_VARIABLE_NAME_IT_GOES_ON_AND_ON_FOR_AVERY_LONG_TIME_IT_MIGHT_NEVER_END = "ThisIsAnUnreasonablyLongVariableNameItGoesOnAndOnForAVeryLongTimeItMightNeverEndWhoIsToKnowHowLongItWillKeepGoingAndGoing"; + final String val1 = THIS_IS_AN_UNREASONABLY_LONG_VARIABLE_NAME_IT_GOES_ON_AND_ON_FOR_AVERY_LONG_TIME_IT_MIGHT_NEVER_END; + final String val2 = THIS_IS_AN_UNREASONABLY_LONG_VARIABLE_NAME_IT_GOES_ON_AND_ON_FOR_AVERY_LONG_TIME_IT_MIGHT_NEVER_END; + final String val3 = THIS_IS_AN_UNREASONABLY_LONG_VARIABLE_NAME_IT_GOES_ON_AND_ON_FOR_AVERY_LONG_TIME_IT_MIGHT_NEVER_END; + } + """ + ) + ); + } + @Test void replaceRedundantLiteralInMethodInvocation() { rewriteRun( From e243b821a5db69073c28cb10cb646906a12c7238 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Oct 2024 09:32:34 +0200 Subject: [PATCH 2/3] Apply suggestions from code review --- .../staticanalysis/ReplaceDuplicateStringLiterals.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java index 00a710be8e..6341dfad70 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java @@ -188,11 +188,11 @@ private String transformToVariableName(String valueOfLiteral) { } } String newNameString = newName.toString(); - while(newNameString.length() > maxVariableLength){ + while (newNameString.length() > maxVariableLength){ int indexOf = newNameString.lastIndexOf("_"); - if(indexOf > -1) { + if (indexOf > -1) { newNameString = newNameString.substring(0, indexOf); - } else{ + } else { newNameString = newNameString.substring(0, maxVariableLength); } } From 88f26f85c82d39eed0dbf145a31aa0d23f58ad97 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Oct 2024 09:34:42 +0200 Subject: [PATCH 3/3] Use ternary for `.substring` instead of similar if/else --- .../staticanalysis/ReplaceDuplicateStringLiterals.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java index 6341dfad70..4047398824 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java @@ -190,11 +190,7 @@ private String transformToVariableName(String valueOfLiteral) { String newNameString = newName.toString(); while (newNameString.length() > maxVariableLength){ int indexOf = newNameString.lastIndexOf("_"); - if (indexOf > -1) { - newNameString = newNameString.substring(0, indexOf); - } else { - newNameString = newNameString.substring(0, maxVariableLength); - } + newNameString = newNameString.substring(0, indexOf > -1 ? indexOf : maxVariableLength); } return VariableNameUtils.normalizeName(newNameString); }