mbeckerle commented on code in PR #1431:
URL: https://github.com/apache/daffodil/pull/1431#discussion_r1953234578
##########
build.sbt:
##########
@@ -293,7 +293,10 @@ def buildScalacOptions(scalaVersion: String) = {
Seq(
"-Xlint:inaccessible",
"-Xlint:infer-any",
- "-Xlint:nullary-unit"
+ "-Xlint:nullary-unit",
+ // the import is needed for Scala 2.12 but issues an unused import
warning under 2.13, so we add this to
+ // suppresss the warning
Review Comment:
Whenever we do this sort of scala 2.12 compatibility thing, I think we
should anticipate that we will drop scala 2.12 fairly quickly once we've moved
to scala 2.13 or scala 3. So I would add a `// TODO: scala 2.12 phase out` or
other easily identified TODO item.
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/util/MultiMapWrapper.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.lib.util
+
+import scala.collection.mutable
+
+/**
+ * Compatibility class for 2.12 and 2.13 since MultiMap and inheritance
+ * from class mutable.HashMap have been deprecated in 2.13.
+ */
+class MultiMapWrapper[K, V] {
Review Comment:
Yes this is a wrapper, but I think this should just be called MultiMap.
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -953,7 +954,7 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent:
DFDLTestSuite) {
tunables
)
- val newCompileResult: TDML.CompileResult = compileResult.right.map {
+ val newCompileResult: TDML.CompileResult = compileResult.map {
Review Comment:
So compileResult isnt an `Either[L,R]` type anymore? That's a pretty major
change and seems unmotivated for this port to new scala version.
Can you explain why this was needed?
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/util/MultiMapWrapper.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.lib.util
+
+import scala.collection.mutable
+
+/**
+ * Compatibility class for 2.12 and 2.13 since MultiMap and inheritance
+ * from class mutable.HashMap have been deprecated in 2.13.
+ */
+class MultiMapWrapper[K, V] {
+ private val underlying = mutable.Map.empty[K, mutable.Set[V]]
+
+ def addBinding(key: K, value: V): Unit =
+ underlying.getOrElseUpdate(key, mutable.Set.empty) += value
+
+ def addBinding(key: K, values: mutable.Set[V]): Unit = {
+ values.foreach(addBinding(key, _))
+ }
+
+ def removeBinding(key: K, value: V): Unit =
+ underlying.get(key).foreach { values =>
+ values -= value
+ if (values.isEmpty) underlying -= key
Review Comment:
I'm going to suggest we just ignore these codecov as part of this scala
2.13/scala 3 conversion process.
We can always run a complete code-cov analysis later.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/debugger/InteractiveDebugger.scala:
##########
@@ -1862,7 +1863,7 @@ class InteractiveDebugger(
}
abstract class InfoProcessorBase extends DebugCommand with
DebugCommandValidateZeroArgs {
- val desc = "display the current Daffodil " + name
+ val desc = "display the current Daffodil " + this.name
Review Comment:
Q: What rule in scala 2.13 or scala 3 is requiring you to put "this." on
these member references?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
