Scala 2.13: Fix various syntax errors #25425

ioannakok · 2022-08-23T18:20:35Z

What does this change?

This is the last PR that can be pulled in Scala 2.12 from #25190. Changes the following (see commit messages for the errors we are getting rid off):

Removes call to deprecated seq method of Iterable
Replaces deprecated unicode escapes in triple quoted strings
Calls linesIterator on a String instead of lines on a WrappedString
Becomes stricter about using a Long value
Replaces deprecated Traversable with Iterable
Replaces deprecated Stream with LazyList
Adds @unchecked in superfluous pattern matching check
Replaces deprecated method replaceAllLiterally call with replace in a StringOps

To get rid of this error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/admin/app/tools/charts/charts.scala:50:15: method seq in trait Iterable is deprecated (since 2.13.0): Iterable.seq always returns the iterable itself [error] chartRows.seq [error] ^ ```

To get rid of the error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/common/StringEncodings.scala:19:22: Unicode escapes in triple quoted strings are deprecated, use the literal character instead [error] .replaceAll("""\u2028""", """\\u2028""") [error] ^ [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/common/StringEncodings.scala:20:22: Unicode escapes in triple quoted strings are deprecated, use the literal character instead [error] .replaceAll("""\u2029""", """\\u2029""") ```

`lines` is not a method of `WrappedString` anymore but we can call `linesIterator` on a String after it has been undeprecated in this PR: scala/scala#7269 This way we get rid of the following error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/common/TrailsToShowcase.scala:509:46: value lines is not a member of scala.collection.immutable.WrappedString [error] val lines = new WrappedString(trailText).lines.toSeq [error] ^ [error] one error found ```

Scala 2.13 compiler does not recoginise this is Long anymore so we have to be more specific. ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/common/configuration.scala:213:95: type mismatch; [error] found : AnyVal [error] required: Long [error] Duration.create(configuration.getIntegerProperty("content.api.timeout.millis").getOrElse(2000), MILLISECONDS) [error] ```

To get rid of the following error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/model/Badges.scala:174:26: type Traversable in package scala is deprecated (since 2.13.0): Use Iterable instead of Traversable [error] def badgeForTags(tags: Traversable[String]): Option[Badge] = { [error] ```

To get rid of the following error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/model/CrosswordGridModel.scala:18:30: value Stream in package scala is deprecated (since 2.13.0): Use LazyList instead of Stream [error] (('A' to 'Z').toList.zip(Stream from 0) map { case (letter, number) => (number, letter) }).toMap [error] ```

Scala 2.13 compiler is stricter in checking whether pattern matching has been exhaustive but in this case we don't need to check for `Nil` because split() never gives the empty list. To get rid off the following error: ``` [error] /Users/Ioanna_Kokkini/Projects/frontend/common/app/model/facia.scala:24:21: match may not be exhaustive. [error] It would fail on the following input: Nil [error] path.split('/').toList match { [error] ^ ```

…gOps To get rid of the following error: ``` [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] ^ ```

jlieb10 · 2022-08-24T10:00:11Z

common/app/pagepresser/HtmlCleaner.scala

@@ -197,7 +197,7 @@ abstract class HtmlCleaner extends GuLogging {
  }

  private def secureSource(src: String): String = {
-    src.replaceAllLiterally("http://", "//")
+    src.replace("http://", "//")


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?

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.

jlieb10

One non-blocking question- Looks good!

mxdvl · 2022-08-25T06:27:28Z

common/app/model/facia.scala

@@ -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


Is this always true? What about an empty path?

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(" ")

common/app/common/configuration.scala

prout-bot · 2022-08-26T10:13:51Z

Seen on PROD (merged by @ioannakok 16 minutes and 53 seconds ago)

Check your changes on www.theguardian.com ✔️
Keep an eye on the deploy dashboard 📉

ioannakok added 5 commits August 23, 2022 19:21

ioannakok force-pushed the scala-2.13/fix-various-syntax-errors branch from e036e89 to 315128b Compare August 23, 2022 18:22

ioannakok mentioned this pull request Aug 23, 2022

Switch to Scala 2.13 #25190

Merged

2 tasks

ioannakok added 3 commits August 23, 2022 19:37

ioannakok force-pushed the scala-2.13/fix-various-syntax-errors branch from 315128b to 344573f Compare August 23, 2022 18:37

ioannakok changed the title ~~Scala 2.13/fix various syntax errors~~ Scala 2.13: Fix various syntax errors Aug 24, 2022

ioannakok marked this pull request as ready for review August 24, 2022 08:45

ioannakok requested a review from a team as a code owner August 24, 2022 08:45

jlieb10 reviewed Aug 24, 2022

View reviewed changes

jlieb10 approved these changes Aug 24, 2022

View reviewed changes

mxdvl approved these changes Aug 25, 2022

View reviewed changes

ioannakok merged commit 7c88efb into main Aug 26, 2022

ioannakok deleted the scala-2.13/fix-various-syntax-errors branch August 26, 2022 09:56

prout-bot added Pending-on-PROD Seen-on-PROD and removed Pending-on-PROD labels Aug 26, 2022

mxdvl linked an issue Aug 31, 2022 that may be closed by this pull request

Scala 2.12 -> 2.13 #24817

Closed

mxdvl removed a link to an issue Aug 31, 2022

Scala 2.12 -> 2.13 #24817

Closed

rtyley added the Scala 2.13 See https://github.com/guardian/maintaining-scala-projects/issues/2 label Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scala 2.13: Fix various syntax errors #25425

Scala 2.13: Fix various syntax errors #25425

ioannakok commented Aug 23, 2022 •

edited

Loading

jlieb10 Aug 24, 2022

ioannakok Aug 24, 2022 •

edited

Loading

jlieb10 left a comment

mxdvl Aug 25, 2022

ioannakok Aug 25, 2022

prout-bot commented Aug 26, 2022

Scala 2.13: Fix various syntax errors #25425

Scala 2.13: Fix various syntax errors #25425

Conversation

ioannakok commented Aug 23, 2022 • edited Loading

What does this change?

jlieb10 Aug 24, 2022

Choose a reason for hiding this comment

ioannakok Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

Short version

Long version (feel free to skip! 🙂)

jlieb10 left a comment

Choose a reason for hiding this comment

mxdvl Aug 25, 2022

Choose a reason for hiding this comment

ioannakok Aug 25, 2022

Choose a reason for hiding this comment

prout-bot commented Aug 26, 2022

ioannakok commented Aug 23, 2022 •

edited

Loading

ioannakok Aug 24, 2022 •

edited

Loading