propensive / contextual

Statically-checked string interpolation in Scala
https://soundness.dev/contextual/
251 stars 23 forks source link

Embedding custom types not working with version 2.0.0 #62

Closed balhoff closed 3 years ago

balhoff commented 3 years ago

Thank you for contextual, it has served me well! I would like to make my interpolator available for Scala 2.13, but contextual 2.0.0 isn't working for me. I have a string interpolator for SPARQL queries, which embeds various types into SPARQL query strings with appropriate formatting. I upgraded to 2.0.0, and now I can't get any of my tests for interpolating various types to compile (https://github.com/phenoscape/sparql-utils/pull/26).

There is a strong possibility I'm doing something wrong, but it has been working well for me with the previous version. I thought this might be related to #61.

Basically the only thing I changed in the upgrade is this line: https://github.com/phenoscape/sparql-utils/blob/7a58aec65caf1d296a01a2ba097b7d88411abdaa/modules/core/src/main/scala/org/phenoscape/sparql/SPARQLInterpolator.scala#L125

propensive commented 3 years ago

Thanks for trying it out so quickly, @balhoff! I don't think it's unique to you, at all! I opened issue #61 because two (IIRC) of the examples stopped compiling, but I was happy to get the majority of cases working at the time.

I just had a look at your errors from CI, and it makes me wonder whether type inference in Scala 2.13 has changed sufficiently that it's failing to understand the type of the embedder as I originally intended.

Are you able to compile your code with Contextual 2.0.0 on Scala 2.12.x? It would be interesting to see whether that behaves the same (which would imply that my changes are responsible for the breakage) or works (which implies something fundamentally different about 2.13).

balhoff commented 3 years ago

Thanks for taking a look!

Are you able to compile your code with Contextual 2.0.0 on Scala 2.12.x?

No, 2.12 doesn't work either. I'm testing on both—here's 2.12.12: https://travis-ci.org/github/phenoscape/sparql-utils/jobs/743601895

The errors seem the same.

beikern commented 3 years ago

Seems we are having problems with embedding post upgrade to 2.0.0 (for the sweet sweet scala 2.13):

The compilation fails using scala 2.12.

[info] compiling 1 Scala source to /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/target/scala-2.12/test-classes ...
Literals: 'List( foo = "bar" )'
Literals: 'List( foo = "bar" )'
Literals: 'List( foo = "3" )'
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:12:7: not found: value <none>
[error]       lightql""" foo = "bar" """ shouldEqual Query(Clause("foo", EqualityOperator.`=`, "bar"))
[error]       ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:26:7: exception during macro expansion: 
[error] scala.MatchError: (v1,0) (of class scala.Tuple2)
[error]     at contextual.Macros$.$anonfun$contextual$2(macros.scala:70)
[error]     at scala.collection.immutable.List.map(List.scala:293)
[error]     at contextual.Macros$.contextual(macros.scala:70)
[error]       lightql""" foo = "$v1" """ shouldEqual Query(Clause("foo", `=`, "bar"))
[error]       ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:27:30: type mismatch;
[error]  found   : Int
[error]  required: String
[error]       lightql""" foo = "${v2 * 2}" """ shouldEqual Query(Clause("foo", `=`, "4"))
[error]                              ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:28:26: type mismatch;
[error]  found   : V3
[error]  required: String
[error]       lightql""" foo = "$v3" """ shouldEqual Query(Clause("foo", `=`, "test"))
[error]                          ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:32:42: dead code following this construct
[warn]       "lightql\"\"\" foo = \"3\" \"\"\"" should compile
[warn]                                          ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:33:59: dead code following this construct
[warn]       "val v = \"bar\";lightql\"\"\" foo = \"$v\" \"\"\"" should compile
[warn]                                                           ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:24:11: local val v3 in value <local LightqlInterpolatorSpec> is never used
[warn]       val v3 = V3("test")
[warn]           ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:18:21: parameter value v in class V3 is never used
[warn]       case class V3(v: String) {
[warn]                     ^
[warn] four warnings found
[error] four errors found
[error] stack trace is suppressed; run last Test / compileIncremental for the full output
[error] (Test / compileIncremental) Compilation failed
[error] Total time: 0 s, completed 18-nov-2020 21:06:05

scala 2.13 also fails with the following errors:

[info] compiling 1 Scala source to /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/target/scala-2.13/test-classes ...
Literals: 'List( foo = "bar" )'
Literals: 'List( foo = "bar" )'
Literals: 'List( foo = "3" )'
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:12:7: not found: value <none>
[error]       lightql""" foo = "bar" """ shouldEqual Query(Clause("foo", EqualityOperator.`=`, "bar"))
[error]       ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:26:7: exception during macro expansion: 
[error] scala.MatchError: (v1,0) (of class scala.Tuple2)
[error]     at contextual.Macros$.$anonfun$contextual$2(macros.scala:70)
[error]     at scala.collection.immutable.List.map(List.scala:246)
[error]     at scala.collection.immutable.List.map(List.scala:79)
[error]     at contextual.Macros$.contextual(macros.scala:70)
[error]       lightql""" foo = "$v1" """ shouldEqual Query(Clause("foo", `=`, "bar"))
[error]       ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:27:30: type mismatch;
[error]  found   : Int
[error]  required: String
[error]       lightql""" foo = "${v2 * 2}" """ shouldEqual Query(Clause("foo", `=`, "4"))
[error]                              ^
[error] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:28:26: type mismatch;
[error]  found   : V3
[error]  required: String
[error]       lightql""" foo = "$v3" """ shouldEqual Query(Clause("foo", `=`, "test"))
[error]                          ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:32:42: dead code following this construct
[warn]       "lightql\"\"\" foo = \"3\" \"\"\"" should compile
[warn]                                          ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:33:59: dead code following this construct
[warn]       "val v = \"bar\";lightql\"\"\" foo = \"$v\" \"\"\"" should compile
[warn]                                                           ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:24:11: local val v3 in value <local LightqlInterpolatorSpec> is never used
[warn]       val v3 = V3("test")
[warn]           ^
[warn] /home/beikern/desarrollo/tecsisa/arq/kommodo-scala-bundle/wr-filters/wr-filters-lightql-interpolator/src/test/scala/com/tecsisa/wr/filters/lightql/interpolator/LightqlInterpolatorSpec.scala:18:21: parameter value v in class V3 is never used
[warn]       case class V3(v: String) {
[warn]                     ^
[warn] four warnings found
[error] four errors found
[error] stack trace is suppressed; run last Test / compileIncremental for the full output
[error] (Test / compileIncremental) Compilation failed
[error] Total time: 2 s, completed 18-nov-2020 21:10:1

This is our Embedder:

    /*
     * Embedder definition specifies how types are converted to the required
     * Input type (String). This specific case invokes 'toString' method for
     * any object representing a Hole in the interpolated string.
     */
    implicit def embedAll[T <: Any]: Embedder[
      (LightqlInterpolator.DefaultContext.type, LightqlInterpolator.DefaultContext.type),
      T,
      String,
      interpolator.LightqlInterpolator.type
    ] = LightqlInterpolator.embed[T](Case(DefaultContext, DefaultContext)(_.toString))
RustedBones commented 3 years ago

I am facing the same issue.

In my case the StaticInterpolation.interpolatorTerm resolved to NoSymbol here

I could hack and make it work by overriding the evaluator method and returning the Quasiquote hardcoding my Interpolator: commit.

I also ran some tests with the following code

  def testMacro[I <: Interpolator: c.WeakTypeTag]
  (c: whitebox.Context)(expressions: c.Tree*): c.Tree = {
    val _ = expressions
    println(implicitly[c.WeakTypeTag[I]].tpe)
    println(implicitly[c.WeakTypeTag[I]].tpe.getClass)
    println(implicitly[c.WeakTypeTag[I]].tpe.typeSymbol)
    println(implicitly[c.WeakTypeTag[I]].tpe.termSymbol)
    ???
  }

  implicit class TestStringContext(sc: StringContext) {
    def iri(expressions: String*): Iri = macro testMacro[TestInterpolator.type] // instead of macro Macros.contextual[TestInterpolator.type]
  }

We have a ModuleTypeRef but no idea why the termSymbol returns a NoSymbol .

Changing the StaticInterpolation.interpolatorTerm to get the typeSymbol works in my case. Hope this can help

propensive commented 3 years ago

Thanks for the feedback on this, everyone. I spent some time a couple of weeks ago looking at the problem, and I was seeing exactly the same behavior as @RustedBones. I persevered further with trying to force the compiler to "see" the type that's coming out as NoSymbol, but my best (probably slightly naïve) guess is that that symbol is simply not available at the time the macro runs because calculating it has some dependency on the macro's evaluation.

We ended up here because I couldn't get the old Prefix style of defining an interpolator to work in Scala 2.13, but I didn't try that hard. Prefix also had two types inferred (rather than the one we have at the moment), and although I have no recollection of the original design decision that led to that (the second type parameter should be a function of the first, in theory), I thought it would be possible to get away with just one. It is possible in many cases, in fact, but it's common enough that it's not to be a problem.

So my approach now is going to be to try to go back to fixing the Prefix style, based off the previous working version.

(In unrelated news, I've made some progress with a Scala 3 version of Contextual, but that's going to have a different design completely.) ;)

balhoff commented 3 years ago

Thanks @propensive, updating to 3.0.0 seems to work for me!

beikern commented 3 years ago

Works like a charm. Hooray!