feature: Automatically add -release option when needed

[tgodzik]

Previously, if we compiled with a Bloop running an newer JDK we would not see any errors even when using methods belonging to a newer JDK. Now, we automatically add -release flag, which makes sure that we will fail any compilation that should not pass with an older JDK (if specified in configuration)

This is needed for incorporating the forks.

[ckipp01]

Suggested change

	s"Bloop is run with ${JavaRuntime.version} but your code requires $version to compile, " +
	s"Bloop is running with ${JavaRuntime.version} but your code requires $version to compile, " +

Should we maybe change it to running to hint at the fact that it needs to stop and be restarted with that version if necessary?

[tgodzik]

Done!

[kpodsiad]

nit: I'd extract getCompilationOptions to standalone function, not nested one. Compile is already too long and complicated without your changes

[tgodzik]

Good idea, extracted!

[kpodsiad]

Extracting that method helped me realize one thing, the main complexity of getCompilationOptions is connected with adjusting scalac options with release flag if needed. That is why I think it's worth to extract another function, which I hope makes getCompilationOptions even easier and allows to explain why there is need for adding release flag in the first place. I put some scaladoc based on your replies, but it definitely needs to be improved. I'm leaving this as a suggestion, it's up to you to decide whether extract another function or not.

adjustScalacOptions

  /**
   * Bloop runs Scala compilation in the same process as the main server,
   * so the compilation process will use the same JDK that Bloop is using.
   * That's why we must ensure that produce class files will be compliant with expected JDK version.
   */
  private def adjustScalacOptions(
      scalacOptions: Array[String],
      javacBin: Option[AbsolutePath],
      logger: Logger
  ): Array[String] = {
    def existsReleaseSetting = scalacOptions.exists(opt =>
      opt.startsWith("-release") ||
        opt.startsWith("--release") ||
        opt.startsWith("-java-output-version")
    )
    def sameHome = javacBin match {
      case Some(bin) => bin.getParent.getParent == JavaRuntime.home
      case None => false
    }

    javacBin.flatMap(binary =>
      // <JAVA_HOME>/bin/java
      JavaRuntime.getJavaVersionFromJavaHome(binary.getParent.getParent)
    ) match {
      case None => scalacOptions
      case Some(_) if existsReleaseSetting || sameHome => scalacOptions
      case Some(version) =>
        try {
          val numVer = if (version.startsWith("1.8")) 8 else version.takeWhile(_.isDigit).toInt
          val bloopNumVer = JavaRuntime.version.takeWhile(_.isDigit).toInt
          if (bloopNumVer > numVer) {
            scalacOptions ++ List("-release", numVer.toString())
          } else {
            logger.warn(
              s"Bloop is runing with ${JavaRuntime.version} but your code requires $version to compile, " +
                "this might cause some compilation issues when using JDK API unsupported by the Bloop's current JVM version"
            )
            scalacOptions
          }
        } catch {
          case NonFatal(_) =>
            scalacOptions
        }
    }
  }

[tgodzik]

Sure, done!

[kpodsiad]

why is checking java home relevant? I thought that what's important is checking current java version of the bloop process?

[tgodzik]

This is the java version of Java we are compiling with, not the Java version Bloop is using. The latter is below in line 313 val bloopNumVer = JavaRuntime.version.takeWhile(_.isDigit).toInt

[tgodzik]

Also in case this wasn't clear. Bloop runs Scala compilation in the same process as the main server, so the compilation process will use the JDK from Bloop unless we specify the release flag.

[kpodsiad]

Extracting that method helped me realize one thing, the main complexity of getCompilationOptions is connected with adjusting scalac options with release flag if needed. That is why I think it's worth to extract another function, which I hope makes getCompilationOptions even easier and allows to explain why there is need for adding release flag in the first place. I put some scaladoc based on your replies, but it definitely needs to be improved. I'm leaving this as a suggestion, it's up to you to decide whether extract another function or not.

adjustScalacOptions

  /**
   * Bloop runs Scala compilation in the same process as the main server,
   * so the compilation process will use the same JDK that Bloop is using.
   * That's why we must ensure that produce class files will be compliant with expected JDK version.
   */
  private def adjustScalacOptions(
      scalacOptions: Array[String],
      javacBin: Option[AbsolutePath],
      logger: Logger
  ): Array[String] = {
    def existsReleaseSetting = scalacOptions.exists(opt =>
      opt.startsWith("-release") ||
        opt.startsWith("--release") ||
        opt.startsWith("-java-output-version")
    )
    def sameHome = javacBin match {
      case Some(bin) => bin.getParent.getParent == JavaRuntime.home
      case None => false
    }

    javacBin.flatMap(binary =>
      // <JAVA_HOME>/bin/java
      JavaRuntime.getJavaVersionFromJavaHome(binary.getParent.getParent)
    ) match {
      case None => scalacOptions
      case Some(_) if existsReleaseSetting || sameHome => scalacOptions
      case Some(version) =>
        try {
          val numVer = if (version.startsWith("1.8")) 8 else version.takeWhile(_.isDigit).toInt
          val bloopNumVer = JavaRuntime.version.takeWhile(_.isDigit).toInt
          if (bloopNumVer > numVer) {
            scalacOptions ++ List("-release", numVer.toString())
          } else {
            logger.warn(
              s"Bloop is runing with ${JavaRuntime.version} but your code requires $version to compile, " +
                "this might cause some compilation issues when using JDK API unsupported by the Bloop's current JVM version"
            )
            scalacOptions
          }
        } catch {
          case NonFatal(_) =>
            scalacOptions
        }
    }
  }

[kpodsiad]

Looks good!