scalastack-overflowimplicit-conversion

Why a stack overflow from my Scala implicit conversion?


I have an issue with my code where I'm getting a stack overflow exception when doing an implicit conversion from Calendar to PublicationSchedule. I've cut down the code and can reproduce in Scastie (this is running with Scala 2.12.19):

import scala.language.implicitConversions
import java.time.{DayOfWeek, LocalDate}

object DayOfTheWeek extends Enumeration {
  type DayOfTheWeek = Value

  val Monday = Value(DayOfWeek.MONDAY.getValue())
  val Tuesday = Value(DayOfWeek.TUESDAY.getValue())
  val Wednesday = Value(DayOfWeek.WEDNESDAY.getValue())
  val Thursday = Value(DayOfWeek.THURSDAY.getValue())
  val Friday = Value(DayOfWeek.FRIDAY.getValue())
  val Saturday = Value(DayOfWeek.SATURDAY.getValue())
  val Sunday = Value(DayOfWeek.SUNDAY.getValue())

  val weekDays = List(Monday, Tuesday, Wednesday, Thursday, Friday)
  val weekEnd = List(Saturday, Sunday)
  val allDays =
    List(Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday)
}

import DayOfTheWeek._

object PublicationDateRule extends Enumeration {
  type PublicationDateRule = Value
  type PublicationDateRules = Map[DayOfTheWeek, PublicationDateRule]
  val Ignore = Value(1, "Ignore")
  val Next = Value(2, "Next")
  val Previous = Value(3, "Previous")
}

import PublicationDateRule._

object MyImplicits {
  implicit val localDateOrdering: Ordering[LocalDate] =
    Ordering.by(_.toEpochDay)
  implicit def calendarToPublicationSchedule(c: Calendar): PublicationSchedule =
    PublicationSchedule(
      c,
      c.days,
      Map(
        Monday -> Ignore,
        Tuesday -> Ignore,
        Wednesday -> Ignore,
        Thursday -> Ignore,
        Friday -> Ignore,
        Saturday -> Ignore,
        Sunday -> Ignore
      )
    )
}

import MyImplicits._

case class Calendar(
    identifier: String,
    nonWorkingDays: Seq[DayOfTheWeek],
    holidays: Seq[LocalDate]
) 

case class PublicationSchedule(
    calendar: Calendar,
    days: Seq[DayOfTheWeek],
    holidayRules: PublicationDateRules
)


val londonBankHolidays = Calendar(
  "London Bank Holidays",
  weekEnd,
  List(
    LocalDate.parse("2024-01-01"),
    LocalDate.parse("2024-03-29"),
    LocalDate.parse("2024-04-01"),
    LocalDate.parse("2024-05-05"),
    LocalDate.parse("2024-08-26"),
    LocalDate.parse("2024-12-25"),
    LocalDate.parse("2024-12-26")
  )
)

val ps: PublicationSchedule = londonBankHolidays

ps

I tried to cut down the example to be much simpler. I.e. have an Inner case class and an Outer case class (that takes Inner as a constructor parameter) and have an implicit conversion from Inner to Outer but this works just fine:

import scala.language.implicitConversions

object MyImplicits {
  implicit def innerToOuter(inner: Inner): Outer =
    Outer(inner, inner.number + 1)
}

import MyImplicits._

case class Inner(number: Int)

case class Outer(inner: Inner, number: Int)

val inner = Inner(42);

val outer: Outer = inner;

outer

If I debug my code then I can see from the stack trace that each call to the implicit conversion calendarToPublicationSchedule keeps on recursing, but for the life of me I can't work out why.

Any suggestions would be most welcome.

David


Solution

  • Because you have a loop there:

      implicit def calendarToPublicationSchedule(c: Calendar): PublicationSchedule =
        PublicationSchedule(
          /* calendar = */ c,
          /* days = */ c.days,
          /* holidayRules = */ Map(
            Monday -> Ignore,
            Tuesday -> Ignore,
            Wednesday -> Ignore,
            Thursday -> Ignore,
            Friday -> Ignore,
            Saturday -> Ignore,
            Sunday -> Ignore
          )
        )
    

    c: Calendar has no val days - you probably meant something based on c.nonWorkingDays - but you used days, which is absent in Calendar but present in PublicationSchedule. So you are ending up with:

      implicit def calendarToPublicationSchedule(c: Calendar): PublicationSchedule =
        PublicationSchedule(
          /* calendar = */ c,
          /* days = */ calendarToPublicationSchedule(c).days, // <- infinite recursion
          /* holidayRules = */ Map(
            Monday -> Ignore,
            Tuesday -> Ignore,
            Wednesday -> Ignore,
            Thursday -> Ignore,
            Friday -> Ignore,
            Saturday -> Ignore,
            Sunday -> Ignore
          )
        )
    

    It should highlight why implicit conversion is dangerous and advised against. I'd suggest creating something like

    
    // no implicit conversions in this scope!
    
    case class Calendar(
        identifier: String,
        nonWorkingDays: Seq[DayOfTheWeek],
        holidays: Seq[LocalDate]
    ) {
    
    
      def toPublicationSchedule: PublicationSchedule =
        PublicationSchedule(
          calendar = this,
          days = DayOfTheWeek.values.filterNot(nonWorkingDays.contains), // (?)
          holidayRules = Map(
            Monday -> Ignore,
            Tuesday -> Ignore,
            Wednesday -> Ignore,
            Thursday -> Ignore,
            Friday -> Ignore,
            Saturday -> Ignore,
            Sunday -> Ignore
          )
        )
    } 
    
    case class PublicationSchedule(
        calendar: Calendar,
        days: Seq[DayOfTheWeek],
        holidayRules: PublicationDateRules
    )
    

    If you need to have this implicit conversion (calling .toPublicationSchedule explicitly is not that much of a deal) - define it to call the explicit conversion which doesn't import any implicits, so that there would be no risk of an accidental recursion.