Merge pull request #251 from Luro02/main

Improve evaluation of expressions

autograder-cmd/src/main/java/de/firemage/autograder/cmd/Application.java

@@ -28,11 +28,13 @@
 import picocli.CommandLine.Parameters;
 import picocli.CommandLine.Spec;
 
+import java.io.Console;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -93,8 +95,12 @@ public Application(TempLocation tempLocation) {
         this.tempLocation = tempLocation;
     }
 
+    private static Charset getConsoleCharset() {
+        return System.console() == null ? StandardCharsets.UTF_8 : System.console().charset();
+    }
+
     public static void main(String... args) {
-        System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out), true, StandardCharsets.UTF_8));
+        System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out), true, getConsoleCharset()));
         int exitCode = runApplication(args);
         System.exit(exitCode);
     }
@@ -120,6 +126,59 @@ private static Highlight highlightFromCodePosition(CodePosition codePosition, St
         );
     }
 
+    private void execute(
+        Linter linter,
+        List<ProblemType> checks,
+        UploadedFile uploadedFile,
+        Consumer<LinterStatus> statusConsumer
+    ) throws LinterException, IOException {
+        if (outputJson) {
+            List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
+            System.out.println(">> Problems <<");
+            printProblemsAsJson(problems, linter);
+            return;
+        }
+
+        if (isPrettyOutput) {
+            CmdUtil.beginSection("Checks");
+            ProgressAnimation progress = new ProgressAnimation("Checking...");
+            progress.start();
+            List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
+            progress.finish("Completed checks");
+
+            if (problems.isEmpty()) {
+                CmdUtil.println("No problems found - good job!");
+            } else {
+                CmdUtil.println("Found " + problems.size() + " problem(s):");
+                problems.stream()
+                    .map(problem -> {
+                        CodePosition position = problem.getPosition();
+                        Text sourceText = Text.fromString(0, position.readString());
+                        Formatter formatter = new Formatter(
+                            System.lineSeparator(),
+                            highlightFromCodePosition(position, linter.translateMessage(problem.getExplanation()))
+                        );
+
+                        return formatter.render(sourceText);
+                    })
+                    .forEach(string -> CmdUtil.println(string + System.lineSeparator()));
+            }
+
+            CmdUtil.endSection();
+            return;
+        }
+
+        CmdUtil.beginSection("Checks");
+        ProgressAnimation progress = new ProgressAnimation("Checking...");
+        progress.start();
+        List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
+        progress.finish("Completed checks");
+
+        printProblems(problems, linter);
+
+        CmdUtil.endSection();
+    }
+
     @Override
     public Integer call() {
         if (!JavaVersion.isValidJavaVersion(javaVersion)) {
@@ -178,48 +237,7 @@ public Integer call() {
                 this.tempLocation,
                 statusConsumer,
             null)) {
-
-            if (outputJson) {
-                List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
-                System.out.println(">> Problems <<");
-                printProblemsAsJson(problems, linter);
-            } else if (isPrettyOutput) {
-                CmdUtil.beginSection("Checks");
-                ProgressAnimation progress = new ProgressAnimation("Checking...");
-                progress.start();
-                List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
-                progress.finish("Completed checks");
-
-                if (problems.isEmpty()) {
-                    CmdUtil.println("No problems found - good job!");
-                } else {
-                    CmdUtil.println("Found " + problems.size() + " problem(s):");
-                    problems.stream()
-                        .map(problem -> {
-                            CodePosition position = problem.getPosition();
-                            Text sourceText = Text.fromString(0, position.readString());
-                            Formatter formatter = new Formatter(
-                                System.lineSeparator(),
-                                highlightFromCodePosition(position, linter.translateMessage(problem.getExplanation()))
-                            );
-
-                            return formatter.render(sourceText);
-                        })
-                        .forEach(string -> CmdUtil.println(string + System.lineSeparator()));
-                }
-
-                CmdUtil.endSection();
-            } else {
-                CmdUtil.beginSection("Checks");
-                ProgressAnimation progress = new ProgressAnimation("Checking...");
-                progress.start();
-                List<Problem> problems = linter.checkFile(uploadedFile, tests, checks, statusConsumer);
-                progress.finish("Completed checks");
-
-                printProblems(problems, linter);
-
-                CmdUtil.endSection();
-            }
+            this.execute(linter, checks, uploadedFile, statusConsumer);
         } catch (CompilationFailureException e) {
             CmdUtil.printlnErr("Compilation failed: " + e.getMessage());
             return COMPILATION_EXIT_CODE;

autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java

@@ -43,40 +43,40 @@ default CtExpression<R> suggest(CtExpression<T> ctExpression) {
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
-                SpoonUtil.castExpression(ctExpression, int.class)
+                SpoonUtil.castExpression(int.class, ctExpression)
             ),
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isLowerCase",
-                SpoonUtil.castExpression(ctExpression, char.class)
+                SpoonUtil.castExpression(char.class, ctExpression)
             ),
             BinaryOperatorKind.AND
         ),
         Range.between('A', 'Z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
-                SpoonUtil.castExpression(ctExpression, int.class)
+                SpoonUtil.castExpression(int.class, ctExpression)
             ),
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isUpperCase",
-                SpoonUtil.castExpression(ctExpression, char.class)
+                SpoonUtil.castExpression(char.class, ctExpression)
             ),
             BinaryOperatorKind.AND
         ),
         Range.between('0', '9'), (factory, ctExpression, targetType) -> SpoonUtil.createStaticInvocation(
             targetType,
             "isDigit",
-            SpoonUtil.castExpression(ctExpression, char.class)
+            SpoonUtil.castExpression(char.class, ctExpression)
         )
     );
 
     private static Optional<CtExpression<Boolean>> makeSuggestion(CtExpression<Character> ctExpression, Range<Character> range) {
         return Optional.ofNullable(MAPPING.get(range)).map(fn -> fn.suggest(
             ctExpression.getFactory(),
             ctExpression,
-            ctExpression.getFactory().Type().createReference(java.lang.Character.class)
+            ctExpression.getFactory().Type().CHARACTER
         ));
     }
 

autograder-core/src/main/java/de/firemage/autograder/core/check/api/StringIsEmptyReimplementationCheck.java

@@ -55,7 +55,7 @@ private boolean equalsWithEmptyString(CtInvocation<?> invocation) {
         CtExecutableReference<?> executable = invocation.getExecutable();
         return executable.getSignature().equals("equals(java.lang.Object)")
             && (SpoonUtil.isStringLiteral(invocation.getArguments().get(0), "")
-             // detect "".equals(s)
+            // detect "".equals(s)
             || SpoonUtil.isStringLiteral(invocation.getTarget(), ""));
     }
 

autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/UseOperatorAssignment.java

@@ -5,8 +5,8 @@
 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.PrintUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
+import de.firemage.autograder.core.integrated.evaluator.OperatorHelper;
 import spoon.processing.AbstractProcessor;
 import spoon.reflect.code.BinaryOperatorKind;
 import spoon.reflect.code.CtAssignment;
@@ -83,12 +83,12 @@ public void process(CtAssignment<?, ?> assignment) {
                 String simplifiedExpr = null;
                 if (left.toString().equals(lhs.toString())) {
                     // left hand side is the same, so we can use an operator assignment
-                    simplifiedExpr = "%s %s= %s".formatted(lhs, PrintUtil.printOperator(operator), right);
+                    simplifiedExpr = "%s %s= %s".formatted(lhs, OperatorHelper.getOperatorText(operator), right);
                 } else if (isCommutative(operator) && isCommutativeType(ctBinaryOperator) && right.toString().equals(lhs.toString())) {
                     // operator is commutative so <lhs> = <left> <op> <right> is equivalent to
                     // <lhs> = <right> <op> <left>
 
-                    simplifiedExpr = "%s %s= %s".formatted(lhs, PrintUtil.printOperator(operator), left);
+                    simplifiedExpr = "%s %s= %s".formatted(lhs, OperatorHelper.getOperatorText(operator), left);
                 }
 
                 if (simplifiedExpr != null) {

autograder-core/src/main/java/de/firemage/autograder/core/check/oop/ClosedSetOfValues.java

@@ -221,7 +221,7 @@ public <T, S> void visitCtSwitchExpression(CtSwitchExpression<T, S> switchExpres
 
             @Override
             public <T> void visitCtField(CtField<T> ctField) {
-                if (!SpoonUtil.isEffectivelyFinal(staticAnalysis, ctField.getReference())) {
+                if (!SpoonUtil.isEffectivelyFinal(ctField.getReference())) {
                     return;
                 }
 

autograder-core/src/main/java/de/firemage/autograder/core/check/oop/MutableEnum.java

@@ -18,13 +18,12 @@ public class MutableEnum extends IntegratedCheck {
      * Tries to detect if the provided type is mutable. It is very difficult to detect every possible
      * case of mutability, therefore this method might return false negatives.
      *
-     * @param staticAnalysis the static analysis
      * @param ctType the type to check
      * @return true if it is mutable, false if mutability cannot be determined
      */
-    private static boolean isMutable(StaticAnalysis staticAnalysis, CtType<?> ctType) {
+    private static boolean isMutable(CtType<?> ctType) {
         for (CtField<?> ctField : ctType.getFields()) {
-            if (!SpoonUtil.isEffectivelyFinal(staticAnalysis, ctField.getReference())) {
+            if (!SpoonUtil.isEffectivelyFinal(ctField.getReference())) {
                 return true;
             }
         }
@@ -37,7 +36,7 @@ protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnaly
         staticAnalysis.processWith(new AbstractProcessor<CtEnum<?>>() {
             @Override
             public void process(CtEnum<?> ctEnum) {
-                if (isMutable(staticAnalysis, ctEnum)) {
+                if (isMutable(ctEnum)) {
                     addLocalProblem(
                         ctEnum,
                         new LocalizedMessage("mutable-enum"),

autograder-core/src/main/java/de/firemage/autograder/core/check/oop/StaticFieldShouldBeInstanceCheck.java

@@ -23,7 +23,7 @@ public void process(CtField<?> field) {
                     return;
                 }
 
-                if (!SpoonUtil.isEffectivelyFinal(staticAnalysis, field.getReference())) {
+                if (!SpoonUtil.isEffectivelyFinal(field.getReference())) {
                     addLocalProblem(field,
                         new LocalizedMessage("static-field-exp", Map.of("name", field.getSimpleName())),
                         ProblemType.STATIC_FIELD_SHOULD_BE_INSTANCE);

autograder-core/src/main/java/de/firemage/autograder/core/check/oop/UtilityClassCheck.java

@@ -19,7 +19,7 @@
     ProblemType.UTILITY_CLASS_ABSTRACT
 })
 public class UtilityClassCheck extends IntegratedCheck {
-    public static boolean isUtilityClass(StaticAnalysis staticAnalysis, CtClass<?> ctClass) {
+    public static boolean isUtilityClass(CtClass<?> ctClass) {
         return
             // it must obviously be a class
             ctClass.isClass()
@@ -31,7 +31,7 @@ public static boolean isUtilityClass(StaticAnalysis staticAnalysis, CtClass<?> c
                 && ctClass.getMethods().stream().allMatch(CtMethod::isStatic)
                 // all fields should be static and effectively final (no assignments)
                 && ctClass.getFields().stream().allMatch(
-                    ctField -> ctField.isStatic() && SpoonUtil.isEffectivelyFinal(staticAnalysis, ctField.getReference())
+                    ctField -> ctField.isStatic() && SpoonUtil.isEffectivelyFinal(ctField.getReference())
                 )
                 // the class should not extend anything
                 && ctClass.getSuperclass() == null
@@ -72,7 +72,7 @@ protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnaly
             @Override
             public void process(CtClass<?> ctClass) {
                 // ignore everything that is not a utility class
-                if (!isUtilityClass(staticAnalysis, ctClass)) {
+                if (!isUtilityClass(ctClass)) {
                     return;
                 }