Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle incorrect Scalac options and prevent printing ScalacWorker stacktraces #1606

Merged
merged 8 commits into from
Sep 9, 2024
14 changes: 10 additions & 4 deletions src/java/io/bazel/rulesscala/scalac/ScalacInvoker.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c
comp.process(compilerArgs);
} catch (Throwable ex) {
if (ex.toString().contains("scala.reflect.internal.Types$TypeError")) {
throw new RuntimeException("Build failure with type error", ex);
throw new ScalacWorker.CompilationFailed("with type error", ex);
} else if (ex.toString().contains("java.lang.StackOverflowError")) {
throw new RuntimeException("Build failure with StackOverflowError", ex);
throw new ScalacWorker.CompilationFailed("with StackOverflowError", ex);
} else if (isMacroException(ex)) {
throw new RuntimeException("Build failure during macro expansion", ex);
throw new ScalacWorker.CompilationFailed("during macro expansion", ex);
} else {
throw ex;
}
Expand All @@ -39,6 +39,12 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c
results.stopTime = System.currentTimeMillis();

ConsoleReporter reporter = (ConsoleReporter) comp.getReporter();
if (reporter == null) {
// Can happen only when `ReportableMainClass::newCompiler` was not invoked,
// typically due to invalid settings
throw new ScalacWorker.InvalidSettings();
}

if (reporter instanceof ProtoReporter) {
ProtoReporter protoReporter = (ProtoReporter) reporter;
protoReporter.writeTo(Paths.get(ops.diagnosticsFile));
Expand All @@ -52,7 +58,7 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c

if (reporter.hasErrors()) {
reporter.flush();
throw new RuntimeException("Build failed");
throw new ScalacWorker.CompilationFailed("with errors.");
}

return results;
Expand Down
10 changes: 7 additions & 3 deletions src/java/io/bazel/rulesscala/scalac/ScalacInvoker3.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c
Driver driver = new dotty.tools.dotc.Driver();
Contexts.Context ctx = driver.initCtx().fresh();

Tuple2<scala.collection.immutable.List<AbstractFile>, Contexts.Context> r = driver.setup(compilerArgs, ctx).get();
Tuple2<scala.collection.immutable.List<AbstractFile>, Contexts.Context> r =
driver.setup(compilerArgs, ctx)
.getOrElse(() -> {
throw new ScalacWorker.InvalidSettings();
});

Compiler compiler = driver.newCompiler(r._2);

Expand All @@ -39,8 +43,8 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c


if (reporter.hasErrors()) {
// reporter.flush();
throw new RuntimeException("Build failed");
reporter.flush(ctx);
throw new ScalacWorker.CompilationFailed("with errors.");
}

return results;
Expand Down
15 changes: 15 additions & 0 deletions src/java/io/bazel/rulesscala/scalac/ScalacWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,21 @@

class ScalacWorker implements Worker.Interface {

public static class InvalidSettings extends WorkerException {
public InvalidSettings() {
super("Failed to invoke Scala compiler, ensure passed options are valid");
}
}

public static class CompilationFailed extends WorkerException {
public CompilationFailed(String reason, Throwable cause) {
super("Build failure " + reason, cause);
}
public CompilationFailed(String reason) {
this(reason, null);
}
}

private static final boolean isWindows =
System.getProperty("os.name").toLowerCase().contains("windows");

Expand Down
14 changes: 12 additions & 2 deletions src/java/io/bazel/rulesscala/worker/Worker.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ public final class Worker {

public static interface Interface {
public void work(String[] args) throws Exception;


public abstract class WorkerException extends RuntimeException {
public WorkerException(String message) {
super(message);
}
public WorkerException(String message, Throwable cause) {
super(message, cause);
}
}
}

/**
Expand Down Expand Up @@ -87,8 +97,8 @@ public void checkPermission(Permission permission) {
} catch (ExitTrapped e) {
code = e.code;
} catch (Exception e) {
System.err.println(e.getMessage());
e.printStackTrace();
if (e instanceof Interface.WorkerException) System.err.println(e.getMessage());
else e.printStackTrace();
code = 1;
}

Expand Down
36 changes: 36 additions & 0 deletions test/shell/test_invalid_scalacopts.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# shellcheck source=./test_runner.sh

dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
. "${dir}"/test_runner.sh
. "${dir}"/test_helper.sh
runner=$(get_test_runner "${1:-local}")

test_logs_contains() {
scalaVersion=$1
expected=$2

bazel build \
--repo_env=SCALA_VERSION=${scalaVersion} \
//test_expect_failure/scalacopts_invalid:empty \
2>&1 | grep "$expected"
}

test_logs_not_contains() {
scalaVersion=$1
expected=$2

bazel build \
--repo_env=SCALA_VERSION=${scalaVersion} \
//test_expect_failure/scalacopts_invalid:empty \
2>&1 | grep -v "$expected"
}

for scalaVersion in 2.12.19 2.13.14 3.3.3; do
if [[ "$scalaVersion" == 3.* ]]; then
$runner test_logs_contains $scalaVersion "not-existing is not a valid choice for -source"
else
$runner test_logs_contains $scalaVersion "bad option: '-source:not-existing'"
fi
$runner test_logs_contains $scalaVersion 'Failed to invoke Scala compiler, ensure passed options are valid'
$runner test_logs_not_contains $scalaVersion 'at io.bazel.rulesscala.scalac.ScalacWorker.main'
done
7 changes: 7 additions & 0 deletions test_expect_failure/scalacopts_invalid/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
load("//scala:scala.bzl", "scala_library")

scala_library(
name = "empty",
srcs = ["Empty.scala"],
scalacopts = ["-source:not-existing"],
)
3 changes: 3 additions & 0 deletions test_expect_failure/scalacopts_invalid/Empty.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package test_expect_failure.scalacopts_invalid

class Empty
1 change: 1 addition & 0 deletions test_rules_scala.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@ $runner bazel build //test_statsfile:SimpleNoStatsFile_statsfile --extra_toolcha
. "${test_dir}"/test_persistent_worker.sh
. "${test_dir}"/test_semanticdb.sh
. "${test_dir}"/test_scaladoc.sh
. "${test_dir}"/test_invalid_scalacopts.sh