feat(java_indexer): try to handle unsupported -source flags (kythe#5668)

If the -source flag is set to a version below our minimum supported version, change the flag to our minimum supported version and try to index the CU. java flag behavior may have changed between the source version requested and the one we replace it with, but the alternative is to immediately fail indexing when the Java APIs throw an exception or return an error when they are aware of the unsupported -source version.
trevorade · May 30, 2023 · 5e8ab46 · 5e8ab46
1 parent 30b4039
commit 5e8ab46
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 1 deletion.
diff --git a/kythe/java/com/google/devtools/kythe/platform/java/BUILD b/kythe/java/com/google/devtools/kythe/platform/java/BUILD
@@ -33,11 +33,13 @@ java_library(
     srcs = ["JavacOptionsUtils.java"],
     javacopts = [
         "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+        "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
         "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
         "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
         "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
     ],
     deps = [
+        "//kythe/java/com/google/devtools/kythe/common:flogger",
         "//kythe/proto:analysis_java_proto",
         "//kythe/proto:java_java_proto",
         "//third_party/guava",

diff --git a/kythe/java/com/google/devtools/kythe/platform/java/JavaCompilationDetails.java b/kythe/java/com/google/devtools/kythe/platform/java/JavaCompilationDetails.java
@@ -208,7 +208,8 @@ private static ImmutableList<String> optionsFromCompilationUnit(
     ModifiableOptions arguments =
         ModifiableOptions.of(compilationUnit.getArgumentList())
             .ensureEncodingSet(DEFAULT_ENCODING)
-            .updateWithJavaOptions(compilationUnit);
+            .updateWithJavaOptions(compilationUnit)
+            .updateToMinimumSupportedSourceVersion();
 
     if (processors.isEmpty()) {
       arguments.add("-proc:none");

diff --git a/kythe/java/com/google/devtools/kythe/platform/java/JavacOptionsUtils.java b/kythe/java/com/google/devtools/kythe/platform/java/JavacOptionsUtils.java
@@ -26,11 +26,13 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
+import com.google.common.flogger.FluentLogger;
 import com.google.devtools.kythe.proto.Analysis.CompilationUnit;
 import com.google.devtools.kythe.proto.Java.JavaDetails;
 import com.google.protobuf.Any;
 import com.google.protobuf.InvalidProtocolBufferException;
 import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.file.JavacFileManager;
 import com.sun.tools.javac.main.Option;
 import com.sun.tools.javac.util.Context;
@@ -57,6 +59,8 @@
  * <p>To make modifications to javac commandline arguments, use {@code ModifiableOptions.of(args)}.
  */
 public class JavacOptionsUtils {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   private JavacOptionsUtils() {}
 
   private static final Path JAVA_HOME = Paths.get(StandardSystemProperty.JAVA_HOME.value());
@@ -362,6 +366,56 @@ private ImmutableList<String> removeArgumentPaths(Option option) {
       return paths.build();
     }
 
+    /**
+     * Find any -source flags that specify a version of java that the JRE can't support and replace
+     * them with the lowest version that the JRE does support.
+     *
+     * <p>There may be problems during analysis due to flags passed to javac or language features
+     * that changed, but the alternative is to have the analysis fail immediately when we send the
+     * flag to the underlying Java APIs to do the analysis.
+     */
+    public ModifiableOptions updateToMinimumSupportedSourceVersion() {
+      ArrayList<String> unsupportedVersions = new ArrayList<>();
+      ArrayList<String> supportedVersions = new ArrayList<>();
+      List<String> replacements = new ArrayList<>(internal.size());
+      Consumer<String> matched =
+          (value) -> {
+            Source v = Source.lookup(value);
+            if (v == null) {
+              logger.atWarning().log("Could not parse source version number: %s", value);
+              // Don't mutate the flag if it can't be parsed.
+              supportedVersions.add(value);
+            } else if (v.compareTo(Source.MIN) < 0) {
+              unsupportedVersions.add(value);
+            } else {
+              supportedVersions.add(value);
+            }
+          };
+      Consumer<String> unmatched = replacements::add;
+      acceptOptions(handleOpts(ImmutableList.of(Option.SOURCE)), x -> {}, unmatched, matched);
+      internal = replacements;
+
+      if (!supportedVersions.isEmpty() || !unsupportedVersions.isEmpty()) {
+        if (supportedVersions.size() + unsupportedVersions.size() > 1) {
+          logger.atWarning().log("More than one -source flag passed, only using the last value");
+        }
+        internal.add(Option.SOURCE.getPrimaryName());
+        if (!supportedVersions.isEmpty()) {
+          internal.add(Iterables.getLast(supportedVersions));
+        } else if (!unsupportedVersions.isEmpty()) {
+          internal.add(Source.MIN.name);
+          // If we changed the source version, remove the target flag since the set of valid target
+          // values depends on what source was set to. Since we are already changing the source
+          // version, it shouldn't be any worse to change the explicit target version and instead
+          // use the default.
+          removeOptions(ImmutableSet.of(Option.TARGET));
+          // TODO(salguarnieri) increment a counter here.
+        }
+      }
+
+      return this;
+    }
+
     /** Applies handler to the interal options and returns the result. */
     private List<String> handleOptions(OptionHandler handler) {
       List<String> result = new ArrayList<>(internal.size());

diff --git a/kythe/javatests/com/google/devtools/kythe/platform/java/BUILD b/kythe/javatests/com/google/devtools/kythe/platform/java/BUILD
@@ -7,6 +7,7 @@ java_test(
     size = "small",
     srcs = ["OptionsTest.java"],
     javacopts = [
+        "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
         "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
     ],
     jvm_flags = [

diff --git a/kythe/javatests/com/google/devtools/kythe/platform/java/OptionsTest.java b/kythe/javatests/com/google/devtools/kythe/platform/java/OptionsTest.java
@@ -24,6 +24,7 @@
 import com.google.devtools.kythe.proto.Analysis.CompilationUnit;
 import com.google.devtools.kythe.proto.Java.JavaDetails;
 import com.google.protobuf.Any;
+import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.main.Option;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -208,4 +209,58 @@ public void updateWithJavaOptions_updatesSomePathsFromDetails() {
     assertThat(updatedArgs.get(updatedArgs.indexOf("--boot-class-path") + 1))
         .matches(".*(\\.jar|lib/modules)");
   }
+
+  @Test
+  public void updateToMinimumSupportedSourceVersion_updatesSource() {
+    // We can't test the --source format of the flag because it is only supported by recent versions
+    // of java.
+    ModifiableOptions args =
+        ModifiableOptions.of(ImmutableList.of("-foo", "-source", Source.JDK1_2.name));
+
+    assertThat(args.updateToMinimumSupportedSourceVersion().build())
+        .containsExactly("-foo", "-source", Source.MIN.name)
+        .inOrder();
+  }
+
+  @Test
+  public void updateToMinimumSupportedSourceVersion_updatesSource1DotFormat() {
+    ModifiableOptions args =
+        ModifiableOptions.of(ImmutableList.of("-foo", "-source", Source.JDK1_2.name));
+
+    assertThat(args.updateToMinimumSupportedSourceVersion().build())
+        .containsExactly("-foo", "-source", Source.MIN.name)
+        .inOrder();
+  }
+
+  @Test
+  public void updateToMinimumSupportedSourceVersion_removeTarget() {
+    ModifiableOptions args =
+        ModifiableOptions.of(
+            ImmutableList.of("-foo", "-source", Source.JDK1_2.name, "-target", Source.JDK1_2.name));
+
+    assertThat(args.updateToMinimumSupportedSourceVersion().build())
+        .containsExactly("-foo", "-source", Source.MIN.name)
+        .inOrder();
+  }
+
+  @Test
+  public void updateToMinimumSupportedSourceVersion_updatesMultipleSource() {
+    ModifiableOptions args =
+        ModifiableOptions.of(
+            ImmutableList.of("-foo", "-source", Source.JDK1_2.name, "-source", Source.JDK1_3.name));
+
+    assertThat(args.updateToMinimumSupportedSourceVersion().build())
+        .containsExactly("-foo", "-source", Source.MIN.name)
+        .inOrder();
+  }
+
+  @Test
+  public void updateToMinimumSupportedSourceVersion_doesNotUpdateSupportedVersion() {
+    ModifiableOptions args =
+        ModifiableOptions.of(ImmutableList.of("-foo", "-source", Source.DEFAULT.name));
+
+    assertThat(args.updateToMinimumSupportedSourceVersion().build())
+        .containsExactly("-foo", "-source", Source.DEFAULT.name)
+        .inOrder();
+  }
 }