separate check for constant names which contain their string value in…

…to separate check
Feuermagier · Jan 18, 2024 · a5b5ec7 · a5b5ec7
1 parent 9a0aa89
commit a5b5ec7
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 19 deletions.
diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java b/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java
@@ -14,6 +14,7 @@ public enum ProblemType {
     EQUALS_HASHCODE_COMPARABLE_CONTRACT,
     UNCHECKED_TYPE_CAST,
     TOO_MANY_EXCEPTIONS,
+    CONSTANT_NAME_CONTAINS_VALUE,
     DEPRECATED_COLLECTION_USED,
     COLLECTION_IS_EMPTY_REIMPLEMENTED,
     STRING_IS_EMPTY_REIMPLEMENTED,

diff --git a/...ain/java/de/firemage/autograder/core/check/naming/ConstantsHaveDescriptiveNamesCheck.java b/...ain/java/de/firemage/autograder/core/check/naming/ConstantsHaveDescriptiveNamesCheck.java
@@ -12,14 +12,13 @@
 import spoon.reflect.code.CtLiteral;
 import spoon.reflect.code.CtTypeAccess;
 import spoon.reflect.declaration.CtField;
-import spoon.reflect.declaration.CtVariable;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
 
-@ExecutableCheck(reportedProblems = {ProblemType.MEANINGLESS_CONSTANT_NAME})
+@ExecutableCheck(reportedProblems = {ProblemType.MEANINGLESS_CONSTANT_NAME, ProblemType.CONSTANT_NAME_CONTAINS_VALUE})
 public class ConstantsHaveDescriptiveNamesCheck extends IntegratedCheck {
     private static final List<String> NUMBER_PRE_SUFFIXES =
             List.of("index", "number", "value", "argument", "element", "param", "parameter", "arg", "group", "constant", "value_of");
@@ -145,20 +144,6 @@ private static boolean containsValueInName(String name, CtLiteral<?> value) {
         return lowerCaseName.contains(valueString);
     }
 
-    private void reportProblem(String key, CtVariable<?> ctVariable) {
-        this.addLocalProblem(
-            ctVariable,
-            new LocalizedMessage(
-                key,
-                Map.of(
-                    "name", ctVariable.getSimpleName(),
-                    "value", ctVariable.getDefaultExpression().prettyprint()
-                )
-            ),
-            ProblemType.MEANINGLESS_CONSTANT_NAME
-        );
-    }
-
     @Override
     protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
         staticAnalysis.processWith(new AbstractProcessor<CtField<?>>() {
@@ -189,9 +174,29 @@ public void process(CtField<?> field) {
 
                 if (literal.getValue() instanceof Integer v1 && isNonDescriptiveIntegerName(fieldName, v1)
                     || literal.getValue() instanceof String v2 && isNonDescriptiveStringName(fieldName, v2)) {
-                    reportProblem("constants-name-exp", field);
+                    addLocalProblem(
+                        field,
+                        new LocalizedMessage(
+                            "constants-name-exp",
+                            Map.of(
+                                "name", field.getSimpleName(),
+                                "value", field.getDefaultExpression().prettyprint()
+                            )
+                        ),
+                        ProblemType.MEANINGLESS_CONSTANT_NAME
+                    );
                 } else if (containsValueInName(fieldName, literal)) {
-                    reportProblem("constants-name-exp-value", field);
+                    addLocalProblem(
+                        field,
+                        new LocalizedMessage(
+                            "constants-name-exp-value",
+                            Map.of(
+                                "name", field.getSimpleName(),
+                                "value", field.getDefaultExpression().prettyprint()
+                            )
+                        ),
+                        ProblemType.CONSTANT_NAME_CONTAINS_VALUE
+                    );
                 }
             }
         });

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java
@@ -1172,7 +1172,6 @@ public static <T> List<CtElement> findUsesOf(CtExecutable<T> ctExecutable) {
         return SpoonUtil.findUses(ctExecutable);
     }
 
-
     public static List<CtElement> findUses(CtElement ctElement) {
         return new ArrayList<>(ctElement.getFactory().getModel().getElements(new UsesFilter(ctElement)));
     }

diff --git a/sample_config.yaml b/sample_config.yaml
@@ -147,3 +147,4 @@
 - OBJECT_DATATYPE
 - COMMON_REIMPLEMENTATION_SQRT
 - COMMON_REIMPLEMENTATION_HYPOT
+- CONSTANT_NAME_CONTAINS_VALUE
-Original file line number
+Diff line change
@@ Expand Up @@
             return SpoonUtil.findUses(ctExecutable);
         }
         public static List<CtElement> findUses(CtElement ctElement) {
             return new ArrayList<>(ctElement.getFactory().getModel().getElements(new UsesFilter(ctElement)));
         }
@@ Expand Down @@