Fix some more collection errors #1431
Conversation
- fix implicit Array to Seq error using ArraySeq.unsafeWrapArray - fix inheritance shadowing error by renaming function and using this._ - fix deprecated Array to Scala varargs - add MultiMapWrapper class since MultiMaps and extending HashMaps are deprecated - replace deprecated Map.filterKeys with Map.filter( case(k,v) => k...) - replace deprecated Map.mapValues with suggested Map.values.map - remove deprecated Either.right since it's the default view now DAFFODIL-2152
olabusayoT
requested review from
mbeckerle,
stevedlawrence and
jadams-tresys
| "-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 |
There was a problem hiding this 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.
| * 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] { |
There was a problem hiding this comment.
Yes this is a wrapper, but I think this should just be called MultiMap.
| def removeBinding(key: K, value: V): Unit = | ||
| underlying.get(key).foreach { values => | ||
| values -= value | ||
| if (values.isEmpty) underlying -= key |
There was a problem hiding this 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.
...odil-runtime1/src/main/scala/org/apache/daffodil/runtime1/debugger/InteractiveDebugger.scala
Show resolved
Hide resolved
| @@ -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 { | |||
There was a problem hiding this 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?
There was a problem hiding this comment.
compileResult is still an Either[L,R], but .right is deprecated since it is now the default representation. .left still works though
"Either is right-biased, which means that Right is assumed to be the default case to operate on. "
https://www.scala-lang.org/api/2.13.16/scala/util/Either.html
There was a problem hiding this comment.
Should I add a comment in the code?
- MultiMapWrapper -> MultiMap - add comments for 2.12 phase out DAFFODIL-2152
- reformat DAFFODIL-2152
DAFFODIL-2152