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.

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

Ugh implicit value classes #154

Closed
travisbrown opened this issue Dec 24, 2015 · 8 comments
Closed

Ugh implicit value classes #154

travisbrown opened this issue Dec 24, 2015 · 8 comments
Assignees

Comments

@travisbrown
Copy link
Member

If we've got the following set-up:

import io.circe._, io.circe.optics.JsonPath.root, io.circe.syntax._

val json = Map("a" -> Map("b" -> Map("c" -> 1))).asJson
val path = root.a.b.c.as[Int]

And then we try to use the Optional, we'll get this:

scala> path.getOption(json)
res0: Option[Int] = None

scala> path.modify(_ + 1)(json)
res1: io.circe.Json =
{
  "a" : {
    "b" : {
      "c" : 1
    }
  }
}

But…

scala> root.x.y.z.as[Int].getOption(Map("x" -> Map("y" -> Map("z" -> 1))).asJson)
res2: Option[Int] = Some(1)

This made absolutely no sense to me for about ten minutes this morning, until I remembered that the io.circe.syntax.EncoderOps class looks like this:

implicit class EncoderOps[A](val a: A) extends AnyVal {
  def asJson(implicit e: Encoder[A]): Json = e(a)
}

Which means that anything with an Encoder instance gets a a method that's just the identity. For some reason I never realized that implicit value classes have this horrible property. This happens in the standard library, too. For example, in a fresh REPL:

scala> 13.self
res0: Int = 13

…thanks to RichInt. I hate implicits.

At the moment I can imagine three solutions:

  1. Make EncoderOps not a value class.
  2. Give the value member a more obfuscated name that isn't likely to result in collisions.
  3. Introduce a dummy class with a method name that intentionally collides.

I'm leaning toward the first option.

@xuwei-k
Copy link
Contributor

xuwei-k commented Dec 24, 2015

or drop Scala 2.10 support?

@travisbrown
Copy link
Member Author

Thanks, @xuwei-k. There are at least a couple of other places where we have workarounds for 2.10—maybe it's time for version-specific source trees.

@fthomas
Copy link
Contributor

fthomas commented Dec 24, 2015

Note that Scala.js currently does not look into shared/src/main/scala-$VERSION directories (see scala-js/scala-js#2005) so you would have to duplicate some files across js and jvm directories.

@chwthewke
Copy link

Sorry for kind of barging in (saw @travisbrown 's tweet), but wouldn't making the field private (class FooOps(private val a: A) extends AnyVal etc.) be a viable fourth solution? It does seem to properly hide the a method.

@travisbrown
Copy link
Member Author

@chwthewke That works on 2.11, but not 2.10, unfortunately.

@chwthewke
Copy link

@travisbrown Gotcha, thanks for the explanation

@propensive
Copy link
Contributor

I don't think option 2 is a bad choice. It would be consistent with the common approach of giving implicit values long, descriptive names to avoid likely clashes.

@ghost
Copy link

ghost commented Dec 25, 2015

Note that Scala.js currently does not look into shared/src/main/scala-$VERSION dir....

Or copy https://github.com/InTheNow/sbt-catalysts/blob/master/src/main/scala/sbtcatalysts/CatalystsPlugin.scala#L156-L172

(Or... just use sbt-catalysts)

@travisbrown travisbrown self-assigned this Feb 13, 2016
julienrf pushed a commit to scalacenter/circe that referenced this issue May 13, 2021
Remove old versions, housekeeping, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants