Modernize scala syntax #87
Conversation
| facets.addFields(doc, List(new CategoryPath(name, value)).asJava) | ||
| private def addFields(doc: Document, field0: Any): Unit = { | ||
| field0 match { | ||
| case (name: String, value: String, options: List[_]) => |
There was a problem hiding this comment.
Should not we keep the parameter to the List[A] type here?
There was a problem hiding this comment.
If we would have List[A] then newer compiler would be producing warnings like this
warning: non-variable type argument (String, Any) in type pattern List[(String, Any)] (the underlying of List[(String, Any)]) is unchecked since it is eliminated by erasure
case (name: String, value: String, options: List[(String, Any)]) =>
^
New versions of scala do not have ability to match on inner type of containers. It cannot distinguish the following two cases
case strings: List[Strings] => ...
case integers: List[Int] => ...As a result Scala designers decided to introduce the warning. The List[_] signals to compiler that user is not trying to match on the type so it is fine use.
There was a problem hiding this comment.
This also applies to generics. For example I cannot do
def asListOf[T](input: List[Any]): List[T] = {
input match {
case list: List[T] => list.asInstanceOf[List[T]]
case _ => throw new Exception("")
}
}There was a problem hiding this comment.
Personally I would prefer to keep the type and find a way to disable the warning. Because in this case I know what I am doing. But I couldn't find the way to control the warning. I'll try again.
There was a problem hiding this comment.
BTW the following also not going to work.
input match {
case list if (list.isInstanceOf[List[String]]) => ...There was a problem hiding this comment.
I've found a way
case (name: String, value: String, options: List[(String, Any) @unchecked]) => ...There was a problem hiding this comment.
I'll add the types back and use @unchecked trick.
There was a problem hiding this comment.
Thank you @iilyak for both the explanation and the fix.
| * and throw an exception if it encounters a non-string element. | ||
| */ | ||
|
|
||
| def elementWise[Inner](pf: PartialFunction[Any, Inner], orElse: PartialFunction[(List[Any], Any), List[Inner]]): PartialFunction[Any, List[Inner]] = { |
|
Are you planning to update the CI as well? Apparently it tries to build the code with old Java (and Scala) and it fails. |
This work doesn't make it possible to build full project with new Java/Scala. I just renovated syntax of Therefore updating CI is not needed. |
The `ValueSource` requires overriding `equals` and `hashCode()`. These two are automatically derived for case classes.
The previous implementation `case e => ...` produces following warning on
modern scala
```
warning: This catches all Throwables. If this is really intended, use
`case e : Throwable` to clear this warning.
case e =>
^
```
iilyak
force-pushed
the
modernize-scala-syntax
branch
from
a12e954 to
22d7615
Compare
| @@ -107,7 +107,7 @@ class ClouseauQueryParser(version: Version, | |||
| fpRegex.matcher(str).matches() | |||
| } | |||
|
|
|||
| private def setLowercaseExpandedTerms(field: String) { | |||
| private def setLowercaseExpandedTerms(field: String) = { | |||
There was a problem hiding this comment.
why avoid "procedural syntax" for all these functions where Unit is the appropriate result? Is it being removed in Scala 3?
There was a problem hiding this comment.
The procedural syntax has been removed in scala/scala#3076. I am not sure in which version of scala this landed though. I can find out the version.
There was a problem hiding this comment.
no need. that's a perfectly good reason for the change.
| lat: String, | ||
| multiplier: Double, | ||
| from: Point) | ||
| case class DistanceValueSource(ctx: SpatialContext, |
This PR removes deprecated forms of Scala syntax as well as
The main goal of the PR is to prepare the codebase for Scala version upgrade.