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.

Sign up for GitHub

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

Jump to bottom

Scala 2.13: Fix various syntax errors #25425

Merged
merged 8 commits into from
Aug 26, 2022
Merged

Scala 2.13: Fix various syntax errors #25425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ebc0672
Remove call in deprecated `seq` method of Iterable
ioannakok Aug 23, 2022
b596f2c
Replace deprecated unicode escapes in triple quoted strings
ioannakok Aug 23, 2022
9993fb3
Use linesIterator on String instead of lines on a WrappedString
ioannakok Aug 23, 2022
53e894d
Be strict about using a Long value
ioannakok Aug 23, 2022
7aaf61a
Replace deprecated Traversable with Iterable
ioannakok Aug 23, 2022
027bd32
Replace deprecated Stream with LazyList
ioannakok Aug 23, 2022
2fbfdcf
Add @uncheched in superfluous pattern matching check
ioannakok Aug 23, 2022
344573f
Replace deprecated method replaceAllLiterally with replace in a Strin…
ioannakok Aug 23, 2022
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
2 changes: 1 addition & 1 deletion admin/app/tools/charts/charts.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ChartTable(private val labels: Seq[String]) {
ChartRow(dateFormat(new DateTime(row._1)), row._2)
}

chartRows.seq
chartRows
}
}

Expand Down
5 changes: 2 additions & 3 deletions common/app/common/StringEncodings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ object StringEncodings {
*/
def jsonToJS(json: String): String =
json
.replaceAll("""\u2028""", """\\u2028""")
.replaceAll("""\u2029""", """\\u2029""")

.replaceAll("\u2028", """\\u2028""")
.replaceAll("\u2029", """\\u2029""")
}
2 changes: 1 addition & 1 deletion common/app/common/TrailsToShowcase.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ object TrailsToShowcase {
}

private def extractBulletsFrom(trailText: String): Either[Seq[String], BulletList] = {
val lines = new WrappedString(trailText).lines.toSeq
val lines = trailText.linesIterator.toSeq

val proposedBulletTexts = lines
.map(_.stripLeading)
Expand Down
15 changes: 13 additions & 2 deletions common/app/common/configuration.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,24 @@ class GuardianConfiguration extends GuLogging {

lazy val key: Option[String] = configuration.getStringProperty("content.api.key")
lazy val timeout: FiniteDuration =
Duration.create(configuration.getIntegerProperty("content.api.timeout.millis").getOrElse(2000), MILLISECONDS)
Duration.create(
configuration
.getIntegerProperty("content.api.timeout.millis")
.getOrElse(2000L)
.asInstanceOf[Number]
.longValue(),
mxdvl marked this conversation as resolved.
Show resolved Hide resolved
MILLISECONDS,
)

lazy val circuitBreakerErrorThreshold: Int =
configuration.getIntegerProperty("content.api.circuit_breaker.max_failures").getOrElse(30)
lazy val circuitBreakerResetTimeout: FiniteDuration =
FiniteDuration(
configuration.getIntegerProperty("content.api.circuit_breaker.reset_timeout").getOrElse(2000),
configuration
.getIntegerProperty("content.api.circuit_breaker.reset_timeout")
.getOrElse(2000L)
.asInstanceOf[Number]
.longValue(),
MILLISECONDS,
)

Expand Down
2 changes: 1 addition & 1 deletion common/app/model/Badges.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ object Badges {
badgeForTags(c.tags.tags.map(_.id))
}

def badgeForTags(tags: Traversable[String]): Option[Badge] = {
def badgeForTags(tags: Iterable[String]): Option[Badge] = {

val badgesForTags =
for {
Expand Down
10 changes: 6 additions & 4 deletions common/app/model/CrosswordGridModel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
package crosswords

import com.gu.contentapi.client.model.v1.{
CrosswordEntry,
CrosswordDimensions,
Crossword,
CrosswordDimensions,
CrosswordEntry,
CrosswordPosition => ApiCrosswordPosition,
}
import model.{Entry, CrosswordPosition}
import model.{CrosswordPosition, Entry}

import Function.const
import scala.collection.compat.immutable.LazyList

trait CrosswordGridDataOrdering {
implicit val positionOrdering = Ordering.by[CrosswordPosition, (Int, Int)](position => (position.y, position.x))
}

trait CrosswordGridColumnNotation {
val columnsByLetters =
(('A' to 'Z').toList.zip(Stream from 0) map { case (letter, number) => (number, letter) }).toMap
(('A' to 'Z').toList.zip(LazyList from 0) map { case (letter, number) => (number, letter) }).toMap
}

case class Cell(number: Option[Int])
Expand Down
2 changes: 1 addition & 1 deletion common/app/model/facia.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ object SeoData extends GuLogging {
val editions = Edition.all.map(_.id.toLowerCase)

def fromPath(path: String): SeoData =
path.split('/').toList match {
(path.split('/').toList: @unchecked) match { // split() never gives the empty list
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this always true? What about an empty path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the following:

val s = ""
s.split('/')

Evaluates:
val s: String = ""
val res0: Array[String] = Array("")


val s = " "
s.split('/')

Evaluates:
val s: String = " "
val res0: Array[String] = Array(" ")

//This case is only to handle the nonevent of uk/technology/games
case edition :: section :: name :: tail if editions.contains(edition.toLowerCase) =>
val webTitle: String = webTitleFromTail(name :: tail)
Expand Down
2 changes: 1 addition & 1 deletion common/app/pagepresser/HtmlCleaner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ abstract class HtmlCleaner extends GuLogging {
}

private def secureSource(src: String): String = {
src.replaceAllLiterally("http://", "//")
src.replace("http://", "//")
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be replaceAll like in StringEncodings.scala - only asking because the original method is a type of replaceAll - unlikely that there will be more than one "http://" but just in case?

Copy link
Contributor Author

@ioannakok ioannakok Aug 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for your suggestion!

Short version

We're using the recommendation of the error here so replace is what we need:

[error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/pagepresser/HtmlCleaner.scala:200:9: method replaceAllLiterally in class StringOps is deprecated (since 2.13.2): Use `s.replace` as an exact replacement
[error]     src.replaceAllLiterally("http://", "//")
[error]         ^

See also: https://www.scala-lang.org/api/current/scala/collection/StringOps.html in section Deprecated Value Members def replaceAllLiterally`.

Long version (feel free to skip! 🙂)

I took your suggestion as an opportunity to understand this a bit better and here is what I found:

  • src is of type java.lang.String so it has access to all its methods.
  • But it is also a scala.collection.immutable.StringOps and has access to some extra methods. replaceAllLiterally is one of them.
  • How is it possible to be both a Java String and a Scala StringOps? Scala offers the magic of implicit conversions, which "give the Scala compiler the ability to convert one type into another" (I'm like 🤯 but that's the way it is.)
  • So what happened here is replaceAllLiterally of scala.collection.immutable.StringOps got deprecated but we can use replace (of java.lang.String) "as an exact replacement".
  • Why replace and not replaceAll?
    From the docs replace is actually doing the same thing as replaceAllLiterally, they replace all the occurrences of a substring, whereas replaceAll replaces all the occurrences of a given regex.

Here is a great explanation of how implicit conversion works on a String and a more in depth tutorial of how implicit conversions work where I found the above info.

}

}