detect uses of Object as type #365

autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java

@@ -147,5 +147,6 @@ public enum ProblemType {
     UNNECESSARY_BOXING,
     AVOID_STRING_CONCAT,
     UNNECESSARY_COMMENT,
+    OBJECT_DATATYPE,
     REDUNDANT_UNINITIALIZED_VARIABLE
 }

autograder-core/src/main/java/de/firemage/autograder/core/check/general/ObjectDatatype.java

@@ -0,0 +1,49 @@
+package de.firemage.autograder.core.check.general;
+
+import de.firemage.autograder.core.LocalizedMessage;
+import de.firemage.autograder.core.ProblemType;
+import de.firemage.autograder.core.check.ExecutableCheck;
+import de.firemage.autograder.core.dynamic.DynamicAnalysis;
+import de.firemage.autograder.core.integrated.IntegratedCheck;
+import de.firemage.autograder.core.integrated.SpoonUtil;
+import de.firemage.autograder.core.integrated.StaticAnalysis;
+import spoon.processing.AbstractProcessor;
+import spoon.reflect.declaration.CtVariable;
+import spoon.reflect.reference.CtTypeReference;
+
+import java.util.Map;
+
+@ExecutableCheck(reportedProblems = { ProblemType.OBJECT_DATATYPE })
+public class ObjectDatatype extends IntegratedCheck {
+    private static boolean hasObjectType(CtTypeReference<?> ctTypeReference) {
+        return !ctTypeReference.isGenerics() && SpoonUtil.isTypeEqualTo(ctTypeReference, java.lang.Object.class)
+            || ctTypeReference.getActualTypeArguments().stream().anyMatch(ObjectDatatype::hasObjectType);
+    }
+
+    @Override
+    protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
+        staticAnalysis.processWith(new AbstractProcessor<CtVariable<?>>() {
+            @Override
+            public void process(CtVariable<?> ctVariable) {
+                if (ctVariable.isImplicit() || !ctVariable.getPosition().isValidPosition()) {
+                    return;
+                }
+
+                if (SpoonUtil.isInOverriddenMethod(ctVariable) || ctVariable.getType().isArray()) {
+                    return;
+                }
+
+                if (hasObjectType(ctVariable.getType())) {
+                    addLocalProblem(
+                        ctVariable,
+                        new LocalizedMessage(
+                            "object-datatype",
+                            Map.of("variable", ctVariable.getSimpleName())
+                        ),
+                        ProblemType.OBJECT_DATATYPE
+                    );
+                }
+            }
+        });
+    }
+}

autograder-core/src/main/resources/strings.de.ftl

@@ -187,6 +187,8 @@ merge-nested-if = Die Verschachtelte if kann mit der äußeren if kombiniert wer
 
 binary-operator-on-boolean = Statt '|' und '&' sollte man '||' und '&&' verwenden.
 
+object-datatype = Statt dem Datentyp 'Object', sollte die Variable '{$variable}' einen konkreten oder generischen Datentyp haben.
+
 # Naming
 bool-getter-name = Für boolean getter bietet es sich an ein Verb als Präfix zu verwenden. Zum Beispiel '{$newName}' statt '{$oldName}'.
 

autograder-core/src/main/resources/strings.en.ftl

@@ -187,6 +187,9 @@ avoid-recompiling-regex = The constant is only used with 'Pattern.compile' or 'P
 
 binary-operator-on-boolean = Instead of '|' and '&' one should use '||' and '&&'.
 
+object-datatype = Instead of the datatype 'Object', the variable '{$variable}' should have a concrete or generic datatype.
+
+
 # Naming
 bool-getter-name = For boolean getters it is recommended to use a verb as a prefix. For example '{$newName}' instead of '{$oldName}'.
 

autograder-core/src/test/java/de/firemage/autograder/core/check/general/TestObjectDatatype.java

@@ -0,0 +1,129 @@
+package de.firemage.autograder.core.check.general;
+
+import de.firemage.autograder.core.LinterException;
+import de.firemage.autograder.core.LocalizedMessage;
+import de.firemage.autograder.core.Problem;
+import de.firemage.autograder.core.ProblemType;
+import de.firemage.autograder.core.check.AbstractCheckTest;
+import de.firemage.autograder.core.compiler.JavaVersion;
+import de.firemage.autograder.core.file.StringSourceInfo;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class TestObjectDatatype extends AbstractCheckTest {
+    private static final List<ProblemType> PROBLEM_TYPES = List.of(ProblemType.OBJECT_DATATYPE);
+
+    void assertHasObject(Problem problem, String variable) {
+        assertEquals(
+            this.linter.translateMessage(new LocalizedMessage(
+                "object-datatype",
+                Map.of("variable", variable)
+            )),
+            this.linter.translateMessage(problem.getExplanation())
+        );
+    }
+
+    @Test
+    void testObjectVariable() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+            class Test {
+                Object field;
+
+                void method() {
+                    Object local = "abc";
+                }
+            }
+            """
+        ), PROBLEM_TYPES);
+
+        assertHasObject(problems.next(), "field");
+        assertHasObject(problems.next(), "local");
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testObjectVariableArray() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+            class Test {
+                Object[] field1;
+                Object[][] field2;
+
+                void method() {
+                    Object[][] local = null;
+                }
+            }
+            """
+        ), PROBLEM_TYPES);
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testObjectEquals() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+            class Test {
+                @Override
+                public boolean equals(Object o) {
+                    return false;
+                }
+            }
+            """
+        ), PROBLEM_TYPES);
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testGenerics() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+            import java.util.List;
+
+            class Test<T> {
+                List<T> list;
+                List<?> erased;
+                T field;
+                
+                List<Object> objects;
+            }
+            """
+        ), PROBLEM_TYPES);
+
+        assertHasObject(problems.next(), "objects");
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testRawTypes() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+            import java.util.List;
+
+            class Test {
+                List list;
+            }
+            """
+        ), PROBLEM_TYPES);
+
+        problems.assertExhausted();
+    }
+}

sample_config.yaml

@@ -144,3 +144,4 @@
 - AVOID_STATIC_BLOCKS
 - AVOID_STRING_CONCAT
 - UNNECESSARY_COMMENT
+- OBJECT_DATATYPE