Added apply methods to import selectors in Quotes #21225

[ghik]

Fixes #21225

[bishabosha]

Suggested change

	@experimental def apply(bound: Option[TypeTree]): GivenSelector
	@experimental def apply(bound: Option[TypeRepr]): GivenSelector

I would suggest internally wrapping the type in TypeTree with the default span

[ghik]

That would be inconsistent with unapply and the .bound accessor method. Is that a problem?

[jchyb]

TypeTree makes more sense to me too

[jchyb]

Thank you for submitting! The PR looks good, and in my opinion the use case is valid. I would only suggest adding a test, at least for GivenSelector and OmitSelector - I don't really know how we could test others. Here's what I came up with, based on the snippets submitted in #21225:
In tests/run-macros/i21225/Macro_1.scala:

//> using options -experimental

import scala.quoted.*

trait Codec[-T] { def print(): Unit }
object Codec {
  inline def derivedWithDeps[T](deps: Any): Codec[T] = ${derivedWithDepsImpl[T]('deps)}

  private def derivedWithDepsImpl[T](deps: Expr[Any])(using q: Quotes)(using Type[T]): Expr[Codec[T]] = {
    import q.reflect.*

    val givenSelector: Selector = GivenSelector(None)
    val theImport = Import(deps.asTerm, List(givenSelector))
    Block(List(theImport), '{scala.compiletime.summonInline[Codec[T]]}.asTerm).asExprOf[Codec[T]]
    /* import deps.given
     * summonInline[Codec[T]]
     */
  }

  inline def derivedWithDepsWithNamedOmitted[T](deps: Any): Codec[T] = ${derivedWithDepsWithNamedOmittedImpl[T]('deps)}

  private def derivedWithDepsWithNamedOmittedImpl[T](deps: Expr[Any])(using q: Quotes)(using Type[T]): Expr[Codec[T]] = {
    import q.reflect.*

    val givenSelector: Selector = GivenSelector(None)
    val omitSelector: Selector = OmitSelector("named")
    val theImport = Import(deps.asTerm, List(givenSelector, omitSelector))
    Block(List(theImport), '{scala.compiletime.summonInline[Codec[T]]}.asTerm).asExprOf[Codec[T]]
    /* import deps.{given, named => _}
     * summonInline[Codec[T]]
     */
  }
}

In tests/run-macros/i21225/Test_2.scala :

//> using options -experimental

import scala.quoted.*

sealed trait Foo
case class Bar() extends Foo
object CustomCodecs {
  given named: Codec[Bar] = new Codec[Bar]{ def print(): Unit = println("bar codec")}
  given Codec[Foo] = new Codec[Foo]{ def print(): Unit = println("foo codec") }
}

@main def Test =
  Codec.derivedWithDeps[Bar](CustomCodecs).print()
  Codec.derivedWithDepsWithNamedOmitted[Bar](CustomCodecs).print()

In tests/run-macros/i21225.check:

bar codec
foo codec

Sorry about the long wait, let me know if you have the time to add those, alternatively I can take this over.

[ghik]

@jchyb added tests as you suggested

[jchyb]

LGTM! Thank you!

[bishabosha]

@jchyb i would like to ask about this because it seems the purpose is to allow new implicits to become available on demand at the site where the macro is spliced?

here is a possible dangerous example from mill 0.12 for scala 2

c.Expr[Discover](
 q"""import mill.main.TokenReaders._;
   _root_.mill.define.Discover.apply2(
     _root_.scala.collection.immutable.Map(..$mapping)
   )"""
)

this code was defined in the main.define module, where TokenReaders is not available statically, because it is defined in a downstream module main.

But because macro evaluation was delayed until a call site where both modules are available on the classpath it works.

I think this is potentially dangerous to allow in scala 3?

Perhaps it always works because you already need to have access to the prefix's symbol to even construct it? do we know that for sure

[jchyb]

They only should be available in the scope of macro expanded code (so they should not leak outside of that).
The symbol has to exists to be able to be added to the import selector - although you can get one via a string like in Symbol.classSymbol("..."), potentially receiving Symbol.noSymbol if that symbol does not exist during macro expansion. So I guess the mill case is possible, but I am not sure if it's dangerous here - we do not have to reintroduce the concept of "hygiene" from scala-2, as we have use a Symbol instead of scala-2's TermName anyway and it's on the metaprogrammer to ensure it exists (what I'm trying to say here is, there should be no weird errors here unlike in scala-2). Bear in mind it was already possible to reference Symbols which might not exist until the macro call site when constructing other types of trees with the reflect API

Sorry, just noticed we do use Strings here instead of Symbols. I need to revisit this. Thank you for bringing this up @bishabosha