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

Use Nothing for no Result #52

Merged
merged 8 commits into from
Dec 13, 2021
Merged

Conversation

armanbilge
Copy link
Member

Based on #51.

@bpholt Before cherry-picking 00eb18f I wanted to try and write a test in 06c9eee that demonstrates the compiler issue with Nothing. I tried replicating your example from #45 (comment) but didn't have much luck. When you have chance would you mind poking at this? Thanks :)

@armanbilge armanbilge marked this pull request as draft November 19, 2021 05:44
@armanbilge armanbilge mentioned this pull request Nov 19, 2021
@bpholt
Copy link
Member

bpholt commented Nov 19, 2021

This is the best minimization I've found so far:

class LambdaSuite {
  def lambda[F[_]]: Resource[F, Lambda[F, Unit, Nothing]] = ???

  def middleware[F[_], Event, Result, Env](lambda: Resource[F, Lambda[Kleisli[F, Env, *], Event, Result]]): Resource[F, Lambda[F, Event, Result]] = ???

  implicit val nothing: Encoder[Nothing] = _ => ???

  class NothingLambda extends IOLambda[Unit, Nothing] {
    def handler =
      middleware(lambda[Kleisli[IO, Unit, *]].mapK(Kleisli.applyK(())))
  }
}
[error] …/feral/lambda/shared/src/test/scala/feral/lambda/LambdaSuite.scala:35:14: type mismatch;
[error]  found   : cats.effect.kernel.Resource[[+A]cats.effect.IO[A],feral.lambda.Lambda[[γ$1$]cats.data.Kleisli[cats.effect.IO,Unit,γ$1$],Unit,Nothing]]
[error]     (which expands to)  cats.effect.kernel.Resource[[+A]cats.effect.IO[A],(Unit, feral.lambda.Context[[γ$1$]cats.data.Kleisli[cats.effect.IO,Unit,γ$1$]]) => cats.data.Kleisli[cats.effect.IO,Unit,Option[Nothing]]]
[error]  required: cats.effect.kernel.Resource[[+A]cats.effect.IO[A],feral.lambda.Lambda[[γ$0$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$0$],Unit,Result]]
[error]     (which expands to)  cats.effect.kernel.Resource[[+A]cats.effect.IO[A],(Unit, feral.lambda.Context[[γ$0$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$0$]]) => cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,Option[Result]]]
[error]         .mapK(Kleisli.applyK(())))
[error]              ^
[error] one error found

It doesn't matter if the value passed to middleware is a Resource in F[_] or Kleisli[F, E, *]. If it's the latter, then the mapK in the NothingLambda goes away, but AFAICT it doesn't change the inference failure:

[error] …/feral/lambda/shared/src/test/scala/feral/lambda/LambdaSuite.scala:33:24: type mismatch;
[error]  found   : cats.effect.kernel.Resource[[γ$2$]cats.data.Kleisli[cats.effect.IO,Unit,γ$2$],feral.lambda.Lambda[[γ$2$]cats.data.Kleisli[cats.effect.IO,Unit,γ$2$],Unit,Nothing]]
[error]     (which expands to)  cats.effect.kernel.Resource[[γ$2$]cats.data.Kleisli[cats.effect.IO,Unit,γ$2$],(Unit, feral.lambda.Context[[γ$2$]cats.data.Kleisli[cats.effect.IO,Unit,γ$2$]]) => cats.data.Kleisli[cats.effect.IO,Unit,Option[Nothing]]]
[error]  required: cats.effect.kernel.Resource[[γ$1$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$1$],feral.lambda.Lambda[[γ$0$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$0$],Unit,Result]]
[error]     (which expands to)  cats.effect.kernel.Resource[[γ$1$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$1$],(Unit, feral.lambda.Context[[γ$0$]cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,γ$0$]]) => cats.data.Kleisli[[+A]cats.effect.IO[A],Unit,Option[Result]]]
[error]       middleware(lambda[Kleisli[IO, Unit, *]])
[error]                        ^
[error] one error found

@armanbilge
Copy link
Member Author

@bpholt thanks! That's an excellent minimization, exactly what I was looking for :)

Unfortunately, placing the INothing encoder in the companion object for IOLambda doesn't seem to bring it in scope for whatever reason. I'm going to futz with this some more.

@armanbilge
Copy link
Member Author

Darn, 2.12 seems to be lacking. Dare I say ... we drop it? 😆

@bpholt
Copy link
Member

bpholt commented Nov 19, 2021

Dropping 2.12 seems reasonable to me. I don't think the common reasons why people are stuck on 2.12 would apply in this context.

@armanbilge
Copy link
Member Author

Yeah, apparently we are "future-forward" or something 😆

If it becomes an issue we can probably bring back 2.12 very easily anyway. Those users would just need to explicitly import this implicit and we'd need to make this test 2.13/3.x-only. FTR if sbt supported this config out-of-the-box that's exactly what I would have done.

@armanbilge
Copy link
Member Author

armanbilge commented Nov 19, 2021

Ummm ... Scala 3 is broken too? Seems like a compiler bug 🤔

[error] -- Error: /workspace/feral/lambda/shared/src/test/scala/feral/lambda/LambdaSuite.scala:35:54 
[error] 35 |  class NothingLambda extends IOLambda[Unit, INothing] {
[error]    |                                                      ^
[error]    |ambiguous implicit arguments: both feral.lambda.nothingEncoder and feral.lambda.nothingEncoder match type io.circe.Encoder[feral.lambda.INothing] of parameter encoder of constructor IOLambda in class IOLambda

Update: tracking in scala/scala3#13981.
Update 2: it was a combination of weird incremental compiling issues, and the fact that implicit scopes have changed in Scala 3. So, this needed difference sources as I suspected.

@kubukoz
Copy link
Member

kubukoz commented Dec 9, 2021

tl;dr but I like the idea of Nothing in the result :D

@armanbilge
Copy link
Member Author

@kubukoz you're not stuck on Scala 2.12 are you? The tl;dr here is mostly about implicit scope for Nothing not working on 2.12.

@kubukoz
Copy link
Member

kubukoz commented Dec 9, 2021

I'm on 2.13, Nothing was giving me a missing Decoder instance error.

@armanbilge
Copy link
Member Author

Try INothing.

@kubukoz
Copy link
Member

kubukoz commented Dec 9, 2021

man, I can only have one experimental branch in my dependencies at a time 😂

* userland code returns `Some`.
*/
@nowarn("msg=dead code following this construct")
implicit val nothingEncoder: Encoder[INothing] = (_: INothing) => ???
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not 100% sure but can't this just return its argument? (a: INothing) => a. INothing <: Nothing and Nothing <: Json.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, this does work :)

@armanbilge
Copy link
Member Author

Actually you should have this in your branch, it's split off :)

@kubukoz
Copy link
Member

kubukoz commented Dec 9, 2021

we're reaching levels of stable that shouldn't be possible!

@armanbilge armanbilge mentioned this pull request Dec 9, 2021
6 tasks
@armanbilge armanbilge marked this pull request as ready for review December 13, 2021 02:39
def middleware[F[_], Event, Result](lambda: Lambda[Kleisli[F, Unit, *], Event, Result])
: Resource[F, Lambda[F, Event, Result]] = ???

class NothingLambda extends IOLambda[Unit, INothing] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I'm not mistaken this should even work with just Nothing, at least on 2.13

@kubukoz
Copy link
Member

kubukoz commented Dec 13, 2021

This seems mergeable as well...

@armanbilge
Copy link
Member Author

Yep, I'm just dealing with the conflicts rn :)

@armanbilge armanbilge merged commit 7b494d3 into typelevel:main Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants