Add UseLocaleWithCaseConversions recipe for STR02-J (#78)

src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java

@@ -67,6 +67,15 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
                         if (shouldRetainOnStaticMethod(methodType)) {
                             return m;
                         }
+                        // If the enclosing method has dependent type parameters (e.g., <T, S extends T>),
+                        // removing the explicit type argument from this call can prevent the compiler from
+                        // correctly inferring the dependent type parameters. For example:
+                        //   lexicographical(Comparator.<Integer>naturalOrder())
+                        // where lexicographical is declared as <T, S extends T>, the explicit <Integer>
+                        // is required so the compiler can bind both T and S correctly.
+                        if (hasDependentTypeParameters(enclosingMethod.getMethodType())) {
+                            return m;
+                        }
                         // Cannot remove type parameters if it would introduce ambiguity about which method should be called
                         if (enclosingMethod.getMethodType() == null) {
                             return m;
@@ -173,6 +182,73 @@ private boolean canInferTypeArgumentsFromArguments(JavaType.Method methodType) {
                         .forEach(it -> formalTypeNames.remove(((JavaType.GenericTypeVariable) it).getName()));
                 return formalTypeNames.isEmpty();
             }
+
+            /**
+             * Returns true if the given method has dependent type parameters — that is, at least one
+             * type parameter has a bound that references another type parameter declared by the same
+             * method (e.g., {@code <T, S extends T>}).
+             * <p>
+             * When such dependencies exist, removing explicit type arguments from an argument expression
+             * can prevent the compiler from resolving the dependent type parameters correctly.
+             */
+            private boolean hasDependentTypeParameters(JavaType.@Nullable Method enclosingMethodType) {
+                if (enclosingMethodType == null) {
+                    return false;
+                }
+                List<String> formalNames = enclosingMethodType.getDeclaredFormalTypeNames();
+                if (formalNames.size() < 2) {
+                    // Need at least two type parameters for one to bound another
+                    return false;
+                }
+                List<JavaType> typesToSearch = new ArrayList<>(enclosingMethodType.getParameterTypes());
+                if (enclosingMethodType.getReturnType() != null) {
+                    typesToSearch.add(enclosingMethodType.getReturnType());
+                }
+                return typesToSearch.stream()
+                        .anyMatch(t -> hasGenericWithFormalParamBound(t, formalNames));
+            }
+
+            /**
+             * Recursively searches {@code type} for a {@link JavaType.GenericTypeVariable} whose name
+             * is among {@code formalNames} AND whose bounds directly reference another name in
+             * {@code formalNames}. Such a variable represents a dependent type parameter.
+             */
+            private boolean hasGenericWithFormalParamBound(@Nullable JavaType type, List<String> formalNames) {
+                if (type instanceof JavaType.GenericTypeVariable) {
+                    JavaType.GenericTypeVariable gtv = (JavaType.GenericTypeVariable) type;
+                    if (formalNames.contains(gtv.getName())) {
+                        for (JavaType bound : gtv.getBounds()) {
+                            if (referencesAnyFormalName(bound, formalNames)) {
+                                return true;
+                            }
+                        }
+                    }
+                    return false;
+                }
+                if (type instanceof JavaType.Parameterized) {
+                    return ((JavaType.Parameterized) type).getTypeParameters().stream()
+                            .anyMatch(t -> hasGenericWithFormalParamBound(t, formalNames));
+                }
+                if (type instanceof JavaType.Array) {
+                    return hasGenericWithFormalParamBound(((JavaType.Array) type).getElemType(), formalNames);
+                }
+                return false;
+            }
+
+            /**
+             * Returns true if {@code type} is, or transitively contains, a
+             * {@link JavaType.GenericTypeVariable} whose name is in {@code formalNames}.
+             */
+            private boolean referencesAnyFormalName(@Nullable JavaType type, List<String> formalNames) {
+                if (type instanceof JavaType.GenericTypeVariable) {
+                    return formalNames.contains(((JavaType.GenericTypeVariable) type).getName());
+                }
+                if (type instanceof JavaType.Parameterized) {
+                    return ((JavaType.Parameterized) type).getTypeParameters().stream()
+                            .anyMatch(t -> referencesAnyFormalName(t, formalNames));
+                }
+                return false;
+            }
         });
     }
 }

src/main/java/org/openrewrite/staticanalysis/UseLocaleWithCaseConversions.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.staticanalysis;
+
+import lombok.EqualsAndHashCode;
+import lombok.Value;
+import org.jspecify.annotations.Nullable;
+import org.openrewrite.*;
+import org.openrewrite.java.JavaIsoVisitor;
+import org.openrewrite.java.JavaTemplate;
+import org.openrewrite.java.MethodMatcher;
+import org.openrewrite.java.search.UsesMethod;
+import org.openrewrite.java.tree.J;
+
+import java.util.Set;
+
+import static java.util.Collections.singleton;
+
+@EqualsAndHashCode(callSuper = false)
+@Value
+public class UseLocaleWithCaseConversions extends Recipe {
+
+    private static final MethodMatcher TO_LOWER_CASE_METHOD_MATCHER = new MethodMatcher("java.lang.String toLowerCase()");
+    private static final MethodMatcher TO_UPPER_CASE_METHOD_MATCHER = new MethodMatcher("java.lang.String toUpperCase()");
+
+    @Option(displayName = "Locale",
+            description = "The name of the constant field on `java.util.Locale` to use, such as `ROOT` or `US`. " +
+                    "Defaults to `ROOT` when not provided, which is recommended for locale-independent, " +
+                    "algorithmic case conversions.",
+            example = "ROOT",
+            required = false)
+    @Nullable
+    String locale;
+
+    @Override
+    public String getDisplayName() {
+        return "Specify a `Locale` when comparing locale-dependent data";
+    }
+
+    @Override
+    public String getDescription() {
+        return "Explicitly specify a `Locale` when calling `String#toLowerCase()` or `String#toUpperCase()`, " +
+                "as the result of case conversion can vary between locales. For example, converting the letter " +
+                "`I` to lower case yields a dotless `ı` under a Turkish locale instead of the expected `i`. " +
+                "Relying on the platform default locale can therefore lead to unexpected and hard-to-reproduce " +
+                "bugs. See [STR02-J](https://wiki.sei.cmu.edu/confluence/display/java/STR02-J.+Specify+an+appropriate+locale+when+comparing+locale-dependent+data) " +
+                "for more information.";
+    }
+
+    @Override
+    public Set<String> getTags() {
+        return singleton("STR02-J");
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getVisitor() {
+        String localeConstant = (locale == null || locale.isEmpty()) ? "ROOT" : locale;
+        return Preconditions.check(
+                Preconditions.or(
+                        new UsesMethod<>(TO_LOWER_CASE_METHOD_MATCHER),
+                        new UsesMethod<>(TO_UPPER_CASE_METHOD_MATCHER)
+                ),
+                new JavaIsoVisitor<ExecutionContext>() {
+                    @Override
+                    public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+                        J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
+                        if (TO_LOWER_CASE_METHOD_MATCHER.matches(m) || TO_UPPER_CASE_METHOD_MATCHER.matches(m)) {
+                            m = JavaTemplate.builder("java.util.Locale." + localeConstant)
+                                    .contextSensitive()
+                                    .imports("java.util.Locale")
+                                    .build()
+                                    .apply(updateCursor(m), m.getCoordinates().replaceArguments());
+                            maybeAddImport("java.util.Locale");
+                        }
+                        return m;
+                    }
+                }
+        );
+    }
+}

src/main/resources/META-INF/rewrite/common-static-analysis.yml

@@ -102,6 +102,9 @@ recipeList:
 #  - org.openrewrite.staticanalysis.UseCollectionInterfaces
   - org.openrewrite.staticanalysis.UseDiamondOperator
   - org.openrewrite.staticanalysis.UseJavaStyleArrayDeclarations
+# Changes case-conversion semantics for locale-sensitive strings; opt-in only.
+# https://github.com/openrewrite/rewrite-static-analysis/issues/78
+#  - org.openrewrite.staticanalysis.UseLocaleWithCaseConversions
   - org.openrewrite.staticanalysis.UsePortableNewlines
 # https://github.com/openrewrite/rewrite-static-analysis/issues/10
 #  - org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface

src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.staticanalysis;
+
+import org.junit.jupiter.api.Test;
+import org.openrewrite.DocumentExample;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import static org.openrewrite.java.Assertions.java;
+
+class UseLocaleWithCaseConversionsTest implements RewriteTest {
+
+    @Override
+    public void defaults(RecipeSpec spec) {
+        spec.recipe(new UseLocaleWithCaseConversions(null));
+    }
+
+    @DocumentExample
+    @Test
+    void toLowerCaseWithoutLocale() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  String lower(String str) {
+                      return str.toLowerCase();
+                  }
+              }
+              """,
+            """
+              import java.util.Locale;
+
+              class Test {
+                  String lower(String str) {
+                      return str.toLowerCase(Locale.ROOT);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void toUpperCaseWithoutLocale() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  String upper(String str) {
+                      return str.toUpperCase();
+                  }
+              }
+              """,
+            """
+              import java.util.Locale;
+
+              class Test {
+                  String upper(String str) {
+                      return str.toUpperCase(Locale.ROOT);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeWhenLocaleAlreadySpecified() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.util.Locale;
+
+              class Test {
+                  String lower(String str) {
+                      return str.toLowerCase(Locale.US);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void useConfiguredLocale() {
+        rewriteRun(
+          spec -> spec.recipe(new UseLocaleWithCaseConversions("US")),
+          //language=java
+          java(
+            """
+              class Test {
+                  String lower(String str) {
+                      return str.toLowerCase();
+                  }
+              }
+              """,
+            """
+              import java.util.Locale;
+
+              class Test {
+                  String lower(String str) {
+                      return str.toLowerCase(Locale.US);
+                  }
+              }
+              """
+          )
+        );
+    }
+}

src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArgumentsTest.java

@@ -346,6 +346,33 @@ <T> void test(Class<T> clazz) {
         }
     }
 
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/783")
+    @Test
+    void retainExplicitTypeArgumentForDependentTypeParameters() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.util.Comparator;
+
+              class Test {
+                  /**
+                   * A method with dependent type parameters where S extends T.
+                   * The explicit type argument on the Comparator is required so the compiler
+                   * can bind both T and S (S extends T) correctly.
+                   */
+                  static <T, S extends T> Comparator<Iterable<S>> lexicographical(Comparator<T> comparator) {
+                      return null;
+                  }
+
+                  static final Comparator<Iterable<Integer>> COMPARATOR =
+                      lexicographical(Comparator.<Integer>naturalOrder());
+              }
+              """
+          )
+        );
+    }
+
     @Nested
     class kotlinTest {
         @Test