Merge pull request #2006 from lf-lang/spotbugs-fixes

Cleaned up test code to fix all test related spotbugs warnings

cli/base/src/testFixtures/java/org/lflang/cli/CliToolTestFixture.java

@@ -31,6 +31,7 @@
 
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import org.hamcrest.Matcher;
 import org.opentest4j.AssertionFailedError;
@@ -66,7 +67,11 @@ public ExecutionResult run(Path wd, String... args) {
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     ByteArrayOutputStream err = new ByteArrayOutputStream();
 
-    Io testIo = new Io(new PrintStream(err), new PrintStream(out), wd);
+    Io testIo =
+        new Io(
+            new PrintStream(err, false, StandardCharsets.UTF_8),
+            new PrintStream(out, false, StandardCharsets.UTF_8),
+            wd);
     int exitCode = testIo.fakeSystemExit(io -> runCliProgram(io, args));
 
     return new ExecutionResult(out, err, exitCode);
@@ -82,11 +87,11 @@ public ExecutionResult run(Path wd, String... args) {
   record ExecutionResult(ByteArrayOutputStream out, ByteArrayOutputStream err, int exitCode) {
 
     public String getOut() {
-      return out.toString();
+      return out.toString(StandardCharsets.UTF_8);
     }
 
     public String getErr() {
-      return err.toString();
+      return err.toString(StandardCharsets.UTF_8);
     }
 
     public void checkOk() {
@@ -117,9 +122,9 @@ public void verify(ThrowingConsumer<ExecutionResult> actions) {
         System.out.println("TEST FAILED");
         System.out.println("> Return code: " + exitCode);
         System.out.println("> Standard output -------------------------");
-        System.err.println(out.toString());
+        System.err.println(out.toString(StandardCharsets.UTF_8));
         System.out.println("> Standard error --------------------------");
-        System.err.println(err.toString());
+        System.err.println(err.toString(StandardCharsets.UTF_8));
         System.out.println("> -----------------------------------------");
 
         if (e instanceof Exception) {

cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java

@@ -111,7 +111,10 @@ public TempDirBuilder file(String relativePath, String contents) throws IOExcept
         throw new IllegalArgumentException("Should be a relative path: " + relativePath);
       }
       Path filePath = curDir.resolve(relPath);
-      Files.createDirectories(filePath.getParent());
+      final var parent = filePath.getParent();
+      if (parent != null) {
+        Files.createDirectories(parent);
+      }
       Files.writeString(filePath, contents);
       return this;
     }

cli/lfc/src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt

@@ -38,7 +38,9 @@ import java.nio.file.Paths
 
 class SpyPrintStream {
     private val baout = ByteArrayOutputStream()
-    val ps = PrintStream(baout)
+    private val ps = PrintStream(baout, false, StandardCharsets.UTF_8)
+
+    fun getSpiedErrIo(): Io = Io(err = ps)
 
     override fun toString(): String = baout.toString(StandardCharsets.UTF_8)
 }
@@ -111,7 +113,7 @@ class LfcIssueReportingTest {
 
         val stderr = SpyPrintStream()
 
-        val io = Io(err = stderr.ps)
+        val io = stderr.getSpiedErrIo()
         val backend = ReportingBackend(io, AnsiColors(useColors).bold("lfc: "), AnsiColors(useColors), 2)
         val injector = LFStandaloneSetup(LFRuntimeModule(), LFStandaloneModule(backend, io))
             .createInjectorAndDoEMFRegistration()

core/build.gradle

@@ -44,6 +44,10 @@ dependencies {
     testImplementation "org.opentest4j:opentest4j:$openTest4jVersion"
     testImplementation "org.eclipse.xtext:org.eclipse.xtext.testing:$xtextVersion"
     testImplementation "org.eclipse.xtext:org.eclipse.xtext.xbase.testing:$xtextVersion"
+
+    // For spotbugs annotations
+    compileOnly "com.github.spotbugs:spotbugs-annotations:$spotbugsToolVersion"
+    compileOnly "net.jcip:jcip-annotations:$jcipVersion"
 }
 
 configurations {

core/src/integrationTest/java/org/lflang/tests/RunSingleTest.java

@@ -70,7 +70,7 @@ public void runSingleTest() throws FileNotFoundException {
 
     Class<? extends TestBase> testClass = getTestInstance(target);
 
-    LFTest testCase = new LFTest(target, path.toAbsolutePath());
+    LFTest testCase = new LFTest(path.toAbsolutePath());
 
     TestBase.runSingleTestAndPrintResults(testCase, testClass, TestBase.pathToLevel(path));
   }

core/src/integrationTest/java/org/lflang/tests/lsp/ErrorInserter.java

@@ -4,14 +4,11 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.ListIterator;
-import java.util.Random;
+import java.nio.file.Paths;
+import java.util.*;
 import java.util.function.BiFunction;
 import java.util.function.BiPredicate;
 import java.util.function.Function;
@@ -125,7 +122,7 @@ public void write() throws IOException {
       if (!src.toFile().renameTo(swapFile(src).toFile())) {
         throw new IOException("Failed to create a swap file.");
       }
-      try (PrintWriter writer = new PrintWriter(src.toFile())) {
+      try (PrintWriter writer = new PrintWriter(src.toFile(), StandardCharsets.UTF_8)) {
         lines.forEach(writer::println);
       }
     }
@@ -233,7 +230,13 @@ private void alter(BiFunction<ListIterator<String>, String, Boolean> alterer) {
 
     /** Return the swap file associated with {@code f}. */
     private static Path swapFile(Path p) {
-      return p.getParent().resolve("." + p.getFileName() + ".swp");
+      final var parent = p.getParent();
+      final var swpName = "." + p.getFileName() + ".swp";
+      if (parent == null) {
+        return Paths.get(swpName);
+      } else {
+        return parent.resolve(swpName);
+      }
     }
   }
 
@@ -265,6 +268,10 @@ public boolean hasNext() {
 
         @Override
         public T next() {
+          if (!hasNext()) {
+            throw new NoSuchElementException();
+          }
+
           T ret = current.item;
           current = current.previous;
           return ret;

core/src/integrationTest/java/org/lflang/tests/lsp/LspTests.java

@@ -91,7 +91,7 @@ void typescriptValidationTest() throws IOException {
    */
   private void targetLanguageValidationTest(Target target, ErrorInserter.Builder builder)
       throws IOException {
-    long seed = new Random().nextLong();
+    long seed = System.nanoTime();
     System.out.printf(
         "Running validation tests for %s with random seed %d.%n", target.getDisplayName(), seed);
     Random random = new Random(seed);

core/src/main/java/org/lflang/Target.java

@@ -24,17 +24,15 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import net.jcip.annotations.Immutable;
 import org.lflang.lf.TargetDecl;
 
 /**
- * Enumeration of targets and their associated properties. These classes are written in Java, not
- * Xtend, because the enum implementation in Xtend more primitive. It is safer to use enums rather
- * than string values since it allows faulty references to be caught at compile time. Switch
- * statements that take as input an enum but do not have cases for all members of the enum are also
- * reported by Xtend with a warning message.
+ * Enumeration of targets and their associated properties.
  *
  * @author Marten Lohstroh
  */
+@Immutable
 public enum Target {
   C(
       "C",
@@ -90,8 +88,6 @@ public enum Target {
   CPP(
       "Cpp",
       true,
-      "cpp",
-      "Cpp",
       Arrays.asList(
           // List via: https://en.cppreference.com/w/cpp/keyword
           "alignas", // (since C++11)
@@ -194,8 +190,6 @@ public enum Target {
   TS(
       "TypeScript",
       false,
-      "ts",
-      "TS",
       Arrays.asList(
           // List via: https://github.com/Microsoft/TypeScript/issues/2536
           // Reserved words
@@ -352,8 +346,6 @@ public enum Target {
   Rust(
       "Rust",
       true,
-      "rust",
-      "Rust",
       // In our Rust implementation, the only reserved keywords
       // are those that are a valid expression. Others may be escaped
       // with the syntax r#keyword.
@@ -362,17 +354,11 @@ public enum Target {
   /** String representation of this target. */
   private final String displayName;
 
-  /** Name of package containing Kotlin classes for the target language. */
-  public final String packageName;
-
-  /** Prefix of names of Kotlin classes for the target language. */
-  public final String classNamePrefix;
-
   /** Whether or not this target requires types. */
   public final boolean requiresTypes;
 
   /** Reserved words in the target language. */
-  public final Set<String> keywords;
+  private final Set<String> keywords;
 
   /** An unmodifiable list of all known targets. */
   public static final List<Target> ALL = List.of(Target.values());
@@ -382,26 +368,12 @@ public enum Target {
    *
    * @param displayName String representation of this target.
    * @param requiresTypes Types Whether this target requires type annotations or not.
-   * @param packageName Name of package containing Kotlin classes for the target language.
-   * @param classNamePrefix Prefix of names of Kotlin classes for the target language.
    * @param keywords List of reserved strings in the target language.
    */
-  Target(
-      String displayName,
-      boolean requiresTypes,
-      String packageName,
-      String classNamePrefix,
-      Collection<String> keywords) {
+  Target(String displayName, boolean requiresTypes, Collection<String> keywords) {
     this.displayName = displayName;
     this.requiresTypes = requiresTypes;
     this.keywords = Collections.unmodifiableSet(new LinkedHashSet<>(keywords));
-    this.packageName = packageName;
-    this.classNamePrefix = classNamePrefix;
-  }
-
-  /** Private constructor for targets without packageName and classNamePrefix. */
-  Target(String displayName, boolean requiresTypes, Collection<String> keywords) {
-    this(displayName, requiresTypes, "N/A", "N/A", keywords); // FIXME: prefix
   }
 
   /**
@@ -469,16 +441,10 @@ public boolean supportsInheritance() {
 
   /** Return true if the target supports multiports and banks of reactors. */
   public boolean supportsMultiports() {
-    switch (this) {
-      case C:
-      case CCPP:
-      case CPP:
-      case Python:
-      case Rust:
-      case TS:
-        return true;
-    }
-    return false;
+    return switch (this) {
+      case C, CCPP, CPP, Python, Rust, TS -> true;
+      default -> false;
+    };
   }
 
   /**
@@ -494,15 +460,10 @@ public boolean supportsParameterizedWidths() {
    * this target.
    */
   public boolean supportsReactionDeclarations() {
-    if (this.equals(Target.C) || this.equals(Target.CPP)) return true;
-    else return false;
+    return this.equals(Target.C) || this.equals(Target.CPP);
   }
 
-  /**
-   * Return true if this code for this target should be built using Docker if Docker is used.
-   *
-   * @return
-   */
+  /** Return true if this code for this target should be built using Docker if Docker is used. */
   public boolean buildsUsingDocker() {
     return switch (this) {
       case TS -> false;