Merge pull request #671 from Fenmore/main

improve commented out code check with JavaParser
Feuermagier · Jan 11, 2025 · 8ed1456 · 8ed1456
2 parents 5a8d07f + 0d95290
commit 8ed1456
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 3 deletions.
diff --git a/autograder-core/pom.xml b/autograder-core/pom.xml
@@ -76,6 +76,13 @@
             <groupId>org.reflections</groupId>
             <artifactId>reflections</artifactId>
         </dependency>
+
+        <!-- JavaParser -->
+        <dependency>
+            <groupId>com.github.javaparser</groupId>
+            <artifactId>javaparser-core</artifactId>
+            <version>${javaparser.version}</version>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/...r-core/src/main/java/de/firemage/autograder/core/check/comment/CommentedOutCodeCheck.java b/...r-core/src/main/java/de/firemage/autograder/core/check/comment/CommentedOutCodeCheck.java
@@ -1,11 +1,12 @@
 package de.firemage.autograder.core.check.comment;
 
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.CodePosition;
 import de.firemage.autograder.core.LocalizedMessage;
 import de.firemage.autograder.core.ProblemType;
-import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.check.ExecutableCheck;
-
 import de.firemage.autograder.core.file.SourcePath;
 import de.firemage.autograder.core.integrated.IntegratedCheck;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
@@ -15,8 +16,11 @@
 import spoon.reflect.cu.SourcePosition;
 
 import java.nio.file.Path;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
@@ -44,7 +48,7 @@ public void process(CtComment comment) {
                 }
                 String content = comment.getContent().trim();
 
-                if (StringUtils.containsAny(content, ';', '{', '}', '=')) {
+                if (isValidCode(content)) {
                     var position = comment.getPosition();
                     files
                         .computeIfAbsent(position.getFile().toPath(), path -> new TreeSet<>(POSITION_COMPARATOR))
@@ -81,6 +85,59 @@ public void process(CtComment comment) {
         });
     }
 
+    private static boolean isValidCode(String content) {
+        return !content.isEmpty() && containsSpecialCharacters(content) && isValidCodeInternal(content);
+    }
+
+    private static boolean containsSpecialCharacters(String content) {
+        return StringUtils.containsAny(content, ';', '{', '}', '=', '(', ')', '-', '<', '>', '?', ':', '+', '.', ',');
+    }
+
+    private static boolean isValidCodeInternal(String content) {
+        return prepareCode(content).stream().anyMatch(formatted -> {
+                    try {
+                        StaticJavaParser.parseBlock(formatted);
+                        return true;
+                    } catch (ParseProblemException e) {
+                        return false;
+                    }
+                });
+    }
+
+    private static List<String> prepareCode(String content) {
+        String stripped = content.strip();
+        return wrapAsBlock(getDifferentVersions(stripped));
+    }
+
+    private static Collection<String> getDifferentVersions(String original) {
+        Set<String> options = new HashSet<>();
+        options.add(original.endsWith(";") ? original : original + ";");
+        options.add(formatBrackets(formatBrackets(formatBrackets(original, '{', '}'), '(', ')'), '[', ']'));
+        return options;
+    }
+
+    private static String formatBrackets(String original, char opening, char closing) {
+        int difference = StringUtils.countMatches(original, closing) - StringUtils.countMatches(original, opening);
+        if (difference == 0) {
+            return original;
+        }
+
+        return difference > 0 
+                ? String.valueOf(opening).repeat(difference) + original 
+                : original + String.valueOf(closing).repeat(-difference);
+    }
+
+    private static List<String> wrapAsBlock(Collection<String> differentVersions) {
+        return differentVersions.stream()
+                .map(content -> {
+                    if (!content.startsWith("{") || !content.endsWith("}")) {
+                        return "{" + content + "}";
+                    }
+                    return content;
+                })
+                .toList();
+    }
+
     private final class RunningPosition {
         int startLine;
         int endLine;

diff --git a/pom.xml b/pom.xml
@@ -57,6 +57,7 @@
         <spoon.version>11.1.1-beta-21</spoon.version>
         <fluent.version>0.70</fluent.version>
         <reflections.version>0.10.2</reflections.version>
+        <javaparser.version>3.26.1</javaparser.version>
 
         <revision>0.5.13</revision>
     </properties>