From a1c00016d48df4b24a668f6b0c1296efbdc910fc Mon Sep 17 00:00:00 2001 From: Jaden Peterson Date: Thu, 16 Jan 2025 11:14:51 -0500 Subject: [PATCH 1/3] Throw a more informative error when scala_macro_library isn't used --- .../bazel/rulesscala/scalac/ScalacWorker.java | 17 ++++++++++++-- test/macros/BUILD | 23 +++++++++++++++++++ test/macros/IdentityMacro.scala | 9 ++++++++ test/macros/MacroUser.scala | 5 ++++ test/shell/test_macros.sh | 17 ++++++++++++++ 5 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 test/macros/BUILD create mode 100644 test/macros/IdentityMacro.scala create mode 100644 test/macros/MacroUser.scala create mode 100755 test/shell/test_macros.sh diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java index 6a6679762..dfc944de9 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java @@ -237,7 +237,7 @@ private static String[] getPluginParamsFrom(CompileOptions ops) { } if (ops.directTargets.length > 0) { pluginParams.add( - "-P:dependency-analyzer:direct-targets:" + encodeStringSeqPluginParam(ops.directTargets)); + "-P:dependency-analyzer:direct-targets:" + encodeStringSeqPluginParam(ops.directTargets)); } if (ops.indirectJars.length > 0) { pluginParams.add( @@ -269,7 +269,20 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource String[] compilerArgs = merge(ops.scalaOpts, pluginArgs, constParams, pluginParams, scalaSources); - ScalacInvokerResults compilerResults = ScalacInvoker.invokeCompiler(ops, compilerArgs); + ScalacInvokerResults compilerResults; + + try { + compilerResults = ScalacInvoker.invokeCompiler(ops, compilerArgs); + } catch (CompilationFailed exception) { + if (exception.getCause() instanceof ClassFormatError) { + throw new Exception( + "You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target.", + exception + ); + } + + throw exception; + } if (ops.printCompileTime) { System.err.println("Compiler runtime: " + (compilerResults.stopTime - compilerResults.startTime) + "ms."); diff --git a/test/macros/BUILD b/test/macros/BUILD new file mode 100644 index 000000000..1033a3899 --- /dev/null +++ b/test/macros/BUILD @@ -0,0 +1,23 @@ +load("//scala:scala.bzl", "scala_library", "scala_macro_library") + +scala_library( + name = "incorrect-macro", + srcs = ["IdentityMacro.scala"], +) + +scala_macro_library( + name = "correct-macro", + srcs = ["IdentityMacro.scala"], +) + +scala_library( + name = "incorrect-macro-user", + srcs = ["MacroUser.scala"], + deps = [":incorrect-macro"], +) + +scala_library( + name = "correct-macro-user", + srcs = ["MacroUser.scala"], + deps = [":correct-macro"], +) diff --git a/test/macros/IdentityMacro.scala b/test/macros/IdentityMacro.scala new file mode 100644 index 000000000..f2504485f --- /dev/null +++ b/test/macros/IdentityMacro.scala @@ -0,0 +1,9 @@ +package macros + +import scala.language.experimental.macros +import scala.reflect.macros.blackbox + +object IdentityMacro { + def identityMacro[A](value: A): A = macro identityMacroImpl[A] + def identityMacroImpl[A](context: blackbox.Context)(value: context.Expr[A]): context.Expr[A] = value +} diff --git a/test/macros/MacroUser.scala b/test/macros/MacroUser.scala new file mode 100644 index 000000000..f05d36eb2 --- /dev/null +++ b/test/macros/MacroUser.scala @@ -0,0 +1,5 @@ +package macros + +object Main { + def main(arguments: Array[String]): Unit = println(IdentityMacro.identityMacro("Hello, world!")) +} diff --git a/test/shell/test_macros.sh b/test/shell/test_macros.sh new file mode 100755 index 000000000..05ef82490 --- /dev/null +++ b/test/shell/test_macros.sh @@ -0,0 +1,17 @@ +# 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}") + +incorrect_macro_user_does_not_build() { + (! bazel build //test/macros:incorrect-macro-user) |& + grep --fixed-strings 'java.lang.Exception: You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target.' +} + +correct_macro_user_builds() { + bazel build //test/macros:correct-macro-user +} + +$runner incorrect_macro_user_does_not_build +$runner correct_macro_user_builds From cc2f0d88ae9a130ade95429d4127a6f009df7c7a Mon Sep 17 00:00:00 2001 From: Jaden Peterson Date: Thu, 16 Jan 2025 13:34:45 -0500 Subject: [PATCH 2/3] fixup! Fixed failing tests --- test/macros/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/test/macros/BUILD b/test/macros/BUILD index 1033a3899..b293ff61d 100644 --- a/test/macros/BUILD +++ b/test/macros/BUILD @@ -13,6 +13,7 @@ scala_macro_library( scala_library( name = "incorrect-macro-user", srcs = ["MacroUser.scala"], + tags = ["manual"], deps = [":incorrect-macro"], ) From b827c3db4d7312f80adf1cf442689fae9e8dde4d Mon Sep 17 00:00:00 2001 From: Jaden Peterson Date: Wed, 22 Jan 2025 16:20:43 -0500 Subject: [PATCH 3/3] fixup! Catch `ClassFormatError`s in ScalacInvoker --- .../bazel/rulesscala/scalac/ScalacWorker.java | 17 ++--------------- .../scalac/scala_2/ScalacInvoker.java | 16 ++++++++++++---- test/shell/test_macros.sh | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java index dfc944de9..6a6679762 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java @@ -237,7 +237,7 @@ private static String[] getPluginParamsFrom(CompileOptions ops) { } if (ops.directTargets.length > 0) { pluginParams.add( - "-P:dependency-analyzer:direct-targets:" + encodeStringSeqPluginParam(ops.directTargets)); + "-P:dependency-analyzer:direct-targets:" + encodeStringSeqPluginParam(ops.directTargets)); } if (ops.indirectJars.length > 0) { pluginParams.add( @@ -269,20 +269,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource String[] compilerArgs = merge(ops.scalaOpts, pluginArgs, constParams, pluginParams, scalaSources); - ScalacInvokerResults compilerResults; - - try { - compilerResults = ScalacInvoker.invokeCompiler(ops, compilerArgs); - } catch (CompilationFailed exception) { - if (exception.getCause() instanceof ClassFormatError) { - throw new Exception( - "You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target.", - exception - ); - } - - throw exception; - } + ScalacInvokerResults compilerResults = ScalacInvoker.invokeCompiler(ops, compilerArgs); if (ops.printCompileTime) { System.err.println("Compiler runtime: " + (compilerResults.stopTime - compilerResults.startTime) + "ms."); diff --git a/src/java/io/bazel/rulesscala/scalac/scala_2/ScalacInvoker.java b/src/java/io/bazel/rulesscala/scalac/scala_2/ScalacInvoker.java index 4f1e17501..3b6fd319e 100644 --- a/src/java/io/bazel/rulesscala/scalac/scala_2/ScalacInvoker.java +++ b/src/java/io/bazel/rulesscala/scalac/scala_2/ScalacInvoker.java @@ -11,14 +11,14 @@ //Invokes Scala 2 compiler class ScalacInvoker{ - + public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] compilerArgs) throws IOException, Exception{ ReportableMainClass comp = new ReportableMainClass(ops); ScalacInvokerResults results = new ScalacInvokerResults(); - + results.startTime = System.currentTimeMillis(); try { comp.process(compilerArgs); @@ -28,7 +28,15 @@ public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] c } else if (ex.toString().contains("java.lang.StackOverflowError")) { throw new ScalacWorker.CompilationFailed("with StackOverflowError", ex); } else if (isMacroException(ex)) { - throw new ScalacWorker.CompilationFailed("during macro expansion", ex); + String reason; + + if (ex instanceof ClassFormatError) { + reason = "during macro expansion. You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target"; + } else { + reason = "during macro expansion"; + } + + throw new ScalacWorker.CompilationFailed(reason, ex); } else { throw ex; } @@ -37,7 +45,7 @@ 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, diff --git a/test/shell/test_macros.sh b/test/shell/test_macros.sh index 05ef82490..468c87bca 100755 --- a/test/shell/test_macros.sh +++ b/test/shell/test_macros.sh @@ -6,7 +6,7 @@ runner=$(get_test_runner "${1:-local}") incorrect_macro_user_does_not_build() { (! bazel build //test/macros:incorrect-macro-user) |& - grep --fixed-strings 'java.lang.Exception: You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target.' + grep --fixed-strings 'Build failure during macro expansion. You may have declared a target containing a macro as a `scala_library` target instead of a `scala_macro_library` target' } correct_macro_user_builds() {