Support for zio layers - Githubissues

danilbykov commented 3 years ago

Using ZLayers from zio leads to unreadable compilation messages when environment misses some layer. This plugin can greatly improve error messages. RefinedFormatter already does most of work but small changes are needed.

RefinedFormatter collects traits only from top level while zlayers can have nested RefinedTypes. Here is example from my test

RefinedType(List(
  RefinedType(List(
    TypeRef(ThisType(zio), zio.Has, List(
      RefinedType(List(TypeRef(ThisType(scala), TypeName("AnyRef"), List()), TypeRef(ThisType(layers), layers.Service1, List())), Scope())
    )),
    TypeRef(ThisType(zio), zio.Has, List(
      RefinedType(List(TypeRef(ThisType(scala), TypeName("AnyRef"), List()), TypeRef(ThisType(layers), layers.Service2, List())), Scope())
    ))
  ), Scope()),
  TypeRef(ThisType(zio), zio.Has, List(
    RefinedType(List(TypeRef(ThisType(scala), TypeName("AnyRef"), List()), TypeRef(ThisType(layers), layers.Service3, List())), Scope())
  ))
), Scope())

It has several levels of RefinedTypes and services are on bottom level. This is why I added class DeepRefined.

There is another issue with this syntax tree which breaks error reporting. Compiler defines type of services as zio.Has[AnyRef with layers.Service1] which is equivalent to zio.Has[layers.Service1]. So I was needed to handle this case inside DeepRefined and remove AnyRef inside zio.Has.

What do you think about these changes? Can I add some new option for plugin which allows to replace default Refined by DeepRefined?

tek commented 3 years ago

looks good! what would be the problem with making this the default?

tribbloid commented 3 years ago

Is AnyRef with layers.Service1 the actual type signature in the source code?

I probably encountered a similar case before, if you have a small test case I should be able to help

danilbykov commented 3 years ago

what would be the problem with making this the default?

I'm afraid that my changes may lead to not equivalent types in common case. It must work for zlayers from zio but I'm not sure about common case. I don't know about other cases when compiler can generate such nested RefinedTypes. But if you are OK we can make this the default. :)

Is AnyRef with layers.Service1 the actual type signature in the source code? I probably encountered a similar case before, if you have a small test case I should be able to help

My test already includes this example. For example, if I specify type explicitly

  val service1: ULayer[Has[Service1]] = ZLayer.succeed(new Service1 {})
  val service2: ULayer[Has[Service2]] = ZLayer.succeed(new Service2 {})
  val service3: URLayer[Has[Service1], Has[Service3]] = ZLayer.fromService { (_: Service1) => new Service3 {} }
  val service4: ULayer[Has[Service4]] = ZLayer.succeed(new Service4 {})

then compiler generates following tree for service1 ++ service2 >+> service3

RefinedType(List(
  RefinedType(List(
    TypeRef(ThisType(zio), zio.Has, List(TypeRef(ThisType(ru.tinkoff.mvno.callerid.layers), ru.tinkoff.mvno.callerid.layers.Service1, List()))),
    TypeRef(ThisType(zio), zio.Has, List(TypeRef(ThisType(ru.tinkoff.mvno.callerid.layers), ru.tinkoff.mvno.callerid.layers.Service2, List())))
  ), Scope()),
  TypeRef(ThisType(zio), zio.Has, List(TypeRef(ThisType(ru.tinkoff.mvno.callerid.layers), ru.tinkoff.mvno.callerid.layers.Service3, List())))
), Scope())

Comparing this tree with tree from my initial comment we can see that depth of RefinedTypes is two and services don't have with AnyRef.

But it requires explicit types which we may try to avoid.

tek commented 3 years ago

well since you're checking for the presence of the zio type before the analysis I don't see what the harm is

danilbykov commented 3 years ago

Hmm, I was going to remove DeepRefined and move this functionality into Refined. But I noted that simple Refined is used inside RefinedFormatter.apply. Should I use DeepRefined in this method too?

tek commented 3 years ago

if the tests are green, ship it!

tek commented 3 years ago

@danilbykov Am I right to assume that you wanted to change that thing before merging?

danilbykov commented 3 years ago

Sorry, I thought that it means I should not change current version.

if the tests are green, ship it!

I removed DeepRefined and put all changes in Refined.

tek commented 3 years ago

Oh, I can see how that might have been ambiguous. sorry!

tek / splain

Support for zio layers #49