Merge pull request #290 from lucanonym/tooFewPackages

Too few packages
Feuermagier · Oct 17, 2023 · 56632ac · 56632ac
2 parents e304f77 + 391a7f8
commit 56632ac
Show file tree

Hide file tree

Showing 7 changed files with 142 additions and 1 deletion.
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
@@ -63,6 +63,7 @@ public enum ProblemType {
     STATIC_METHOD_IN_INTERFACE,
     DO_NOT_USE_RAW_TYPES,
     DUPLICATE_CODE,
+    TOO_FEW_PACKAGES,
     REASSIGNED_PARAMETER,
     DOUBLE_BRACE_INITIALIZATION,
     INSTANCE_FIELD_CAN_BE_LOCAL,

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/OldCollectionCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/OldCollectionCheck.java
@@ -30,7 +30,7 @@ public void process(CtConstructorCall ctConstructorCall) {
                 switch (ctConstructorCall.getType().getQualifiedName()) {
                     case "java.util.Vector" -> reportProblem(ctConstructorCall, "Vector", "ArrayList");
                     case "java.util.Hashtable" -> reportProblem(ctConstructorCall, "Hashtable", "HashMap");
-                    case "java.util.Stack" -> reportProblem(ctConstructorCall, "Stack", "Dequeue");
+                    case "java.util.Stack" -> reportProblem(ctConstructorCall, "Stack", "Deque");
                 }
             }
         });

diff --git a/...r-core/src/main/java/de/firemage/autograder/core/check/structure/TooFewPackagesCheck.java b/...r-core/src/main/java/de/firemage/autograder/core/check/structure/TooFewPackagesCheck.java
@@ -0,0 +1,42 @@
+package de.firemage.autograder.core.check.structure;
+
+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.StaticAnalysis;
+import spoon.reflect.declaration.CtPackage;
+import spoon.reflect.declaration.CtType;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+@ExecutableCheck(reportedProblems = {ProblemType.TOO_FEW_PACKAGES})
+public class TooFewPackagesCheck extends IntegratedCheck {
+    public static final int MAX_CLASSES_PER_PACKAGE = 8;
+    public static final String LOCALIZED_MESSAGE_KEY = "too-few-packages";
+
+    @Override
+    protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
+        List<CtPackage> packages = staticAnalysis.getModel().getAllPackages()
+                .stream()
+                .filter(CtPackage::hasTypes)
+                .toList();
+
+        if (packages.size() == 1
+                && packages.stream().anyMatch(ctPackage -> ctPackage.getTypes().size() > MAX_CLASSES_PER_PACKAGE)) {
+
+            Optional<CtType<?>> ctType = packages.get(0).getTypes().stream().findFirst();
+            ctType.ifPresent(type ->
+                this.addLocalProblem(
+                        type,
+                        new LocalizedMessage(LOCALIZED_MESSAGE_KEY, Map.of("max", MAX_CLASSES_PER_PACKAGE)),
+                        ProblemType.TOO_FEW_PACKAGES
+                ));
+        }
+    }
+
+
+}
diff --git a/autograder-core/src/main/resources/strings.de.ftl b/autograder-core/src/main/resources/strings.de.ftl
@@ -253,6 +253,7 @@ should-be-interface = Die Klasse hat nur Methoden und keine Felder. Statt Vererb
 # Structure
 
 default-package = Das default-Paket sollte nicht verwendet werden. Die folgenden Klassen sind im default-Paket: {$positions}
+too-few-packages = Das Projekt hat mehr als {$max} Klassen, aber nur ein Paket. Du solltest dir eine sinnvolle Einteilung der Klassen in mehrere Pakete überlegen (bspw. nach den Kriterien commands, logic, ui, ...).
 
 # Unnecessary
 

diff --git a/autograder-core/src/main/resources/strings.en.ftl b/autograder-core/src/main/resources/strings.en.ftl
@@ -253,6 +253,7 @@ should-be-interface = The parent class has only methods without fields. Instead
 # Structure
 
 default-package = The default-package should not be used. The following classes are in the default-package: {$positions}
+too-few-packages = The project has more than {$max} classes, but only one package is used. You should think about a better classification for the different classes to distribute the classes over multiple packages (e.g. commands, logic, ui, ...).
 
 # Unnecessary
 empty-block = Empty blocks should be removed or have a comment explaining why they are empty.

diff --git a/...re/src/test/java/de/firemage/autograder/core/check/structure/TestTooFewPackagesCheck.java b/...re/src/test/java/de/firemage/autograder/core/check/structure/TestTooFewPackagesCheck.java
@@ -0,0 +1,95 @@
+package de.firemage.autograder.core.check.structure;
+
+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.*;
+
+class TestTooFewPackagesCheck extends AbstractCheckTest {
+    private static final String LOCALIZED_MESSAGE_KEY = "too-few-packages";
+
+
+    void assertEqualsTooFewPackages(Problem problem) {
+        assertEquals(ProblemType.TOO_FEW_PACKAGES, problem.getProblemType());
+        assertEquals(
+                this.linter.translateMessage(new LocalizedMessage(
+                        LOCALIZED_MESSAGE_KEY,
+                        Map.of("max", TooFewPackagesCheck.MAX_CLASSES_PER_PACKAGE))),
+                this.linter.translateMessage(problem.getExplanation())
+        );
+        assertEquals(ProblemType.TOO_FEW_PACKAGES, problem.getProblemType(), "Wrong problem type");
+    }
+    @Test
+    void test() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+                JavaVersion.JAVA_17,
+                Map.ofEntries(
+                        dummySourceEntry("edu.kit", "First"),
+                        dummySourceEntry("edu.kit", "Second"),
+                        dummySourceEntry("edu.kit", "Third"),
+                        dummySourceEntry("edu.kit", "Fourth"),
+                        dummySourceEntry("edu.kit", "Fifth"),
+                        dummySourceEntry("edu.kit", "Sixth"),
+                        dummySourceEntry("edu.kit", "Seventh"),
+                        dummySourceEntry("edu.kit", "Eighth"),
+                        dummySourceEntry("edu.kit", "Ninth")
+                        )
+        ), List.of(ProblemType.TOO_FEW_PACKAGES));
+
+        assertTrue(problems.hasNext(), "At least one problem should be reported");
+        assertEqualsTooFewPackages(problems.next());
+        problems.assertExhausted();
+
+    }
+
+    // if there are multiple packages, the check should not be triggered
+    @Test
+    void testWithMultiplePackages() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+                JavaVersion.JAVA_17,
+                Map.ofEntries(
+                        dummySourceEntry("edu.kit", "First"),
+                        dummySourceEntry("edu.kit", "Second"),
+                        dummySourceEntry("edu.kit", "Third"),
+                        dummySourceEntry("edu.kit", "Fourth"),
+                        dummySourceEntry("edu.kit", "Fifth"),
+                        dummySourceEntry("edu.kit", "Sixth"),
+                        dummySourceEntry("edu.kit", "Seventh"),
+                        dummySourceEntry("edu.kit", "Eighth"),
+                        dummySourceEntry("edu.kit", "Ninth"),
+                        dummySourceEntry("test", "Test")
+                        )
+        ), List.of(ProblemType.TOO_FEW_PACKAGES));
+        problems.assertExhausted();
+    }
+    @Test
+    void testWithAllowedNumberOfClasses() throws IOException, LinterException {
+         ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+                JavaVersion.JAVA_17,
+                Map.ofEntries(
+                        dummySourceEntry("edu.kit", "First"),
+                        dummySourceEntry("edu.kit", "Second"),
+                        dummySourceEntry("edu.kit", "Third"),
+                        dummySourceEntry("edu.kit", "Fourth"),
+                        dummySourceEntry("edu.kit", "Fifth"),
+                        dummySourceEntry("edu.kit", "Sixth"),
+                        dummySourceEntry("edu.kit", "Seventh"),
+                        dummySourceEntry("edu.kit", "Eighth")
+                        )
+        ), List.of(ProblemType.TOO_FEW_PACKAGES));
+        problems.assertExhausted();
+
+
+    }
+}
+
diff --git a/sample_config.yaml b/sample_config.yaml
@@ -139,3 +139,4 @@
 - COMMON_REIMPLEMENTATION_SUBLIST
 - REDUNDANT_MODIFIER_VISIBILITY_ENUM_CONSTRUCTOR
 - TOO_MANY_EXCEPTIONS
+- TOO_FEW_PACKAGES