bugfix: Fix issues when zgc is not fully supported

Turns out setting ZGC on graalvm for JDK 17 will break communication between metals server and vs code as it prints an unexpected warning to STDOUT

packages/metals-languageclient/src/__tests__/getJavaHome.test.ts

@@ -61,7 +61,7 @@ describe("getJavaHome", () => {
       new MockOutput()
     );
     jest.restoreAllMocks();
-    expect(javaHome).toBe(JAVA_HOME);
+    expect(javaHome.path).toBe(JAVA_HOME);
   });
 
   // needs to run on a machine with an actual JAVA_HOME set up
@@ -90,7 +90,7 @@ describe("getJavaHome", () => {
       new MockOutput()
     );
     jest.restoreAllMocks();
-    expect(javaHome).toBe(pathJavaHome);
+    expect(javaHome.path).toBe(pathJavaHome);
   });
 });
 

packages/metals-languageclient/src/getJavaConfig.ts

@@ -1,9 +1,11 @@
+import { JavaHome } from "./getJavaHome";
 import { getJavaOptions } from "./getJavaOptions";
 import * as path from "path";
 
 export interface JavaConfig {
   javaOptions: string[];
   javaPath: string;
+  javaHome: JavaHome;
   coursier: string;
   coursierMirrorFilePath: string | undefined;
   extraEnv: {
@@ -13,7 +15,7 @@ export interface JavaConfig {
 
 interface GetJavaConfigOptions {
   workspaceRoot: string | undefined;
-  javaHome: string;
+  javaHome: JavaHome;
   coursier: string;
   coursierMirrorFilePath: string | undefined;
   customRepositories: string[] | undefined;
@@ -27,7 +29,7 @@ export function getJavaConfig({
   customRepositories = [],
 }: GetJavaConfigOptions): JavaConfig {
   const javaOptions = getJavaOptions(workspaceRoot);
-  const javaPath = path.join(javaHome, "bin", "java");
+  const javaPath = path.join(javaHome.path, "bin", "java");
 
   const coursierRepositories =
     customRepositories.length > 0
@@ -45,6 +47,7 @@ export function getJavaConfig({
   return {
     javaOptions,
     javaPath,
+    javaHome,
     coursier,
     coursierMirrorFilePath,
     extraEnv,

packages/metals-languageclient/src/getJavaHome.ts

@@ -6,6 +6,12 @@ import { realpathSync } from "fs";
 
 export type JavaVersion = "11" | "17" | "21";
 
+export interface JavaHome {
+  path: string;
+  description: string;
+  version: string;
+}
+
 /**
  * Computes the user's Java Home path, using various strategies:
  *
@@ -19,17 +25,17 @@ export type JavaVersion = "11" | "17" | "21";
 export async function getJavaHome(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   const fromEnvValue = await fromEnv(javaVersion, outputChannel);
   return fromEnvValue || (await fromPath(javaVersion, outputChannel));
 }
 
 const versionRegex = /\"\d\d/;
-async function validateJavaVersion(
+export async function validateJavaVersion(
   javaHome: string,
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<boolean> {
+): Promise<JavaHome | undefined> {
   const javaBin = path.join(javaHome, "bin", "java");
   try {
     const javaVersionOut = spawn(javaBin, ["-version"], {
@@ -43,14 +49,18 @@ async function validateJavaVersion(
 
     const javaInfoStr = (await javaVersionOut).stderr as string;
     const matches = javaInfoStr.match(versionRegex);
-    if (matches) {
-      return +matches[0].slice(1, 3) >= +javaVersion;
+    if (matches && +matches[0].slice(1, 3) >= +javaVersion) {
+      return {
+        path: javaHome,
+        description: javaInfoStr,
+        version: matches[0].slice(1, 3),
+      };
     }
   } catch (error) {
     outputChannel.appendLine(`failed while running ${javaBin} -version`);
     outputChannel.appendLine(`${error}`);
   }
-  return false;
+  return undefined;
 }
 
 function propertyValueOf(
@@ -70,7 +80,7 @@ function propertyValueOf(
 export async function fromPath(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   let javaExecutable = findOnPath(["java"]);
   if (javaExecutable) {
     const realJavaPath = realpathSync(javaExecutable);
@@ -86,13 +96,22 @@ export async function fromPath(
         cmdOutput,
         "java.specification.version"
       );
-      const isValid =
-        discoveredJavaHome &&
-        discoveredJavaVersion &&
-        parseInt(discoveredJavaVersion) >= parseInt(javaVersion);
+      function getLastThreeLines(text: string): string {
+        let lines = text.split(/\r?\n/);
+        return lines.slice(-3).join("\n");
+      }
 
-      if (isValid) return discoveredJavaHome;
-      else {
+      if (
+        discoveredJavaVersion &&
+        discoveredJavaHome &&
+        parseInt(discoveredJavaVersion) >= parseInt(javaVersion)
+      ) {
+        return {
+          path: discoveredJavaHome,
+          description: getLastThreeLines(cmdOutput),
+          version: discoveredJavaVersion,
+        };
+      } else {
         outputChannel.appendLine(
           `Java version doesn't match the required one of ${javaVersion}`
         );
@@ -109,18 +128,18 @@ export async function fromPath(
 export async function fromEnv(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   const javaHome = process.env["JAVA_HOME"];
   if (javaHome) {
     outputChannel.appendLine(
       `Checking Java in JAVA_HOME, which points to ${javaHome}`
     );
-    const isValid = await validateJavaVersion(
+    const validatedJavaHome = await validateJavaVersion(
       javaHome,
       javaVersion,
       outputChannel
     );
-    if (isValid) return javaHome;
+    if (validatedJavaHome) return validatedJavaHome;
     else {
       outputChannel.appendLine(
         `Java version doesn't match the required one of ${javaVersion}`

packages/metals-languageclient/src/getServerOptions.ts

@@ -1,39 +1,53 @@
 import { ServerOptions } from "./interfaces/ServerOptions";
 import { JavaConfig } from "./getJavaConfig";
 
-interface GetServerOptions {
-  metalsClasspath: string;
-  serverProperties: string[] | undefined;
-  clientName?: string;
-  javaConfig: JavaConfig;
-}
-
-export function getServerOptions({
-  metalsClasspath,
-  serverProperties = [],
-  clientName,
-  javaConfig: { javaOptions, javaPath, extraEnv },
-}: GetServerOptions): ServerOptions {
+export function getServerOptions(
+  metalsClasspath: string,
+  serverProperties: string[],
+  clientName: string,
+  javaConfig: JavaConfig
+): ServerOptions {
   const baseProperties = ["-Xss4m", "-Xms100m"];
 
   if (clientName) {
     baseProperties.push(`-Dmetals.client=${clientName}`);
   }
 
+  /**
+   * GraalVM for JDK 17-20 prints additional warnings that breaks things
+   */
+  const skipZGC =
+    +javaConfig.javaHome.version < 21 &&
+    javaConfig.javaHome.description.indexOf("GraalVM") > -1;
+
+  var filteredServerProperties = serverProperties;
+  if (skipZGC) {
+    filteredServerProperties = serverProperties.filter(function (prop) {
+      return prop.indexOf("UseZGC") === -1;
+    });
+  }
   const mainArgs = ["-classpath", metalsClasspath, "scala.meta.metals.Main"];
 
   // let user properties override base properties
   const launchArgs = [
     ...baseProperties,
-    ...javaOptions,
-    ...serverProperties,
+    ...javaConfig.javaOptions,
+    ...filteredServerProperties,
     ...mainArgs,
   ];
 
-  const env = () => ({ ...process.env, ...extraEnv });
+  const env = () => ({ ...process.env, ...javaConfig.extraEnv });
 
   return {
-    run: { command: javaPath, args: launchArgs, options: { env: env() } },
-    debug: { command: javaPath, args: launchArgs, options: { env: env() } },
+    run: {
+      command: javaConfig.javaPath,
+      args: launchArgs,
+      options: { env: env() },
+    },
+    debug: {
+      command: javaConfig.javaPath,
+      args: launchArgs,
+      options: { env: env() },
+    },
   };
 }

packages/metals-languageclient/src/setupCoursier.ts

@@ -4,7 +4,12 @@ import {
   PromisifySpawnOptions,
   spawn,
 } from "promisify-child-process";
-import { JavaVersion, getJavaHome } from "./getJavaHome";
+import {
+  JavaHome,
+  JavaVersion,
+  getJavaHome,
+  validateJavaVersion,
+} from "./getJavaHome";
 import { OutputChannel } from "./interfaces/OutputChannel";
 import path from "path";
 import fs from "fs";
@@ -23,7 +28,7 @@ export async function setupCoursier(
   output: OutputChannel,
   forceCoursierJar: boolean,
   serverProperties: string[]
-): Promise<{ coursier: string; javaHome: string }> {
+): Promise<{ coursier: string; javaHome: JavaHome }> {
   const handleOutput = (out: Buffer) => {
     const msg = "\t" + out.toString().trim().split("\n").join("\n\t");
     output.appendLine(msg);
@@ -107,10 +112,18 @@ export async function setupCoursier(
     output.appendLine(
       `No installed java with version ${javaVersion} found. Will fetch one using coursier:`
     );
-    javaHome = await resolveJavaHomeWithCoursier(coursier);
+    const coursierJavaHome = await resolveJavaHomeWithCoursier(coursier);
+    const validatedJavaHome = await validateJavaVersion(
+      coursierJavaHome,
+      javaVersion,
+      output
+    );
+    if (validatedJavaHome) {
+      javaHome = validatedJavaHome;
+    }
   }
 
-  output.appendLine(`Using Java Home: ${javaHome}`);
+  output.appendLine(`Using Java Home: ${javaHome?.path}`);
 
   /* If we couldn't download coursier, but we have Java
    * we can still fall back to jar based launcher.

packages/metals-vscode/src/extension.ts

@@ -344,12 +344,12 @@ function launchMetals(
   // Make editing Scala docstrings slightly nicer.
   enableScaladocIndentation();
 
-  const serverOptions = getServerOptions({
+  const serverOptions = getServerOptions(
     metalsClasspath,
     serverProperties,
-    javaConfig,
-    clientName: "vscode",
-  });
+    "vscode",
+    javaConfig
+  );
 
   const initializationOptions: MetalsInitializationOptions = {
     compilerOptions: {