Merge pull request #474 from Luro02/main

Implement some things
Feuermagier · Apr 6, 2024 · 202de4c · 202de4c
2 parents df1047b + 7252d45
commit 202de4c
Show file tree

Hide file tree

Showing 22 changed files with 421 additions and 105 deletions.
diff --git a/autograder-core/pom.xml b/autograder-core/pom.xml
@@ -15,7 +15,7 @@
     <url>https://github.com/Feuermagier/autograder/autograder-core</url>
 
     <properties>
-        <spoon.version>10.4.2</spoon.version>
+        <spoon.version>11.0.0</spoon.version>
         <pmd.version>7.0.0-rc3</pmd.version>
         <spotbugs.version>6.45.0</spotbugs.version>
 
@@ -77,12 +77,6 @@
             <scope>test</scope>
         </dependency>
 
-        <dependency>
-            <groupId>org.eclipse.jdt</groupId>
-            <artifactId>org.eclipse.jdt.core</artifactId>
-            <version>3.34.0</version>
-        </dependency>
-
         <!-- Spoon -->
         <dependency>
             <groupId>fr.inria.gforge.spoon</groupId>
@@ -93,11 +87,6 @@
                     <groupId>org.slf4j</groupId>
                     <artifactId>slf4j-api</artifactId>
                 </exclusion>
-                <!-- exclude jdt, because spoon is not using the latest version -->
-                <exclusion>
-                    <groupId>org.eclipse.jdt</groupId>
-                    <artifactId>org.eclipse.jdt.core</artifactId>
-                </exclusion>
             </exclusions>
         </dependency>
 
@@ -110,11 +99,6 @@
                     <groupId>org.slf4j</groupId>
                     <artifactId>slf4j-api</artifactId>
                 </exclusion>
-                <!-- exclude jdt, because spoon is not using the latest version -->
-                <exclusion>
-                    <groupId>org.eclipse.jdt</groupId>
-                    <artifactId>org.eclipse.jdt.core</artifactId>
-                </exclusion>
             </exclusions>
         </dependency>
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java
@@ -76,7 +76,7 @@ private static Optional<CtExpression<Boolean>> makeSuggestion(CtExpression<Chara
         return Optional.ofNullable(MAPPING.get(range)).map(fn -> fn.suggest(
             ctExpression.getFactory(),
             ctExpression,
-            ctExpression.getFactory().Type().CHARACTER
+            ctExpression.getFactory().Type().characterType()
         ));
     }
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionAddAll.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionAddAll.java
@@ -39,6 +39,8 @@
     ProblemType.COMMON_REIMPLEMENTATION_ADD_ALL
 })
 public class CollectionAddAll extends IntegratedCheck {
+    private static final int MIN_ADD_ALL_SIZE = 4;
+
     private record AddInvocation(
         CtVariableReference<?> collection,
         CtExecutableReference<?> executableReference,
@@ -117,9 +119,13 @@ private void reportProblem(CtVariable<?> ctVariable, List<? extends CtExpression
             return;
         }
 
+        if (addedValues.size() < MIN_ADD_ALL_SIZE) {
+            return;
+        }
+
 
         addLocalProblem(
-            ctVariable,
+            addedValues.get(0).getParent(CtStatement.class),
             new LocalizedMessage(
                 "common-reimplementation",
                 Map.of(

diff --git a/...ore/src/main/java/de/firemage/autograder/core/check/api/IsEmptyReimplementationCheck.java b/...ore/src/main/java/de/firemage/autograder/core/check/api/IsEmptyReimplementationCheck.java
@@ -40,7 +40,7 @@ private static boolean isSizeCall(CtInvocation<?> ctInvocation) {
             && target.getType().getTypeDeclaration() != null
             // the type with the size method must have an isEmpty method
             && target.getType().getTypeDeclaration().getMethod(
-                ctInvocation.getFactory().Type().BOOLEAN_PRIMITIVE,
+                ctInvocation.getFactory().Type().booleanPrimitiveType(),
                 "isEmpty"
             ) != null
             && SpoonUtil.isSignatureEqualTo(ctInvocation.getExecutable(), int.class, "size");

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseFormatString.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseFormatString.java
@@ -142,15 +142,15 @@ private CtExpression<?> resolveExpression(CtExpression<?> ctExpression) {
             && SpoonUtil.isTypeEqualTo(ctTypeAccess.getAccessedType(), java.lang.System.class)
             && SpoonUtil.isSignatureEqualTo(
                 ctInvocation.getExecutable(),
-                typeFactory.STRING,
+                typeFactory.stringType(),
                 "lineSeparator"
             )) {
-            return SpoonUtil.makeLiteral(typeFactory.STRING, "\n");
+            return SpoonUtil.makeLiteral(typeFactory.stringType(), "\n");
         }
 
         if (ctExpression instanceof CtLiteral<?> ctLiteral
-            && SpoonUtil.areLiteralsEqual(ctLiteral, SpoonUtil.makeLiteral(typeFactory.CHARACTER_PRIMITIVE, '\n'))) {
-            return SpoonUtil.makeLiteral(typeFactory.STRING, "\n");
+            && SpoonUtil.areLiteralsEqual(ctLiteral, SpoonUtil.makeLiteral(typeFactory.characterPrimitiveType(), '\n'))) {
+            return SpoonUtil.makeLiteral(typeFactory.stringType(), "\n");
         }
 
         return ctExpression;

diff --git a/...rc/main/java/de/firemage/autograder/core/check/complexity/RedundantIfForBooleanCheck.java b/...rc/main/java/de/firemage/autograder/core/check/complexity/RedundantIfForBooleanCheck.java
@@ -17,12 +17,16 @@
 import spoon.reflect.code.CtIf;
 import spoon.reflect.code.CtReturn;
 import spoon.reflect.code.CtStatement;
+import spoon.reflect.declaration.CtMethod;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 @ExecutableCheck(reportedProblems = { ProblemType.REDUNDANT_IF_FOR_BOOLEAN })
 public class RedundantIfForBooleanCheck extends IntegratedCheck {
+    private static final Set<String> METHODS_TO_IGNORE = Set.of("equals", "hashCode");
+
     private String makeSuggestion(CtExpression<?> ctExpression, Effect thenEffect, Effect elseEffect) {
         if (thenEffect instanceof AssignmentEffect thenAssignment && elseEffect instanceof AssignmentEffect) {
             return "%s = %s".formatted(
@@ -129,8 +133,15 @@ protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnaly
         staticAnalysis.processWith(new AbstractProcessor<CtBlock<?>>() {
             @Override
             public void process(CtBlock<?> block) {
+                CtMethod<?> parentMethod = block.getParent(CtMethod.class);
+                if (parentMethod != null && METHODS_TO_IGNORE.contains(parentMethod.getSimpleName())) {
+                    return;
+                }
+
                 List<CtStatement> statements = SpoonUtil.getEffectiveStatements(block);
 
+                // TODO: write test
+
                 for (int i = 0; i < statements.size(); i++) {
                     CtStatement statement = statements.get(i);
 

diff --git a/...re/src/main/java/de/firemage/autograder/core/check/complexity/RedundantNegationCheck.java b/...re/src/main/java/de/firemage/autograder/core/check/complexity/RedundantNegationCheck.java
@@ -8,7 +8,10 @@
 import de.firemage.autograder.core.integrated.SpoonUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
 
+import de.firemage.autograder.core.integrated.evaluator.Evaluator;
+import de.firemage.autograder.core.integrated.evaluator.fold.Fold;
 import spoon.processing.AbstractProcessor;
+import spoon.reflect.code.CtBinaryOperator;
 import spoon.reflect.code.CtExpression;
 import spoon.reflect.code.CtUnaryOperator;
 import spoon.reflect.code.UnaryOperatorKind;
@@ -17,31 +20,42 @@
 
 @ExecutableCheck(reportedProblems = { ProblemType.REDUNDANT_NEGATION })
 public class RedundantNegationCheck extends IntegratedCheck {
+    private record NegationFolder() implements Fold {
+        @Override
+        @SuppressWarnings("unchecked")
+        public <T> CtExpression<T> foldCtUnaryOperator(CtUnaryOperator<T> ctUnaryOperator) {
+            // only check negations !(operand)
+            if (ctUnaryOperator.getKind() != UnaryOperatorKind.NOT) {
+                return ctUnaryOperator;
+            }
+
+            CtExpression<?> operand = ctUnaryOperator.getOperand();
+
+            // this negates the operand and optimizes it if possible
+            return (CtExpression<T>) SpoonUtil.negate(operand);
+        }
+    }
+
     @Override
     protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
-        staticAnalysis.processWith(new AbstractProcessor<CtUnaryOperator<?>>() {
+        staticAnalysis.processWith(new AbstractProcessor<CtExpression<?>>() {
             @Override
-            public void process(CtUnaryOperator<?> ctUnaryOperator) {
-                if (ctUnaryOperator.isImplicit() || !ctUnaryOperator.getPosition().isValidPosition()) {
+            public void process(CtExpression<?> ctExpression) {
+                if (ctExpression.isImplicit()
+                    || !ctExpression.getPosition().isValidPosition()
+                    || ctExpression.getParent(CtExpression.class) != null) {
                     return;
                 }
 
-                // only check negations !(operand)
-                if (ctUnaryOperator.getKind() != UnaryOperatorKind.NOT) {
-                    return;
-                }
-
-                CtExpression<?> operand = ctUnaryOperator.getOperand();
-
                 // this negates the operand and optimizes it if possible
-                CtExpression<?> negated = SpoonUtil.negate(operand);
+                CtExpression<?> negated = new Evaluator(new NegationFolder()).evaluate(ctExpression);
                 // if they are equal, the negation is not redundant
-                if (ctUnaryOperator.equals(negated)) {
+                if (ctExpression.equals(negated)) {
                     return;
                 }
 
                 addLocalProblem(
-                    ctUnaryOperator,
+                    ctExpression,
                     new LocalizedMessage(
                         "common-reimplementation",
                         Map.of("suggestion", negated.prettyprint())

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java
@@ -38,7 +38,7 @@
 
 @ExecutableCheck(reportedProblems = {ProblemType.COMPLEX_REGEX})
 public class RegexCheck extends IntegratedCheck {
-    private static final double MAX_ALLOWED_SCORE = 12.0;
+    private static final double MAX_ALLOWED_SCORE = 24.0;
     private static final List<String> REGEX_HINTS = List.of("?", "<", ">", "+", "*", "[", "]", "$", "^", "|", "\\");
     private static final int MIN_REGEX_HINTS = 2;