From f9381414068466b9c74ff7681d204e1eb19c7f80 Mon Sep 17 00:00:00 2001 From: Sebastien Coquelin Date: Mon, 27 Nov 2023 06:53:48 -0800 Subject: [PATCH] Fix diagnostics for Scala 2.13.12 (#1522) * Fix diagnostics for Scala 2.13.12 * Fix sha256sum for org.scalameta:semanticdb-scalac_2_11:4.8.6 * Add Tally as an adopter * Fix ProtoReporter issue with semanticdb in Scala 2.12.x --- README.md | 1 + .../io/bazel/rulesscala/scalac/reporter/BUILD | 12 ++- .../ProtoReporter.java | 102 ++++++++++++++++++ .../reporter/after_2_13_12/ProtoReporter.java | 99 +++++++++++++++++ .../{ => before_2_12_13}/ProtoReporter.java | 14 ++- test/diagnostics_reporter/BUILD | 6 +- .../DiagnosticsReporterTest.java | 66 ++++++++++++ .../DiagnosticsReporterTest.java | 0 test_version.sh | 12 +++ third_party/repositories/scala_2_11.bzl | 2 +- 10 files changed, 304 insertions(+), 10 deletions(-) create mode 100644 src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java create mode 100644 src/java/io/bazel/rulesscala/scalac/reporter/after_2_13_12/ProtoReporter.java rename src/java/io/bazel/rulesscala/scalac/reporter/{ => before_2_12_13}/ProtoReporter.java (97%) create mode 100644 test/diagnostics_reporter/after_2_13_12/DiagnosticsReporterTest.java rename test/diagnostics_reporter/{ => before_2_13_12}/DiagnosticsReporterTest.java (100%) diff --git a/README.md b/README.md index 70a0d5d3a..3bbc0aeb3 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,7 @@ Here's a (non-exhaustive) list of companies that use `rules_scala` in production * [Meetup](https://meetup.com/) * [Spotify](https://www.spotify.com/) * [Stripe](https://stripe.com/) +* [Tally](https://www.meettally.com/) * [Twitter](https://twitter.com/) * [VSCO](https://vsco.co) * [Wix](https://www.wix.com/) diff --git a/src/java/io/bazel/rulesscala/scalac/reporter/BUILD b/src/java/io/bazel/rulesscala/scalac/reporter/BUILD index b9c7f2a4f..70659052b 100644 --- a/src/java/io/bazel/rulesscala/scalac/reporter/BUILD +++ b/src/java/io/bazel/rulesscala/scalac/reporter/BUILD @@ -1,13 +1,19 @@ load("@rules_proto//proto:defs.bzl", "proto_library") load("@rules_java//java:defs.bzl", "java_library", "java_proto_library") -load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION") +load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION", "SCALA_MINOR_VERSION") java_library( name = "reporter", srcs = [ "//src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter", - "ProtoReporter.java", - ] if SCALA_MAJOR_VERSION.startswith("2") else ["PlaceholderForEmptyScala3Lib.java"], + "before_2_12_13/ProtoReporter.java", + ] if SCALA_MAJOR_VERSION.startswith("2.11") or (SCALA_MAJOR_VERSION.startswith("2.12") and int(SCALA_MINOR_VERSION) < 13) else [ + "//src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter", + "after_2_12_13_and_before_2_13_12/ProtoReporter.java", + ] if ((SCALA_MAJOR_VERSION.startswith("2.12") and int(SCALA_MINOR_VERSION) >= 13) or (SCALA_MAJOR_VERSION.startswith("2.13") and int(SCALA_MINOR_VERSION) < 12)) else [ + "//src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter", + "after_2_13_12/ProtoReporter.java", + ] if (SCALA_MAJOR_VERSION.startswith("2.13") and int(SCALA_MINOR_VERSION) >= 12) else ["PlaceholderForEmptyScala3Lib.java"], visibility = ["//visibility:public"], deps = [ ":scala_deps_java_proto", diff --git a/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java b/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java new file mode 100644 index 000000000..abc69bbc6 --- /dev/null +++ b/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java @@ -0,0 +1,102 @@ +package io.bazel.rulesscala.scalac.reporter; + +import io.bazel.rules_scala.diagnostics.Diagnostics; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.*; +import scala.reflect.internal.util.Position; +import scala.reflect.internal.util.RangePosition; +import scala.tools.nsc.Settings; +import scala.tools.nsc.reporters.ConsoleReporter; + +public class ProtoReporter extends ConsoleReporter { + + private final Map> builder; + + public ProtoReporter(Settings settings) { + super(settings); + builder = new LinkedHashMap<>(); + } + + @Override + public void reset() { + super.reset(); + } + + public void writeTo(Path path) throws IOException { + Diagnostics.TargetDiagnostics.Builder targetDiagnostics = + Diagnostics.TargetDiagnostics.newBuilder(); + for (Map.Entry> entry : builder.entrySet()) { + targetDiagnostics.addDiagnostics( + Diagnostics.FileDiagnostics.newBuilder() + .setPath(entry.getKey()) + .addAllDiagnostics(entry.getValue())); + } + Files.write( + path, + targetDiagnostics.build().toByteArray(), + StandardOpenOption.CREATE, + StandardOpenOption.APPEND); + } + + @Override + public void info0(Position pos, String msg, Severity severity, boolean force) { + doReport(pos, msg, severity); + } + + @Override + public void doReport(Position pos, String msg, Severity severity) { + super.doReport(pos, msg, severity); + + Diagnostics.Diagnostic diagnostic = + Diagnostics.Diagnostic.newBuilder() + .setRange(positionToRange(pos)) + .setSeverity(convertSeverity(severity)) + .setMessage(msg) + .build(); + // TODO: Handle generated files + String uri = "workspace-root://" + pos.source().file().path(); + List diagnostics = builder.computeIfAbsent(uri, key -> new ArrayList()); + diagnostics.add(diagnostic); + } + + private Diagnostics.Severity convertSeverity(Object severity) { + String stringified = severity.toString().toLowerCase(); + if ("error".equals(stringified)) { + return Diagnostics.Severity.ERROR; + } else if ("warning".equals(stringified)) { + return Diagnostics.Severity.WARNING; + } else if ("info".equals(stringified)) { + return Diagnostics.Severity.INFORMATION; + } + throw new RuntimeException("Unknown severity: " + stringified); + } + + private Diagnostics.Range positionToRange(Position pos) { + if (pos instanceof RangePosition) { + RangePosition rangePos = (RangePosition) pos; + int startLine = pos.source().offsetToLine(rangePos.start()); + int endLine = pos.source().offsetToLine(rangePos.end()); + return Diagnostics.Range.newBuilder() + .setStart( + Diagnostics.Position.newBuilder() + .setLine(startLine) + .setCharacter(rangePos.start() - pos.source().lineToOffset(startLine))) + .setEnd( + Diagnostics.Position.newBuilder() + .setLine(endLine) + .setCharacter(rangePos.end() - pos.source().lineToOffset(endLine)) + .build()) + .build(); + } + return Diagnostics.Range.newBuilder() + .setStart( + Diagnostics.Position.newBuilder() + .setLine(pos.line() - 1) + .setCharacter(pos.column() - 1)) + .setEnd(Diagnostics.Position.newBuilder().setLine(pos.line())) + .build(); + } +} diff --git a/src/java/io/bazel/rulesscala/scalac/reporter/after_2_13_12/ProtoReporter.java b/src/java/io/bazel/rulesscala/scalac/reporter/after_2_13_12/ProtoReporter.java new file mode 100644 index 000000000..c26f42b78 --- /dev/null +++ b/src/java/io/bazel/rulesscala/scalac/reporter/after_2_13_12/ProtoReporter.java @@ -0,0 +1,99 @@ +package io.bazel.rulesscala.scalac.reporter; + +import io.bazel.rules_scala.diagnostics.Diagnostics; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.*; +import scala.collection.immutable.List$; +import scala.reflect.internal.util.CodeAction; +import scala.reflect.internal.util.Position; +import scala.reflect.internal.util.RangePosition; +import scala.tools.nsc.Settings; +import scala.tools.nsc.reporters.ConsoleReporter; + +public class ProtoReporter extends ConsoleReporter { + + private final Map> builder; + + public ProtoReporter(Settings settings) { + super(settings); + builder = new LinkedHashMap<>(); + } + + @Override + public void reset() { + super.reset(); + } + + public void writeTo(Path path) throws IOException { + Diagnostics.TargetDiagnostics.Builder targetDiagnostics = + Diagnostics.TargetDiagnostics.newBuilder(); + for (Map.Entry> entry : builder.entrySet()) { + targetDiagnostics.addDiagnostics( + Diagnostics.FileDiagnostics.newBuilder() + .setPath(entry.getKey()) + .addAllDiagnostics(entry.getValue())); + } + Files.write( + path, + targetDiagnostics.build().toByteArray(), + StandardOpenOption.CREATE, + StandardOpenOption.APPEND); + } + + @Override + public void doReport(Position pos, String msg, Severity severity, scala.collection.immutable.List actions) { + super.doReport(pos, msg, severity, List$.MODULE$.empty()); + + Diagnostics.Diagnostic diagnostic = + Diagnostics.Diagnostic.newBuilder() + .setRange(positionToRange(pos)) + .setSeverity(convertSeverity(severity)) + .setMessage(msg) + .build(); + // TODO: Handle generated files + String uri = "workspace-root://" + pos.source().file().path(); + List diagnostics = builder.computeIfAbsent(uri, key -> new ArrayList()); + diagnostics.add(diagnostic); + } + + private Diagnostics.Severity convertSeverity(Object severity) { + String stringified = severity.toString().toLowerCase(); + if ("error".equals(stringified)) { + return Diagnostics.Severity.ERROR; + } else if ("warning".equals(stringified)) { + return Diagnostics.Severity.WARNING; + } else if ("info".equals(stringified)) { + return Diagnostics.Severity.INFORMATION; + } + throw new RuntimeException("Unknown severity: " + stringified); + } + + private Diagnostics.Range positionToRange(Position pos) { + if (pos instanceof RangePosition) { + RangePosition rangePos = (RangePosition) pos; + int startLine = pos.source().offsetToLine(rangePos.start()); + int endLine = pos.source().offsetToLine(rangePos.end()); + return Diagnostics.Range.newBuilder() + .setStart( + Diagnostics.Position.newBuilder() + .setLine(startLine) + .setCharacter(rangePos.start() - pos.source().lineToOffset(startLine))) + .setEnd( + Diagnostics.Position.newBuilder() + .setLine(endLine) + .setCharacter(rangePos.end() - pos.source().lineToOffset(endLine)) + .build()) + .build(); + } + return Diagnostics.Range.newBuilder() + .setStart( + Diagnostics.Position.newBuilder() + .setLine(pos.line() - 1) + .setCharacter(pos.column() - 1)) + .setEnd(Diagnostics.Position.newBuilder().setLine(pos.line())) + .build(); + } +} diff --git a/src/java/io/bazel/rulesscala/scalac/reporter/ProtoReporter.java b/src/java/io/bazel/rulesscala/scalac/reporter/before_2_12_13/ProtoReporter.java similarity index 97% rename from src/java/io/bazel/rulesscala/scalac/reporter/ProtoReporter.java rename to src/java/io/bazel/rulesscala/scalac/reporter/before_2_12_13/ProtoReporter.java index 243c01726..dd2e6c156 100644 --- a/src/java/io/bazel/rulesscala/scalac/reporter/ProtoReporter.java +++ b/src/java/io/bazel/rulesscala/scalac/reporter/before_2_12_13/ProtoReporter.java @@ -1,16 +1,20 @@ package io.bazel.rulesscala.scalac.reporter; import io.bazel.rules_scala.diagnostics.Diagnostics; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.util.*; import scala.reflect.internal.util.Position; import scala.reflect.internal.util.RangePosition; import scala.tools.nsc.Settings; import scala.tools.nsc.reporters.ConsoleReporter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + public class ProtoReporter extends ConsoleReporter { private final Map> builder; diff --git a/test/diagnostics_reporter/BUILD b/test/diagnostics_reporter/BUILD index db3bf7e05..09b1df76a 100644 --- a/test/diagnostics_reporter/BUILD +++ b/test/diagnostics_reporter/BUILD @@ -1,9 +1,13 @@ load("@rules_java//java:defs.bzl", "java_binary") +load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION", "SCALA_MINOR_VERSION") java_binary( name = "diagnostics_reporter_test", srcs = [ - "DiagnosticsReporterTest.java", + "before_2_13_12/DiagnosticsReporterTest.java", + "VerifyDiagnosticsOutput.java", + ] if SCALA_MAJOR_VERSION.startswith("2.11") or SCALA_MAJOR_VERSION.startswith("2.12") or (SCALA_MAJOR_VERSION.startswith("2.13") and int(SCALA_MINOR_VERSION) < 12) else [ + "after_2_13_12/DiagnosticsReporterTest.java", "VerifyDiagnosticsOutput.java", ], main_class = "diagnostics_reporter.DiagnosticsReporterTest", diff --git a/test/diagnostics_reporter/after_2_13_12/DiagnosticsReporterTest.java b/test/diagnostics_reporter/after_2_13_12/DiagnosticsReporterTest.java new file mode 100644 index 000000000..fd9e07c67 --- /dev/null +++ b/test/diagnostics_reporter/after_2_13_12/DiagnosticsReporterTest.java @@ -0,0 +1,66 @@ +package diagnostics_reporter; + +import io.bazel.rules_scala.diagnostics.Diagnostics; +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class DiagnosticsReporterTest { + @SuppressWarnings("DoubleBraceInitialization") + private static final Map tests = + new HashMap() { + { + put( + "error_file", + new diagnostics_reporter.VerifyDiagnosticsOutput[] { + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.ERROR, 5, 2, 6, 0) + }); + put( + "two_errors_file", + new diagnostics_reporter.VerifyDiagnosticsOutput[] { + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.ERROR, 4, 4, 4, 10), + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.ERROR, 5, 4, 5, 9) + }); + put( + "warning_file", + new diagnostics_reporter.VerifyDiagnosticsOutput[] { + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.WARNING, 0, 0, 0, 26) + }); + put( + "error_and_warning_file", + new diagnostics_reporter.VerifyDiagnosticsOutput[] { + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.WARNING, 0, 0, 0, 26), + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.ERROR, 4, 4, 4, 10) + }); + put( + "info_file", + new diagnostics_reporter.VerifyDiagnosticsOutput[] { + new diagnostics_reporter.VerifyDiagnosticsOutput( + Diagnostics.Severity.INFORMATION, -1, -1, 0, 0) + }); + } + }; + + public static void main(String[] args) throws IOException { + if (args.length != 1) throw new IllegalArgumentException("Args: "); + + String diagnosticsOutput = args[0]; + for (Map.Entry entry : tests.entrySet()) { + String test = entry.getKey(); + VerifyDiagnosticsOutput[] expectedDiagnosticsOutputs = entry.getValue(); + List diagnostics = + VerifyDiagnosticsOutput.getDiagnostics( + diagnosticsOutput + "/" + test + ".diagnosticsproto"); + for (VerifyDiagnosticsOutput expectedDiagnostic : expectedDiagnosticsOutputs) { + expectedDiagnostic.testOutput(diagnostics); + } + } + } +} diff --git a/test/diagnostics_reporter/DiagnosticsReporterTest.java b/test/diagnostics_reporter/before_2_13_12/DiagnosticsReporterTest.java similarity index 100% rename from test/diagnostics_reporter/DiagnosticsReporterTest.java rename to test/diagnostics_reporter/before_2_13_12/DiagnosticsReporterTest.java diff --git a/test_version.sh b/test_version.sh index ec0fa4a84..e47ab3c7a 100755 --- a/test_version.sh +++ b/test_version.sh @@ -68,6 +68,15 @@ test_reporter() { run_in_test_repo "compilation_should_fail build //... --repo_env=SCALA_VERSION=${SCALA_VERSION} --extra_toolchains=${SCALA_TOOLCHAIN}" "reporter" "test_reporter/" } +test_diagnostic_proto_files() { + local SCALA_VERSION="$1" + local SCALA_TOOLCHAIN="$2" + + compilation_should_fail build --build_event_publish_all_actions -k --repo_env=SCALA_VERSION=${SCALA_VERSION} --extra_toolchains=${SCALA_TOOLCHAIN} //test_expect_failure/diagnostics_reporter:all + diagnostics_output="$(bazel info bazel-bin)/test_expect_failure/diagnostics_reporter" + bazel run --repo_env=SCALA_VERSION=${SCALA_VERSION} //test/diagnostics_reporter:diagnostics_reporter_test "$diagnostics_output" +} + test_twitter_scrooge_versions() { local version_under_test=$1 @@ -111,3 +120,6 @@ TEST_TIMEOUT=15 $runner test_reporter "${scala_2_13_version}" "${no_diagnostics_ TEST_TIMEOUT=15 $runner test_reporter "${scala_2_11_version}" "${diagnostics_reporter_toolchain}" TEST_TIMEOUT=15 $runner test_reporter "${scala_2_12_version}" "${diagnostics_reporter_toolchain}" TEST_TIMEOUT=15 $runner test_reporter "${scala_2_13_version}" "${diagnostics_reporter_toolchain}" + +TEST_TIMEOUT=15 $runner test_diagnostic_proto_files "${scala_2_12_version}" "//test_expect_failure/diagnostics_reporter:diagnostics_reporter_toolchain" +TEST_TIMEOUT=15 $runner test_diagnostic_proto_files "${scala_2_13_version}" "//test_expect_failure/diagnostics_reporter:diagnostics_reporter_toolchain" diff --git a/third_party/repositories/scala_2_11.bzl b/third_party/repositories/scala_2_11.bzl index 4c90faa44..17ea4e377 100644 --- a/third_party/repositories/scala_2_11.bzl +++ b/third_party/repositories/scala_2_11.bzl @@ -79,7 +79,7 @@ artifacts = { }, "org_scalameta_semanticdb_scalac": { "artifact": "org.scalameta:semanticdb-scalac_%s:4.8.6" % scala_version, - "sha256": "1253abd1e8c8b3ec0720995d378846bbc439d168e7ea815fe91da6bef8b1de9d", + "sha256": "5700b0b425eac8e1d0c27660ae96879ef688731cc0d2f2e8cdc7e20496f87670", "deps": [ "@io_bazel_rules_scala_scala_library", ],