Task/17 lift warnings

.lift.toml

@@ -0,0 +1,3 @@
+build = "./mvnw -T8C -B -DskipTests=true -Dcheckstyle.skip=true clean package"
+jdkVersion = "11"
+summaryComments = true

java-compiler-testing/src/main/java/io/github/ascopes/jct/assertions/DiagnosticRepresentation.java

@@ -16,11 +16,16 @@
 
 package io.github.ascopes.jct.assertions;
 
+import static javax.tools.Diagnostic.NOPOS;
+
 import io.github.ascopes.jct.compilers.TraceDiagnostic;
+import io.github.ascopes.jct.intern.IoExceptionUtils;
+import io.github.ascopes.jct.intern.StringUtils;
+import java.io.IOException;
 import java.util.Locale;
-import java.util.Objects;
-import java.util.regex.Pattern;
-import javax.tools.FileObject;
+import java.util.Optional;
+import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 import org.assertj.core.presentation.Representation;
@@ -33,13 +38,8 @@
  */
 @API(since = "0.0.1", status = Status.EXPERIMENTAL)
 public class DiagnosticRepresentation implements Representation {
-
-  // Pattern that matches the toString output of java.lang.Object when it is not overridden.
-  // We use this to deal with the fact some ECJ diagnostics do not provide a useful toString,
-  // while javac provides a pretty toString including a nice little code snippet.
-  private static final Pattern NO_TO_STRING = Pattern.compile(
-      "^([a-zA-Z_$][a-zA-Z\\d_$]*\\.)*[a-zA-Z_$][a-zA-Z\\d_$]*@[A-Za-z0-9]+$"
-  );
+  private static final int ADDITIONAL_CONTEXT_LINES = 2;
+  private static final String PADDING = " ".repeat(8);
 
   /**
    * Initialize this diagnostic representation.
@@ -65,29 +65,130 @@ public String toStringOf(Object object) {
       builder.append(code);
     }
 
-    if (diagnostic.getSource() instanceof FileObject) {
-      var name = ((FileObject) diagnostic.getSource()).getName();
-      builder.append(" ").append(name);
-    }
-
     builder
+        .append(' ')
+        .append(diagnostic.getSource().getName())
         .append(" (at line ")
         .append(diagnostic.getLineNumber())
         .append(", col ")
         .append(diagnostic.getColumnNumber())
-        .append(")");
+        .append(")")
+        .append("\n\n");
 
-    var message = diagnostic.toString();
+    IoExceptionUtils.uncheckedIo(() -> {
+      extractSnippet(diagnostic)
+          .ifPresent(snippet -> {
+            snippet.prettyPrintTo(builder);
+            builder.append('\n');
+          });
+      builder
+          .append(PADDING)
+          .append(diagnostic.getMessage(Locale.ROOT));
+    });
 
-    if (NO_TO_STRING.matcher(message).matches()) {
-      message = Objects.toString(diagnostic.getMessage(Locale.ROOT));
+    return builder.toString();
+  }
+
+  private Optional<Snippet> extractSnippet(
+      Diagnostic<? extends JavaFileObject> diagnostic
+  ) throws IOException {
+    if (diagnostic.getStartPosition() == NOPOS || diagnostic.getEndPosition() == NOPOS) {
+      // No info available about position, so don't bother extracting anything.
+      return Optional.empty();
     }
 
-    message
-        .lines()
-        .map("   "::concat)
-        .forEach(line -> builder.append("\n").append(line));
+    var content = diagnostic.getSource().getCharContent(true).toString();
 
-    return builder.toString();
+    var startLine = Math.max(1, (int) diagnostic.getLineNumber() - ADDITIONAL_CONTEXT_LINES);
+    var lineStartOffset = StringUtils.indexOfLine(content, startLine);
+    var lineEndOffset = StringUtils.indexOfEndOfLine(content, (int) diagnostic.getEndPosition());
+
+    // Advance to include the additional lines of context
+    var endOfSnippet = lineEndOffset;
+    for (var i = 0; i < ADDITIONAL_CONTEXT_LINES; ++i) {
+      endOfSnippet = StringUtils.indexOfEndOfLine(content, endOfSnippet + 1);
+    }
+
+    var snippet = new Snippet(
+        content.substring(lineStartOffset, endOfSnippet),
+        Math.max(1, diagnostic.getLineNumber() - ADDITIONAL_CONTEXT_LINES),
+        diagnostic.getStartPosition() - lineStartOffset,
+        lineEndOffset - lineStartOffset
+    );
+
+    return Optional.of(snippet);
+  }
+
+  private static final class Snippet {
+
+    private final String text;
+    private final long startLine;
+    private final long startOffset;
+    private final long endOffset;
+    private final int lineNumberWidth;
+
+    private Snippet(String text, long startLine, long startOffset, long endOffset) {
+      this.text = text;
+      this.startLine = startLine;
+      this.startOffset = startOffset;
+      this.endOffset = endOffset;
+
+      // Width of the line number part of the output.
+      lineNumberWidth = (int) Math.ceil(Math.log10(startLine)) + 1;
+    }
+
+    public void prettyPrintTo(StringBuilder builder) {
+      var lineIndex = 0;
+      var isNewLine = true;
+      var startOfLine = 0;
+
+      for (var i = 0; i < text.length(); ++i) {
+        if (isNewLine) {
+          appendLineNumberPart(builder, lineIndex);
+          isNewLine = false;
+        }
+
+        var nextChar = text.charAt(i);
+        builder.append(nextChar);
+
+        if (nextChar == '\n' || i == text.length() - 1) {
+          if (nextChar != '\n') {
+            // Ensure newline if we are adding the end-of-content line
+            builder.append('\n');
+          }
+          appendPossibleUnderline(builder, startOfLine, i);
+          ++lineIndex;
+          startOfLine = i + 1;
+          isNewLine = true;
+        }
+      }
+    }
+
+    private void appendLineNumberPart(StringBuilder builder, int lineIndex) {
+      var rawLineString = Long.toString(lineIndex + startLine);
+      var paddedLineString = StringUtils.leftPad(rawLineString, lineNumberWidth, ' ');
+
+      builder
+          .append(PADDING)
+          .append(paddedLineString)
+          .append(" | ");
+    }
+
+    private void appendPossibleUnderline(StringBuilder builder, int startOfLine, int endOfLine) {
+      if (startOfLine > endOffset || endOfLine < startOffset) {
+        return;
+      }
+
+      builder
+          .append(PADDING)
+          .append(" ".repeat(lineNumberWidth))
+          .append(" · ");
+
+      for (int i = startOfLine; i < endOfLine; ++i) {
+        builder.append(startOffset <= i && i <= endOffset ? '^' : ' ');
+      }
+
+      builder.append('\n');
+    }
   }
 }

java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/TraceDiagnostic.java

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Optional;
 import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 
@@ -35,7 +36,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.EXPERIMENTAL)
-public class TraceDiagnostic<S> extends ForwardingDiagnostic<S> {
+public class TraceDiagnostic<S extends JavaFileObject> extends ForwardingDiagnostic<S> {
 
   private final Instant timestamp;
   private final long threadId;

java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/TracingDiagnosticListener.java

@@ -26,6 +26,7 @@
 import java.util.stream.Collectors;
 import javax.tools.Diagnostic;
 import javax.tools.DiagnosticListener;
+import javax.tools.JavaFileObject;
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 import org.slf4j.Logger;
@@ -40,7 +41,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.EXPERIMENTAL)
-public class TracingDiagnosticListener<S> implements DiagnosticListener<S> {
+public class TracingDiagnosticListener<S extends JavaFileObject> implements DiagnosticListener<S> {
 
   private final ConcurrentLinkedQueue<TraceDiagnostic<S>> diagnostics;
   private final Logger logger;

java-compiler-testing/src/main/java/io/github/ascopes/jct/intern/StringUtils.java

@@ -35,6 +35,65 @@ private StringUtils() {
     throw new UnsupportedOperationException("static-only class");
   }
 
+  /**
+   * Find the index for the start of the given line number (1-indexed).
+   *
+   * <p>This assumes lines use UNIX line endings ({@code '\n'}).
+   *
+   * <p>The first line number will always be at index 0. If the line is not found, then
+   * {@code -1} is returned.
+   *
+   * @param content the content to read through.
+   * @param lineNumber the 1-indexed line number to find.
+   * @return the index of the line.
+   */
+  public static int indexOfLine(String content, int lineNumber) {
+    var currentLine = 1;
+    var index = 0;
+    var length = content.length();
+
+    while (currentLine < lineNumber && index < length) {
+      if (content.charAt(index) == '\n') {
+        ++currentLine;
+      }
+      ++index;
+    }
+
+    return currentLine == lineNumber
+        ? index
+        : -1;
+  }
+
+  /**
+   * Left-pad the given content with the given padding char until it is the given length.
+   *
+   * @param content the content to process.
+   * @param length the max length of the resultant content.
+   * @param paddingChar the character to pad with.
+   * @return the padded string.
+   */
+  public static String leftPad(String content, int length, char paddingChar) {
+    var builder = new StringBuilder();
+    while (builder.length() + content.length() < length) {
+      builder.append(paddingChar);
+    }
+    return builder.append(content).toString();
+  }
+
+  /**
+   * Find the index of the next UNIX end of line ({@code '\n'}) character from the given offset.
+   *
+   * <p>If there is no further line feed, then the length of the string is returned.
+   *
+   * @param content the content to read through.
+   * @param startAt the 0-indexed position to start at in the string.
+   * @return the index of the end of line or end of string, whichever comes first.
+   */
+  public static int indexOfEndOfLine(String content, int startAt) {
+    var index = content.indexOf('\n', startAt);
+    return index == -1 ? content.length() : index;
+  }
+
   /**
    * Wrap the string representation of the given argument in double-quotes.
    *

java-compiler-testing/src/main/java/io/github/ascopes/jct/paths/RamPath.java

@@ -69,7 +69,6 @@
 public class RamPath {
 
   private static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
-
   private static final Logger LOGGER = LoggerFactory.getLogger(RamPath.class);
   private static final Cleaner CLEANER = Cleaner.create();
 
@@ -432,8 +431,10 @@ public RamPath copyFrom(InputStream input, Path targetPath) {
     return uncheckedIo(() -> {
       var bufferedInput = maybeBuffer(input, targetPath.toUri().getScheme());
       var path = makeRelativeToHere(targetPath);
-      var options = new OpenOption[]{StandardOpenOption.CREATE,
-          StandardOpenOption.TRUNCATE_EXISTING};
+      var options = new OpenOption[]{
+          StandardOpenOption.CREATE,
+          StandardOpenOption.TRUNCATE_EXISTING
+      };
       Files.createDirectories(path.getParent());
 
       try (var output = Files.newOutputStream(path, options)) {
@@ -503,9 +504,12 @@ public RamPath copyTreeFrom(Path tree, String targetPath) {
    * @throws UncheckedIOException if something goes wrong copying the tree out of memory.
    */
   public Path copyToTempDir() {
+    // https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN
+    var safeName = name.substring(Math.max(0, name.lastIndexOf('/')));
+
     var tempPath = uncheckedIo(() -> Files.copy(
         path,
-        Files.createTempDirectory(name),
+        Files.createTempDirectory(safeName),
         StandardCopyOption.REPLACE_EXISTING,
         StandardCopyOption.COPY_ATTRIBUTES
     ));

java-compiler-testing/src/test/java/io/github/ascopes/jct/testing/integration/basic/BasicLegacyCompilationTest.java

@@ -45,7 +45,7 @@ void helloWorldJavac(int version) {
             "com/example/HelloWorld.java",
             "package com.example;",
             "public class HelloWorld {",
-            "  public static void main(String[] args) {",
+            "  public static void main(String[] args) {?",
             "    System.out.println(\"Hello, World\");",
             "  }",
             "}"

java-compiler-testing/src/test/java/io/github/ascopes/jct/testing/unit/compilers/SimpleCompilationTest.java

@@ -123,7 +123,7 @@ void getCompilationUnitsReturnsExpectedValue(int compilationUnitCount) {
   @ParameterizedTest(name = "for diagnosticCount = {0}")
   void getDiagnosticsReturnsExpectedValue(int diagnosticCount) {
     // Given
-    var diagnosticType = new TypeRef<TraceDiagnostic<? extends JavaFileObject>>() {};
+    var diagnosticType = new TypeRef<TraceDiagnostic<JavaFileObject>>() {};
     var diagnostics = Stream
         .generate(() -> MoreMocks.stubCast(diagnosticType))
         .limit(diagnosticCount)

java-compiler-testing/src/test/java/io/github/ascopes/jct/testing/unit/compilers/TraceDiagnosticTest.java

@@ -27,6 +27,7 @@
 import java.util.Random;
 import java.util.UUID;
 import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
@@ -42,7 +43,7 @@ class TraceDiagnosticTest {
   @Test
   void nullTimestampsAreRejected() {
     var stack = MoreMocks.stubCast(new TypeRef<List<StackTraceElement>>() {});
-    var diag = MoreMocks.stubCast(new TypeRef<Diagnostic<?>>() {});
+    var diag = MoreMocks.stubCast(new TypeRef<Diagnostic<JavaFileObject>>() {});
     thenCode(() -> new TraceDiagnostic<>(null, 123, "foo", stack, diag))
         .isInstanceOf(NullPointerException.class);
   }
@@ -51,7 +52,7 @@ void nullTimestampsAreRejected() {
   @Test
   void nullStackTracesAreRejected() {
     var now = Instant.now();
-    var diag = MoreMocks.stubCast(new TypeRef<Diagnostic<?>>() {});
+    var diag = MoreMocks.stubCast(new TypeRef<Diagnostic<JavaFileObject>>() {});
     thenCode(() -> new TraceDiagnostic<>(now, 123, "foo", null, diag))
         .isInstanceOf(NullPointerException.class);
   }

java-compiler-testing/src/test/java/io/github/ascopes/jct/testing/unit/compilers/TracingDiagnosticListenerTest.java

@@ -46,6 +46,7 @@
 import java.util.stream.Stream;
 import javax.tools.Diagnostic;
 import javax.tools.Diagnostic.Kind;
+import javax.tools.JavaFileObject;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -384,8 +385,11 @@ static Stream<Arguments> loggingDisabledArgs() {
         ));
   }
 
-  static <T> Diagnostic<T> someDiagnostic(Kind kind, String message) {
-    var diagnostic = stubCast(new TypeRef<Diagnostic<T>>() {}, withSettings().lenient());
+  static Diagnostic<JavaFileObject> someDiagnostic(Kind kind, String message) {
+    var diagnostic = stubCast(
+        new TypeRef<Diagnostic<JavaFileObject>>() {},
+        withSettings().lenient()
+    );
     when(diagnostic.getKind()).thenReturn(kind);
     when(diagnostic.getMessage(any())).thenReturn(message);
     return diagnostic;
@@ -403,7 +407,7 @@ static Supplier<Thread> dummyThreadSupplier() {
     return () -> thread;
   }
 
-  static class AccessibleImpl<T> extends TracingDiagnosticListener<T> {
+  static class AccessibleImpl<T extends JavaFileObject> extends TracingDiagnosticListener<T> {
 
     AccessibleImpl(
         boolean logging,