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

Symbol.newClass does not support class parameters #21739

Open
bishabosha opened this issue Oct 9, 2024 · 4 comments · May be fixed by #21880
Open

Symbol.newClass does not support class parameters #21739

bishabosha opened this issue Oct 9, 2024 · 4 comments · May be fixed by #21880
Assignees
Labels
area:metaprogramming:reflection Issues related to the quotes reflection API itype:enhancement

Comments

@bishabosha
Copy link
Member

bishabosha commented Oct 9, 2024

Compiler version

3.5.1

Minimized example

Mill's Cross Modules are implemented with a macro that generates a class, and a factory function that creates a new instance of the class given some context.

// mill definitions (simplified)
package mill

trait Module(using Context) 

object Cross:
  trait Module[String] extends mill.Module:
    def crossValue: String
// user code
trait MyCrossModule extends Cross.Module[String]
object myCross extends Cross[MyCrossModule]("foo", "bar")
// simplified generated code as part of implicit conversion from `"foo"` to `Factory[MyCrossModule]`

factoryArgs.map[MyCrossModule]({ (arg: "foo") =>
  class C(ctx: Context) extends MyCrossModule with mill.Module(using ctx) {
    def crossValue: String = arg
  }
  (classOf[C], ctx => new C(ctx))
})

The new class has to be generated via Macro, not quotes and splices, because the Specific cross module trait is not known statically.

The problem is that there is no way via Symbol.newClass to add the necessary (ctx: Context) parameter to the generated class C

Expectation

Allow to customise the parameters of the class, even if its just single term parameter list, I think generic is less needed because the macro can specialise the types.

@bishabosha bishabosha added stat:needs triage Every issue needs to have an "area" and "itype" label itype:enhancement area:metaprogramming:reflection Issues related to the quotes reflection API and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Oct 9, 2024
@jchyb jchyb self-assigned this Oct 17, 2024
@goshacodes
Copy link

goshacodes commented Jan 16, 2025

Not sure whether I should create a new issue or ask here. I think this issue is closely related to mine.

I have an issue ScalaMock/ScalaMock#191 created long time ago.

And to fix it I need possibility to annotate a class created with Symbol.newClass. I didn't find a way to do it currently.
Can this also be added?

@jchyb
Copy link
Contributor

jchyb commented Feb 4, 2025

@goshacodes I've now added the ability to add annotations to classes in #21880. I will probably end up changing the title/explanations of the PR before review from the compiler team members, but in terms of the code/functionality itself - I consider it done, and if nothing bad comes up, and after merging with @experimental for 3.7 I would like to stabilize it in 3.8/3.9. As a power user of newClass, It would be helpful to me if you could look at the added functionality/api/documentation there (only if you are available, of course), to see if something major is missing, before the main review.

@goshacodes
Copy link

Looks like everything is covered, has nothing special in mind currently, thank you!

@goshacodes I've now added the ability to add annotations to classes in #21880. I will probably end up changing the title/explanations of the PR before review from the compiler team members, but in terms of the code/functionality itself - I consider it done, and if nothing bad comes up, and after merging with @experimental for 3.7 I would like to stabilize it in 3.8/3.9. As a power user of newClass, It would be helpful to me if you could look at the added functionality/api/documentation there (only if you are available, of course), to see if something major is missing, before the main review.

@goshacodes
Copy link

goshacodes commented Feb 7, 2025

Found one test case recently, which looks strange:

  class MyClass
  class MyTycon[T]
  class WithGeneric[T, B[_]](b: B[T], t: T)

mock[WithGeneric[MyClass, Option]] // not compiles
mock[WithGeneric[MyClass, MyTycon]] // not compiles

Error is:

[error] 57 |      mock[WithGeneric[MyClass, Option]]
[error]    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |      Found:    Option[MyClass]
[error]    |      Required: Option[MyClass]
[error]    |

----
[error]    | Explanation (enabled by `-explain`)
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |
[error]    | Tree: org.scalamock.util.Defaultable.default[Option[Int]].default.asInstanceOf[
[error]    |   Option[Int]]
[error]    | I tried to show that
[error]    |   Option[Int]
[error]    | conforms to
[error]    |   Option[Int]
[error]    | but none of the attempts shown below succeeded:
[error]    |
[error]    |   ==> Option[Int]  <:  Option[Int]
[error]    |     ==> Any  <:  Option[Int]  = false
[error]    |
[error]    | The tests were made under a constraint with:
[error]    |  uninstantiated variables: X0
[error]    |  constrained types: [X0]: X0
[error]    |  bounds:
[error]    |      X0
[error]    |  ordering:
[error]    |  co-deps:
[error]    |  contra-deps:

Now I'm replacing all generic types with applied types by hand and casting nulls to that type - will be nice to check if this can be done now with Symbol.newClass.

This is exactly what I do, maybe I'm just doing something wrong:
https://github.com/ScalaMock/ScalaMock/pull/585/files

UPDATE: found correct solution

@goshacodes I've now added the ability to add annotations to classes in #21880. I will probably end up changing the title/explanations of the PR before review from the compiler team members, but in terms of the code/functionality itself - I consider it done, and if nothing bad comes up, and after merging with @experimental for 3.7 I would like to stabilize it in 3.8/3.9. As a power user of newClass, It would be helpful to me if you could look at the added functionality/api/documentation there (only if you are available, of course), to see if something major is missing, before the main review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metaprogramming:reflection Issues related to the quotes reflection API itype:enhancement
Projects
None yet
3 participants