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

Scala 2 13 #1118

Merged
merged 28 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -167,24 +167,22 @@ rbe_autoconfig(

load("//third_party/repositories:repositories.bzl", "repositories")

# `jvm_maven_import_external` is required by scala_import tests
jvm_maven_import_external(
name = "org_typelevel__cats_core",
artifact = scala_mvn_artifact(
"org.typelevel:cats-core:0.9.0",
"org.typelevel:cats-core:2.2.0",
SCALA_MAJOR_VERSION,
),
artifact_sha256 = "3ca705cba9dc0632e60477d80779006f8c636c0e2e229dda3410a0c314c1ea1d",
artifact_sha256 = "d7b6f800b50028c61ebed12763a100f31e900eb2f7a4af0d68f9aaaf19c24dc3",
server_urls = MAVEN_SERVER_URLS,
)

repositories(
fetch_sources = False,
for_artifact_ids = [
# test adding a scala jar:
"com_twitter__scalding_date",
# For testing that we don't include sources jars to the classpath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a misleading leftover from repositories refactoring. Also the last supproted org_psywerx_hairyfotr__linter is for 2.12.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe I didn't notice that before (in the repositories refactoring) but from the comment it still sounds like we need to test that we don't include source jars to the classpath

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liucijus not for this PR can you please check this? We either need to remove # "org_typelevel__cats_core" and move the comment up or we should uncomment this and remove the jvm_maven_import_extrernal from above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably it's ok to stay without this comment as it is the duplicate of 1b141a9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On master, line 183

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ittaiz do you expect me to do anything more regarding this thread?

Copy link
Member

@ittaiz ittaiz Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this commnet still stands. I can merge it and we'll do a followup PR just for it (a bit of a shame but I can live with it). What do you prefer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer merge

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging now. Please followup on this

# "org_typelevel__cats_core",
# test of a plugin
"org_psywerx_hairyfotr__linter",
# test of strict deps (scalac plugin UT + E2E)
"com_google_guava_guava_21_0_with_file",
"com_github_jnr_jffi_native",
Expand Down
51 changes: 28 additions & 23 deletions scala/scalafmt/scalafmt_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load(
_default_maven_server_urls = "default_maven_server_urls",
)
load("//third_party/repositories:repositories.bzl", "repositories")
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION")

def scalafmt_default_config(path = ".scalafmt.conf"):
build = []
Expand All @@ -16,30 +17,34 @@ def scalafmt_default_config(path = ".scalafmt.conf"):
def scalafmt_repositories(
maven_servers = _default_maven_server_urls(),
overriden_artifacts = {}):
artifact_ids = [
"org_scalameta_common",
"org_scalameta_fastparse",
"org_scalameta_fastparse_utils",
"org_scalameta_parsers",
"org_scalameta_scalafmt_core",
"org_scalameta_scalameta",
"org_scalameta_trees",
"org_typelevel_paiges_core",
"com_typesafe_config",
"org_scala_lang_scalap",
"com_thesamet_scalapb_lenses",
"com_thesamet_scalapb_scalapb_runtime",
"com_lihaoyi_fansi",
"com_lihaoyi_fastparse",
"org_scalameta_fastparse_utils",
"org_scala_lang_modules_scala_collection_compat",
"com_lihaoyi_pprint",
"com_lihaoyi_sourcecode",
"com_google_protobuf_protobuf_java",
"com_geirsson_metaconfig_core",
"com_geirsson_metaconfig_typesafe_config",
]
if SCALA_MAJOR_VERSION == "2.13":
artifact_ids.append("io_bazel_rules_scala_scala_parallel_collections")

repositories(
for_artifact_ids = [
"org_scalameta_common",
"org_scalameta_fastparse",
"org_scalameta_fastparse_utils",
"org_scalameta_parsers",
"org_scalameta_scalafmt_core",
"org_scalameta_scalameta",
"org_scalameta_trees",
"org_typelevel_paiges_core",
"com_typesafe_config",
"org_scala_lang_scalap",
"com_thesamet_scalapb_lenses",
"com_thesamet_scalapb_scalapb_runtime",
"com_lihaoyi_fansi",
"com_lihaoyi_fastparse",
"org_scalameta_fastparse_utils",
"org_scala_lang_modules_scala_collection_compat",
"com_lihaoyi_pprint",
"com_lihaoyi_sourcecode",
"com_google_protobuf_protobuf_java",
"com_geirsson_metaconfig_core",
"com_geirsson_metaconfig_typesafe_config",
],
for_artifact_ids = artifact_ids,
maven_servers = maven_servers,
fetch_sources = True,
overriden_artifacts = overriden_artifacts,
Expand Down
16 changes: 16 additions & 0 deletions scala/support/BUILD
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
load("//scala:scala.bzl", "scala_library")
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION")

scala_library(
name = "test_reporter",
srcs = ["JUnitXmlReporter.scala"],
scalacopts = [
"-deprecation:true",
"-encoding",
"UTF-8",
"-feature",
"-language:existentials",
"-language:higherKinds",
"-language:implicitConversions",
"-unchecked",
"-Xfatal-warnings",
"-Xlint",
"-Ywarn-dead-code",
"-Ywarn-numeric-widen",
"-Ywarn-value-discard",
"-Wunused:imports",
] if SCALA_MAJOR_VERSION == "2.13" else [
"-deprecation:true",
"-encoding",
"UTF-8",
Expand Down
25 changes: 2 additions & 23 deletions src/java/io/bazel/rulesscala/scalac/ProtoReporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,8 @@ public void writeTo(Path path) throws IOException {
}

@Override
public void info0(Position pos, String msg, Object severity, boolean force) {
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
Severity actualSeverity = (Severity) severity;
if(severity.equals(INFO())){
if(isVerbose || force){
actualSeverity.count_$eq(actualSeverity.count());
display(pos, msg, actualSeverity);
}
} else {
boolean hidden = testAndLog(pos, actualSeverity, msg);
if (!severity.equals(WARNING()) || !noWarnings) {
if(!hidden || isPromptSet){
actualSeverity.count_$eq(actualSeverity.count() + 1);
display(pos, msg, actualSeverity);
}
else if(isDebug){
actualSeverity.count_$eq(actualSeverity.count() + 1);
display(pos, "[ suppressed ] " + msg, actualSeverity);
}

if(isPromptSet)
displayPrompt();
}
}
public void info0(Position pos, String msg, Severity severity, boolean force) {
super.info0(pos, msg, severity, force);

Diagnostics.Diagnostic diagnostic = Diagnostics.Diagnostic
.newBuilder()
Expand Down
1 change: 0 additions & 1 deletion src/java/io/bazel/rulesscala/scalac/ScalacWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource
}

if (reporter.hasErrors()) {
reporter.printSummary();
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
reporter.flush();
throw new RuntimeException("Build failed");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.junit.runner.Runner
import org.junit.runners.BlockJUnit4ClassRunner
import org.junit.runners.model.{FrameworkMethod, RunnerBuilder, TestClass}

import scala.collection.JavaConversions._
import scala.collection.JavaConverters._

object FilteredRunnerBuilder {
type FilteringRunnerBuilder = PartialFunction[(Runner, Class[_], Pattern), Runner]
Expand All @@ -31,8 +31,10 @@ class FilteredRunnerBuilder(builder: RunnerBuilder, filteringRunnerBuilder: Filt
private[rulesscala] class FilteredTestClass(testClass: Class[_], pattern: Pattern) extends TestClass(testClass) {
override def getAnnotatedMethods(aClass: Class[_ <: Annotation]): util.List[FrameworkMethod] = {
val methods = super.getAnnotatedMethods(aClass)
if (aClass == classOf[Test]) methods.filter(method => methodMatchesPattern(method, pattern))
else methods
if (aClass == classOf[Test])
methods.asScala.filter(method => methodMatchesPattern(method, pattern)).asJava
else
methods
}

private def methodMatchesPattern(method: FrameworkMethod, pattern: Pattern): Boolean = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import java.util.jar.{ JarFile, JarEntry }
import java.util.logging.Level
import scala.collection.concurrent.TrieMap
import scala.collection.mutable
import scala.collection.JavaConverters._

object CompilerDefaults {
var language: String = "scala"
Expand Down Expand Up @@ -139,7 +140,8 @@ class Compiler {
language,
resolvedDoc,
defaultNamespace,
experimentFlags)
experimentFlags.toList
)

generator match {
case g: ScalaGenerator => g.warnOnJavaNamespaceFallback = scalaWarnOnJavaNSFallback
Expand Down
3 changes: 1 addition & 2 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ scala_binary(
scala_library(
name = "HelloLib",
srcs = ["HelloLib.scala"],
plugins = ["@org_psywerx_hairyfotr__linter//jar"],
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
deps = [
"Exported",
"MacroTest",
Expand Down Expand Up @@ -281,7 +280,7 @@ scala_repl(

scala_library(
name = "jar_export",
exports = ["@com_twitter__scalding_date//jar"],
exports = ["@org_typelevel__cats_core//jar"],
)

#Mix java scala
Expand Down
2 changes: 1 addition & 1 deletion test/coverage/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ java_test(
"TestB2.java",
],
tags = ["manual"],
test_class = "TestB2",
test_class = "coverage.TestB2",
deps = [
":b2",
],
Expand Down
5 changes: 0 additions & 5 deletions test/coverage/D1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ object D1 {
_ <- l
_ <- l
_ <- l
_ <- l
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
_ <- l
_ <- l
_ <- l
_ <- l
} yield {
()
}
Expand Down
2 changes: 1 addition & 1 deletion test/coverage/expected-coverage.dat
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ FNH:3
DA:3,1
DA:5,6
DA:9,8
DA:46,4
DA:41,4
LH:4
LF:4
end_of_record
Expand Down
10 changes: 5 additions & 5 deletions test/shell/test_misc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ test_transitive_deps() {

test_repl() {
bazel build $(bazel query 'kind(scala_repl, //test/...)')
echo "import scalarules.test._; HelloLib.printMessage(\"foo\")" | bazel-bin/test/HelloLibRepl | grep "foo java" &&
echo "import scalarules.test._; TestUtil.foo" | bazel-bin/test/HelloLibTestRepl | grep "bar" &&
echo "import scalarules.test._; ScalaLibBinary.main(Array())" | bazel-bin/test/ScalaLibBinaryRepl | grep "A hui hou" &&
echo "import scalarules.test._; ResourcesStripScalaBinary.main(Array())" | bazel-bin/test/ResourcesStripScalaBinaryRepl | grep "More Hello"
echo "import scalarules.test._; A.main(Array())" | bazel-bin/test/ReplWithSources | grep "4 8 15"
echo "import scalarules.test._; HelloLib.printMessage(\"foo\")" | bazel-bin/test/HelloLibRepl -Xnojline | grep "foo java" &&
echo "import scalarules.test._; TestUtil.foo" | bazel-bin/test/HelloLibTestRepl -Xnojline | grep "bar" &&
echo "import scalarules.test._; ScalaLibBinary.main(Array())" | bazel-bin/test/ScalaLibBinaryRepl -Xnojline | grep "A hui hou" &&
echo "import scalarules.test._; ResourcesStripScalaBinary.main(Array())" | bazel-bin/test/ResourcesStripScalaBinaryRepl -Xnojline | grep "More Hello"
echo "import scalarules.test._; A.main(Array())" | bazel-bin/test/ReplWithSources -Xnojline | grep "4 8 15"
Comment on lines +54 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the TLDR of these changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to pass -Xnojline to repl tests, to avoid loading jline
scala/bug#11654

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, should we be passing this arg in the production wrapper of the REPL? Not 100% sure but came to think about it when I wanted to ask you to document this but then thought- why document when we can fix. WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's a choice of user if they want to have jline on the classpath and use it. Probably if someone cares about repl functionallity, this should be properly solved by adding dep provider for repl first.

}

test_benchmark_jmh() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package scalarules.test.src.main.scala.scalarules.test.compiler_plugin
package scalarules.test.compiler_plugin

import scala.language.higherKinds

Expand Down
2 changes: 1 addition & 1 deletion test_expect_failure/scala_import/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ load("//scala:scala_import.bzl", "scala_import")
scala_import(
name = "dummy_dependency_to_trigger_create_provider_transitive_compile_jar_usage",
testonly = True,
jars = ["@org_psywerx_hairyfotr__linter//jar"],
jars = ["@org_springframework_spring_core//jar"],
)

scala_import(
Expand Down
2 changes: 1 addition & 1 deletion test_reproducibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ test_build_is_identical() {
bazel build --collect_code_coverage --extra_toolchains="//scala:code_coverage_toolchain" -- //test/coverage/...
non_deploy_jar_md5_sum > hash1
bazel clean
sleep 2 # to make sure that if timestamps slip in we get different ones
sleep 10 # to make sure that if timestamps slip in we get different ones
random_dir=/tmp/$RANDOM
bazel build --disk_cache $random_dir test/...
bazel build --disk_cache $random_dir --collect_code_coverage --extra_toolchains="//scala:code_coverage_toolchain" -- //test/coverage/...
Expand Down
7 changes: 2 additions & 5 deletions test_version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set -e

scala_2_11_version="2.11.12"
scala_2_12_version="2.12.11"
scala_2_13_version="2.13.3"

SCALA_VERSION_DEFAULT=$scala_2_11_version
SCALA_VERSION_SHAS_DEFAULT=$scala_2_11_shas
Expand Down Expand Up @@ -41,11 +42,6 @@ run_in_test_repo() {

test_scala_version() {
local SCALA_VERSION="$1"
if [[ $SCALA_VERSION == $scala_2_12_version ]]; then
local SCALA_VERSION_SHAS=$scala_2_12_shas
else
local SCALA_VERSION_SHAS=$scala_2_11_shas
fi

run_in_test_repo "test //..." "scala_version"
}
Expand Down Expand Up @@ -123,6 +119,7 @@ export USE_BAZEL_VERSION=${USE_BAZEL_VERSION:-$(cat $dir/.bazelversion)}

TEST_TIMEOUT=15 $runner test_scala_version "${scala_2_11_version}"
TEST_TIMEOUT=15 $runner test_scala_version "${scala_2_12_version}"
TEST_TIMEOUT=15 $runner test_scala_version "${scala_2_13_version}"

TEST_TIMEOUT=15 $runner test_twitter_scrooge_versions "18.6.0"
TEST_TIMEOUT=15 $runner test_twitter_scrooge_versions "20.5.0"
9 changes: 5 additions & 4 deletions test_version/version_specific_tests_dir/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,11 @@ scala_library(
srcs = glob(["src/main/scala/scalarules/test/mix_java_scala/*.scala"]) + glob([
"src/main/scala/scalarules/test/mix_java_scala/*.java",
]),
scalac_jvm_flags = [
"-Xms1G",
"-Xmx4G",
],
# disabled due to Travis CI memory failure
# scalac_jvm_flags = [
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
# "-Xms1G",
# "-Xmx4G",
# ],
)

#needed to test java sources are compiled
Expand Down
4 changes: 4 additions & 0 deletions third_party/dependency_analyzer/src/main/BUILD
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
licenses(["notice"]) # 3-clause BSD

load("//scala:scala.bzl", "scala_library_for_plugin_bootstrapping")
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_MAJOR_VERSION")

scala_library_for_plugin_bootstrapping(
name = "scala_version",
Expand All @@ -16,6 +17,8 @@ scala_library_for_plugin_bootstrapping(
],
)

REPORTER_COMPATIBILITY = "213" if SCALA_MAJOR_VERSION == "2.13" else ""

scala_library_for_plugin_bootstrapping(
name = "dependency_analyzer",
srcs = [
Expand All @@ -24,6 +27,7 @@ scala_library_for_plugin_bootstrapping(
"io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerSettings.scala",
"io/bazel/rulesscala/dependencyanalyzer/HighLevelCrawlUsedJarFinder.scala",
"io/bazel/rulesscala/dependencyanalyzer/OptionsParser.scala",
"io/bazel/rulesscala/dependencyanalyzer/Reporter%s.scala" % REPORTER_COMPATIBILITY,
],
resources = ["resources/scalac-plugin.xml"],
visibility = ["//visibility:public"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import scala.tools.nsc.Global
import scala.tools.nsc.Phase

class DependencyAnalyzer(val global: Global) extends Plugin {

private val reporter = new Reporter(global)
override val name = "dependency-analyzer"
override val description =
"Analyzes the used dependencies. Can check and warn or fail the " +
Expand Down Expand Up @@ -148,13 +148,13 @@ class DependencyAnalyzer(val global: Global) extends Plugin {
errors: Map[String, global.Position]
): Unit = {
val reportFunction: (String, global.Position) => Unit = analyzerMode match {
case AnalyzerMode.Error =>
{ case (message, pos) =>
global.reporter.error(pos, message)
}
case AnalyzerMode.Warn =>
{ case (message, pos) =>
global.reporter.warning(pos, message)
case AnalyzerMode.Error => {
case (message, pos) =>
reporter.error(pos, message)
}
case AnalyzerMode.Warn => {
case (message, pos) =>
reporter.warning(pos, message)
}
case AnalyzerMode.Off => (_, _) => ()
}
Expand Down
Loading