Review comments

neoforged · Jun 25, 2024 · 46632e2 · 46632e2
1 parent 95a5352
commit 46632e2
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 23 deletions.
diff --git a/parchment/src/main/java/net/neoforged/jst/parchment/GatherReplacementsVisitor.java b/parchment/src/main/java/net/neoforged/jst/parchment/GatherReplacementsVisitor.java
@@ -42,7 +42,8 @@ class GatherReplacementsVisitor extends PsiRecursiveElementVisitor {
 
     public GatherReplacementsVisitor(NamesAndDocsDatabase namesAndDocs,
                                      boolean enableJavadoc,
-                                     @Nullable UnaryOperator<String> conflictResolver, Replacements replacements) {
+                                     @Nullable UnaryOperator<String> conflictResolver,
+                                     Replacements replacements) {
         this.namesAndDocs = namesAndDocs;
         this.enableJavadoc = enableJavadoc;
         this.conflictResolver = conflictResolver;
@@ -85,20 +86,16 @@ public void visitElement(@NotNull PsiElement element) {
                 Map<String, String> renamedParameters = new HashMap<>();
 
                 final UnaryOperator<String> namer;
-                if (conflictResolver == null) {
+                if (conflictResolver == null || psiMethod.getBody() == null) {
                     namer = UnaryOperator.identity();
                 } else {
-                    if (psiMethod.getBody() == null) {
-                        namer = conflictResolver;
-                    } else {
-                        final Set<String> localRefs = new HashSet<>();
-                        // Existing parameter names are considered reserved to avoid patched-in parameters to conflict with Parchment names
-                        for (JvmParameter parameter : psiMethod.getParameters()) {
-                            localRefs.add(parameter.getName());
-                        }
-                        new ReservedVariableNamesCollector(localRefs).visitElement(psiMethod.getBody());
-                        namer = p -> localRefs.contains(p) ? conflictResolver.apply(p) : p;
+                    final Set<String> localRefs = new HashSet<>();
+                    // Existing parameter names are considered reserved to avoid patched-in parameters to conflict with Parchment names
+                    for (JvmParameter parameter : psiMethod.getParameters()) {
+                        localRefs.add(parameter.getName());
                     }
+                    new ReservedVariableNamesCollector(localRefs).visitElement(psiMethod.getBody());
+                    namer = p -> localRefs.contains(p) ? conflictResolver.apply(p) : p;
                 }
 
                 List<String> parameterOrder = new ArrayList<>();
@@ -192,11 +189,4 @@ private void applyJavadoc(PsiJavaDocumentedElement psiElement,
             JavadocHelper.enrichJavadoc(psiElement, javadoc, replacements);
         }
     }
-
-    private static String uppercase(String str) {
-        if (str.length() == 1) {
-            return String.valueOf(Character.toUpperCase(str.charAt(0)));
-        }
-        return Character.toUpperCase(str.charAt(0)) + str.substring(1);
-    }
 }
diff --git a/parchment/src/main/java/net/neoforged/jst/parchment/ParchmentTransformer.java b/parchment/src/main/java/net/neoforged/jst/parchment/ParchmentTransformer.java
@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Path;
+import java.util.Locale;
 import java.util.function.UnaryOperator;
 
 public class ParchmentTransformer implements SourceTransformer {
@@ -34,7 +35,7 @@ public void beforeRun(TransformContext context) {
             }
 
             if (Character.isLetterOrDigit(conflictPrefix.charAt(conflictPrefix.length() - 1))) {
-                conflictResolver = p -> conflictPrefix + toUpperCase(p);
+                conflictResolver = p -> conflictPrefix + capitalize(p);
             } else {
                 conflictResolver = p -> conflictPrefix + p;
             }
@@ -54,9 +55,9 @@ public void visitFile(PsiFile psiFile, Replacements replacements) {
         visitor.visitElement(psiFile);
     }
 
-    private static String toUpperCase(String str) {
+    private static String capitalize(String str) {
         if (str.length() == 1) {
-            return String.valueOf(Character.toUpperCase(str.charAt(0)));
+            return str.toUpperCase(Locale.ROOT);
         }
         return Character.toUpperCase(str.charAt(0)) + str.substring(1);
     }

diff --git a/parchment/src/main/java/net/neoforged/jst/parchment/PsiParchmentHelper.java b/parchment/src/main/java/net/neoforged/jst/parchment/PsiParchmentHelper.java
@@ -69,7 +69,10 @@ public static NamesAndDocsForMethod getMethodData(NamesAndDocsDatabase namesAndD
 
     public static Set<String> getAllFieldNames(PsiClass clazz) {
         var existing = clazz.getUserData(ALL_FIELD_NAMES);
-        if (existing != null) return existing;
+        if (existing != null) {
+            return existing;
+        }
+
         existing = new HashSet<>();
         for (PsiField field : clazz.getAllFields()) {
             existing.add(field.getName());