Added formated diagnostics to Javac Compiler

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BUILD

@@ -52,6 +52,7 @@ java_library(
         ":javac_options",
         "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins",
         "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/statistics",
+        "//src/main/protobuf:diagnostics_java_proto",
         "//third_party:auto_value",
         "//third_party:guava",
         "//third_party:jsr305",
@@ -89,6 +90,7 @@ java_library(
         "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:errorprone",
         "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:processing",
         "//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/statistics",
+        "//src/main/protobuf:diagnostics_java_proto",
         "//src/main/protobuf:worker_protocol_java_proto",
         "//third_party:guava",
         "//third_party:jsr305",

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java

@@ -24,16 +24,20 @@
 import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
 import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
 import com.google.devtools.build.buildjar.javac.plugins.errorprone.ErrorPronePlugin;
+import com.google.devtools.build.lib.diagnostics.Diagnostics;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.net.URI;
 import java.nio.charset.Charset;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.*;
+import java.util.stream.Collectors;
 
 /** The JavaBuilder main called by bazel. */
 public abstract class BazelJavaBuilder {
@@ -110,6 +114,8 @@ public static int processRequest(List<String> args, PrintWriter err) {
         for (FormattedDiagnostic d : result.diagnostics()) {
           err.write(d.getFormatted() + "\n");
         }
+
+        writeStructuredDiagnostics(result.diagnostics(), build.getDiagnosticsFile());
         err.write(result.output());
         return result.isOk() ? 0 : 1;
       }
@@ -122,6 +128,51 @@ public static int processRequest(List<String> args, PrintWriter err) {
     }
   }
 
+  private static void writeStructuredDiagnostics(ImmutableList<FormattedDiagnostic> diagnostics, String diagnosticsFile) throws IOException {
+    if(diagnosticsFile == null)
+      return;
+    Map<URI, List<Diagnostics.Diagnostic>> targetDiagnosticsMap = new HashMap<>();
+    for(FormattedDiagnostic diagnostic : diagnostics.stream().filter(diagnostic -> diagnostic.getSource() != null).collect(Collectors.toList())) {
+      Diagnostics.Diagnostic converted = Diagnostics.Diagnostic
+              .newBuilder()
+              .setRange(positionToRange(diagnostic))
+              .setSeverity(convertSeverity(diagnostic))
+              .setMessage(diagnostic.getMessage(Locale.ENGLISH))
+              .build();
+      List<Diagnostics.Diagnostic> fileDiagnostics = targetDiagnosticsMap.getOrDefault(diagnostic.getSource().toUri(), new ArrayList<>());
+      fileDiagnostics.add(converted);
+      targetDiagnosticsMap.put(diagnostic.getSource().toUri(), fileDiagnostics);
+    }
+    Diagnostics.TargetDiagnostics.Builder targetDiagnostics = Diagnostics.TargetDiagnostics.newBuilder();
+    for(Map.Entry<URI, List<Diagnostics.Diagnostic>> entry : targetDiagnosticsMap.entrySet())
+      targetDiagnostics.addDiagnostics(Diagnostics.FileDiagnostics.newBuilder().setPath(entry.getKey().toString()).addAllDiagnostics(entry.getValue()));
+    Files.write(Paths.get(diagnosticsFile) , targetDiagnostics.build().toByteArray(), StandardOpenOption.CREATE, StandardOpenOption.APPEND);
+  }
+
+  private static Diagnostics.Severity convertSeverity(FormattedDiagnostic diagnostic) {
+    switch (diagnostic.getKind()){
+      case WARNING:
+      case MANDATORY_WARNING:
+        return Diagnostics.Severity.WARNING;
+      case NOTE:
+        return Diagnostics.Severity.HINT;
+      case OTHER:
+        return Diagnostics.Severity.INFORMATION;
+      case ERROR:
+      default:
+        return Diagnostics.Severity.ERROR;
+    }
+  }
+
+  private static Diagnostics.Range positionToRange(FormattedDiagnostic diagnostics) {
+    int lineNumber =  (int) diagnostics.getLineNumber();
+    return Diagnostics.Range
+            .newBuilder()
+            .setStart(Diagnostics.Position.newBuilder().setLine(lineNumber - 1).setCharacter(((int) diagnostics.getColumnNumber()) - 1))
+            .setEnd(Diagnostics.Position.newBuilder().setLine(lineNumber))
+            .build();
+  }
+
   /**
    * Parses the list of arguments into a {@link JavaLibraryBuildRequest}. The returned {@link
    * JavaLibraryBuildRequest} object can be then used to configure the compilation itself.

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java

@@ -39,6 +39,7 @@
 
 /** All the information needed to perform a single Java library build operation. */
 public final class JavaLibraryBuildRequest {
+  private final String diagnosticsFile;
   private ImmutableList<String> javacOpts;
 
   /** Where to store source files generated by annotation processors. */
@@ -184,6 +185,7 @@ public JavaLibraryBuildRequest(
     this.generatedClassOutputJar = asPath(optionsParser.getManifestProtoPath());
     this.targetLabel = optionsParser.getTargetLabel();
     this.injectingRuleKind = optionsParser.getInjectingRuleKind();
+    this.diagnosticsFile = optionsParser.getDiagnosticsFile();
   }
 
   private static ImmutableList<Path> asPaths(Collection<String> paths) {
@@ -379,4 +381,8 @@ void addJavacArguments(BlazeJavacArguments.Builder builder) {
 
     builder.javacOptions(javacArguments.build());
   }
+
+  public String getDiagnosticsFile() {
+    return diagnosticsFile;
+  }
 }

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java

@@ -23,17 +23,7 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.ArrayDeque;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Deque;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.NoSuchElementException;
-import java.util.Set;
+import java.util.*;
 import javax.annotation.Nullable;
 
 /**
@@ -50,6 +40,8 @@ public final class OptionsParser {
 
   private String outputDepsProtoFile;
   private final Set<String> depsArtifacts = new LinkedHashSet<>();
+  private String diagnosticsFile;
+
 
   /** This modes controls how a probablistic Java classpath reduction is used. */
   public enum ReduceClasspathMode {
@@ -133,6 +125,7 @@ public OptionsParser(List<String> args, @Nullable JavacOptions normalizer)
    * @throws InvalidCommandLineException on an invalid option being passed.
    */
   private void processCommandlineArgs(Deque<String> argQueue) throws InvalidCommandLineException {
+    Object[] dequeCopy = argQueue.toArray().clone();
     for (String arg = argQueue.pollFirst(); arg != null; arg = argQueue.pollFirst()) {
       switch (arg) {
         case "--javacopts":
@@ -239,8 +232,11 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
         case "--profile":
           profile = getArgument(argQueue, arg);
           break;
+        case "--diagnostics_file":
+          diagnosticsFile = getArgument(argQueue, arg);
+          break;
         default:
-          throw new InvalidCommandLineException("unknown option : '" + arg + "'");
+          throw new InvalidCommandLineException("unknown option : '" + arg + "'" + " with the following arguments: " + Arrays.toString(dequeCopy));
       }
     }
   }
@@ -492,4 +488,8 @@ public String getInjectingRuleKind() {
   public String getProfile() {
     return profile;
   }
+
+  public String getDiagnosticsFile() {
+    return diagnosticsFile;
+  }
 }

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java

@@ -14,12 +14,6 @@
 
 package com.google.devtools.build.buildjar.javac;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.common.collect.MoreCollectors.toOptional;
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Comparator.comparing;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -36,6 +30,9 @@
 import com.sun.tools.javac.main.JavaCompiler;
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.PropagatedException;
+
+import javax.tools.Diagnostic;
+import javax.tools.StandardLocation;
 import java.io.IOError;
 import java.io.IOException;
 import java.io.PrintWriter;
@@ -46,8 +43,12 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
-import javax.tools.Diagnostic;
-import javax.tools.StandardLocation;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.common.collect.MoreCollectors.toOptional;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Comparator.comparing;
 
 /**
  * Main class for our custom patched javac.

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/WerrorCustomOption.java

@@ -19,6 +19,7 @@
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+
 import java.util.LinkedHashMap;
 import java.util.Map;
 

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/statistics/BlazeJavacStatistics.java

@@ -131,8 +131,6 @@ public Builder addBugpatternTiming(String key, Duration value) {
      *
      * <p>Since this method is called across the boundaries of an annotation processorpath and the
      * runtime classpath of the compiler, we want to reduce the number of classes mentioned, hence
-     * the byte[] data type. If we find a way to make this more safe, we would prefer to use a
-     * protobuf ByteString instead for its immutability.
      */
     public Builder addAuxiliaryData(AuxiliaryDataSource key, byte[] serializedData) {
       auxiliaryDataBuilder().put(key, serializedData.clone());

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java

@@ -26,32 +26,8 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.util.concurrent.ListenableFuture;
-import com.google.devtools.build.lib.actions.AbstractAction;
-import com.google.devtools.build.lib.actions.ActionContinuationOrResult;
-import com.google.devtools.build.lib.actions.ActionEnvironment;
-import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ActionKeyContext;
-import com.google.devtools.build.lib.actions.ActionOwner;
-import com.google.devtools.build.lib.actions.ActionResult;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.BaseSpawn;
-import com.google.devtools.build.lib.actions.CommandAction;
-import com.google.devtools.build.lib.actions.CommandLine;
-import com.google.devtools.build.lib.actions.CommandLineExpansionException;
-import com.google.devtools.build.lib.actions.CommandLines;
+import com.google.devtools.build.lib.actions.*;
 import com.google.devtools.build.lib.actions.CommandLines.CommandLineAndParamFileInfo;
-import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
-import com.google.devtools.build.lib.actions.EnvironmentalExecException;
-import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.ParamFileInfo;
-import com.google.devtools.build.lib.actions.ParameterFile;
-import com.google.devtools.build.lib.actions.ResourceSet;
-import com.google.devtools.build.lib.actions.RunfilesSupplier;
-import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.actions.SpawnContinuation;
-import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -76,6 +52,7 @@
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.LazyString;
+import com.google.devtools.build.lib.vfs.*;
 import com.google.devtools.build.lib.view.proto.Deps;
 import java.io.IOException;
 import java.io.InputStream;
@@ -85,6 +62,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /** Action that represents a Java compilation. */
@@ -127,7 +105,7 @@ enum CompilationType {
   private final NestedSet<Artifact> dependencyArtifacts;
   private final Artifact outputDepsProto;
   private final JavaClasspathMode classpathMode;
-
+  private final Artifact diagnosticsFile;
   @Nullable private final ExtraActionInfoSupplier extraActionInfoSupplier;
 
   public JavaCompileAction(
@@ -175,6 +153,20 @@ public JavaCompileAction(
     this.dependencyArtifacts = dependencyArtifacts;
     this.outputDepsProto = outputDepsProto;
     this.classpathMode = classpathMode;
+    FileSystem fileSystem = null;
+    try {
+      fileSystem = new JavaIoFileSystem();
+    } catch (DigestHashFunction.DefaultHashFunctionNotSetException e) {
+      e.printStackTrace();
+    }
+    if(fileSystem != null)
+      this.diagnosticsFile = new Artifact.SourceArtifact(
+              getPrimaryOutput().getRoot(),
+              PathFragment.create(getPrimaryOutput().getFilename() + ".diagnosticsproto"),
+              getPrimaryInput().getArtifactOwner()
+        );
+    else
+      this.diagnosticsFile = null;
   }
 
   @Override
@@ -283,6 +275,7 @@ JavaSpawn getReducedSpawn(
             .addCommandLine(executableLine)
             .addCommandLine(flagLine, PARAM_FILE_INFO)
             .addCommandLine(classpathLine.build(), PARAM_FILE_INFO)
+            .addCommandLine(getDiagnosticsOption(), PARAM_FILE_INFO)
             .build();
     CommandLines.ExpandedCommandLines expandedCommandLines =
         reducedCommandLine.expand(
@@ -301,6 +294,12 @@ JavaSpawn getReducedSpawn(
         inputs);
   }
 
+  @Nullable
+  @Override
+  public Artifact getDiagnostics() {
+    return diagnosticsFile;
+  }
+
   private JavaSpawn getFullSpawn(ActionExecutionContext actionExecutionContext)
       throws CommandLineExpansionException {
     CommandLines.ExpandedCommandLines expandedCommandLines =
@@ -483,9 +482,16 @@ public CommandLines getCommandLines() {
         .addCommandLine(executableLine)
         .addCommandLine(flagLine, PARAM_FILE_INFO)
         .addCommandLine(getFullClasspathLine(), PARAM_FILE_INFO)
+        .addCommandLine(getDiagnosticsOption(), PARAM_FILE_INFO)
         .build();
   }
 
+  private CommandLine getDiagnosticsOption() {
+    return CustomCommandLine.builder()
+            .add("--diagnostics_file", diagnosticsFile.getPath().toString())
+            .build();
+  }
+
   private CommandLine getFullClasspathLine() {
     CustomCommandLine.Builder classpathLine =
         CustomCommandLine.builder().addExecPaths("--classpath", transitiveInputs);

src/main/protobuf/BUILD

@@ -17,6 +17,7 @@ FILES = [
     "crosstool_config",
     "deps",
     "desugar_deps",
+    "diagnostics",
     "execution_statistics",
     "extra_actions_base",
     "invocation_policy",