enable exception message check for any exception Feuermagier#434

autograder-core/src/main/java/de/firemage/autograder/core/check/exceptions/ExceptionMessageCheck.java

@@ -4,41 +4,57 @@
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
 
-import de.firemage.autograder.core.integrated.ExceptionUtil;
 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.code.CtCase;
 import spoon.reflect.code.CtConstructorCall;
 import spoon.reflect.code.CtExpression;
+import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtThrow;
 import spoon.reflect.declaration.CtConstructor;
 import spoon.reflect.declaration.CtElement;
 
-import java.util.List;
-
 @ExecutableCheck(reportedProblems = ProblemType.EXCEPTION_WITHOUT_MESSAGE)
 public class ExceptionMessageCheck extends IntegratedCheck {
     private static boolean isExceptionWithoutMessage(CtExpression<?> expression) {
-        return expression instanceof CtConstructorCall<?> ctorCall
-            && ExceptionUtil.isRuntimeException(ctorCall.getType())
-            && !hasMessage(ctorCall.getArguments());
-    }
+        if (!(expression instanceof CtConstructorCall<?> ctConstructorCall) ||
+            !(SpoonUtil.isSubtypeOf(expression.getType(), java.lang.Exception.class))) {
+            return false;
+        }
 
-    private static boolean hasMessage(List<? extends CtExpression<?>> arguments) {
-        if (arguments.isEmpty()) {
+        // check if the invoked constructor passes a message to the exception
+        if (ctConstructorCall.getExecutable().getExecutableDeclaration() instanceof CtConstructor<?> ctConstructor
+            && ctConstructor.getBody().filterChildren(ctElement -> ctElement instanceof CtInvocation<?> ctInvocation
+                // we just check if there is any invocation with a message, because this is easier and might be enough
+                // for most cases.
+                //
+                // this way will not result in false positives, only in false negatives
+                && ctInvocation.getExecutable().isConstructor()
+                && hasMessage(ctInvocation.getArguments())
+            ).first() != null) {
             return false;
         }
 
-        CtExpression<?> ctExpression = arguments.get(0);
-        String literal = SpoonUtil.tryGetStringLiteral(ctExpression).orElse(null);
+        return !hasMessage(ctConstructorCall.getArguments());
+    }
 
-        if (literal != null) {
-            return !literal.isBlank();
+    private static boolean hasMessage(Iterable<? extends CtExpression<?>> arguments) {
+        for (CtExpression<?> ctExpression : arguments) {
+            // consider a passed throwable as having message
+            if (SpoonUtil.isSubtypeOf(ctExpression.getType(), java.lang.Throwable.class)) {
+                return true;
+            }
+
+            String literal = SpoonUtil.tryGetStringLiteral(ctExpression).orElse(null);
+
+            if (literal != null) {
+                return !literal.isBlank();
+            }
         }
 
-        return true;
+        return false;
     }
 
     private static boolean isInAllowedContext(CtElement ctElement) {

autograder-core/src/test/java/de/firemage/autograder/core/check/exceptions/TestExceptionMessageCheck.java

@@ -11,6 +11,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -164,4 +165,132 @@ public String[] readFile() {
 
         problems.assertExhausted();
     }
+
+    @Test
+    void testExceptionFactoryMethod() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+            JavaVersion.JAVA_17,
+            Map.ofEntries(
+                Map.entry(
+                    "MyException",
+                    """
+                        public class MyException extends Exception {
+                            public static Exception create() {
+                                return new MyException();
+                            }
+                        }
+                        """
+                ),
+                Map.entry(
+                    "Main",
+                    """
+                        public class Main {
+                            public static void main(String[] args) throws Exception {
+                                throw MyException.create(); /*# ok #*/
+                            }
+                        }
+                    """
+                )
+            )
+        ), PROBLEM_TYPES);
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testCustomExceptionNoMessage() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+            JavaVersion.JAVA_17,
+            Map.ofEntries(
+                Map.entry(
+                    "MyException",
+                    """
+                        public class MyException extends Exception {
+                            public MyException() {
+                                super();
+                            }
+                        }
+                        """
+                ),
+                Map.entry(
+                    "Main",
+                    """
+                        public class Main {
+                            public static void main(String[] args) throws Exception {
+                                throw new MyException(); /*# not ok #*/
+                            }
+                        }
+                    """
+                )
+            )
+        ), PROBLEM_TYPES);
+
+        assertMissingMessage(problems.next());
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testCustomExceptionInvalidMessage() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+            JavaVersion.JAVA_17,
+            Map.ofEntries(
+                Map.entry(
+                    "MyException",
+                    """
+                        public class MyException extends Exception {
+                            public MyException() {
+                                super(" ");
+                            }
+                        }
+                        """
+                ),
+                Map.entry(
+                    "Main",
+                    """
+                        public class Main {
+                            public static void main(String[] args) throws Exception {
+                                throw new MyException(); /*# not ok #*/
+                            }
+                        }
+                    """
+                )
+            )
+        ), PROBLEM_TYPES);
+
+        assertMissingMessage(problems.next());
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testCustomExceptionValidMessage() throws IOException, LinterException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
+            JavaVersion.JAVA_17,
+            Map.ofEntries(
+                Map.entry(
+                    "MyException",
+                    """
+                        public class MyException extends Exception {
+                            public MyException() {
+                                super("failed to start the application");
+                            }
+                        }
+                        """
+                ),
+                Map.entry(
+                    "Main",
+                    """
+                        public class Main {
+                            public static void main(String[] args) throws Exception {
+                                throw new MyException(); /*# ok #*/
+                            }
+                        }
+                    """
+                )
+            )
+        ), PROBLEM_TYPES);
+
+        problems.assertExhausted();
+    }
 }