do not change variables if they use the correct naming convention #409

Feuermagier · Feb 19, 2024 · dbaf5b3 · dbaf5b3
1 parent a0e9dc5
commit dbaf5b3
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 30 deletions.
diff --git a/...grader-core/src/main/java/de/firemage/autograder/core/integrated/IdentifierNameUtils.java b/...grader-core/src/main/java/de/firemage/autograder/core/integrated/IdentifierNameUtils.java
@@ -46,10 +46,20 @@ public static boolean isCamelCase(String identifier) {
     }
 
     public static String toUpperSnakeCase(String identifier) {
+        // If the identifier is already in upper snake case like `DAMAGE`,
+        // don't change it, which would otherwise result in `D_A_M_A_G_E`.
+        if (isUpperSnakeCase(identifier)) {
+            return identifier;
+        }
+
         return getCaseFormat(identifier).converterTo(CaseFormat.UPPER_UNDERSCORE).convert(identifier);
     }
 
     public static String toLowerCamelCase(String identifier) {
+        if (isLowerCamelCase(identifier)) {
+            return identifier;
+        }
+
         return getCaseFormat(identifier).converterTo(CaseFormat.LOWER_CAMEL).convert(identifier);
     }
 

diff --git a/...t/java/de/firemage/autograder/core/check/general/TestConstantNamingAndQualifierCheck.java b/...t/java/de/firemage/autograder/core/check/general/TestConstantNamingAndQualifierCheck.java
@@ -15,60 +15,77 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-public class TestConstantNamingAndQualifierCheck extends AbstractCheckTest {
-    private static final String LOCALIZED_MESSAGE_KEY = "variable-should-be";
+class TestConstantNamingAndQualifierCheck extends AbstractCheckTest {
+    private static final List<ProblemType> PROBLEM_TYPES = List.of(
+        ProblemType.FIELD_SHOULD_BE_CONSTANT,
+        ProblemType.LOCAL_VARIABLE_SHOULD_BE_CONSTANT
+    );
+
+    private void assertProblem(Problem problem, String variable, String suggestion) {
+        assertEquals(
+            this.linter.translateMessage(
+                new LocalizedMessage(
+                    "variable-should-be",
+                    Map.of(
+                        "variable", variable,
+                        "suggestion", suggestion
+                    )
+                )
+            ),
+            this.linter.translateMessage(problem.getExplanation())
+        );
+    }
 
     @Test
     void testDefaultVisibility() throws LinterException, IOException {
-        List<Problem> problems = super.check(StringSourceInfo.fromSourceString(
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
             JavaVersion.JAVA_17,
             "Test",
             """
                 public class Test {
                     String exampleConstant = "example";
                 }
                 """
-        ), List.of(ProblemType.FIELD_SHOULD_BE_CONSTANT));
+        ), PROBLEM_TYPES);
 
+        assertProblem(problems.next(), "exampleConstant", "static final String EXAMPLE_CONSTANT = \"example\"");
 
-        assertEquals(1, problems.size());
-        assertEquals(ProblemType.FIELD_SHOULD_BE_CONSTANT, problems.get(0).getProblemType());
-        assertEquals(super.linter.translateMessage(
-            new LocalizedMessage(
-                LOCALIZED_MESSAGE_KEY,
-                Map.of(
-                    "variable", "exampleConstant",
-                    "suggestion", "static final String EXAMPLE_CONSTANT = \"example\""
-                )
-            )),
-            super.linter.translateMessage(problems.get(0).getExplanation())
-        );
+        problems.assertExhausted();
     }
 
     @Test
     void testOtherVisibility() throws LinterException, IOException {
-        List<Problem> problems = super.check(StringSourceInfo.fromSourceString(
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
             JavaVersion.JAVA_17,
             "Test",
             """
                 public class Test {
                     private final int variable = 1;
                 }
                 """
-        ), List.of(ProblemType.FIELD_SHOULD_BE_CONSTANT));
+        ), PROBLEM_TYPES);
 
+        assertProblem(problems.next(), "variable", "private static final int VARIABLE = 1");
 
-        assertEquals(1, problems.size());
-        assertEquals(ProblemType.FIELD_SHOULD_BE_CONSTANT, problems.get(0).getProblemType());
-        assertEquals(super.linter.translateMessage(
-                new LocalizedMessage(
-                    LOCALIZED_MESSAGE_KEY,
-                    Map.of(
-                        "variable", "variable",
-                        "suggestion", "private static final int VARIABLE = 1"
-                    )
-                )),
-            super.linter.translateMessage(problems.get(0).getExplanation())
-        );
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testLocalFinalVariableWithWrongNamingConvention() throws LinterException, IOException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+                public class Test {
+                    void foo() {
+                        final int DAMAGE = 1;
+                    }
+                }
+                """
+        ), PROBLEM_TYPES);
+
+        assertProblem(problems.next(), "DAMAGE", "private static final int DAMAGE = 1");
+
+        problems.assertExhausted();
     }
 }