Kotlin / KEEP

Kotlin Evolution and Enhancement Process
Apache License 2.0
3.29k stars 357 forks source link

Context parameters #367

Open serras opened 4 months ago

serras commented 4 months ago

This is an issue to discuss context parameters. The full text of the proposal can be found here.

JakeWharton commented 4 months ago

The use of summon as an API name seems weird to me. The name to me implies some work is occurring, but it's really only resolving an unnamed local, right?

First thing that comes to mind for me would be like a materialize, although it's not perfect. Maybe identify?

jamesward commented 4 months ago

@JakeWharton Not that is probably matters but summon is what Scala 3 Type Class summoning uses.

Jellymath commented 4 months ago

With the introduction of named context parameters, it feels like summon function can be named something like contextParam or just dropped entirely

serras commented 4 months ago

@JakeWharton Not that is probably matters but summon is what Scala 3 Type Class summoning uses.

Indeed. Also it's uncommon enough to not be taken by any other framework in the ecosystem (contextParams reads like a web framework if you squint your eyes, materialize like a reactive or event-stream operator...)

alllex commented 4 months ago

I can suggest receiver<A>() as an alternative for summon. It highlights the purpose of the function, which is to resolve unnamed receivers. The function will almost never be used for the named context parameters because, well, they can be directly addressed by name.

Another alternative could be to overload the new context() function. All versions that take arguments would be used to introduce context, as in context(foo) { ... }. While the parameterless overload would be used to extract the context as in val t = context<A>(). It plays well in a sense, because the function does not take any lambda argument, and therefore the only thing it can produce/return is the context parameter itself.

zhelenskiy commented 4 months ago

I read the document and have several points to mention:

  1. As mentioned by previous authors, summon seems weird to me; it would be much more obvious to call it context as you get context parameter. Maybe it would be better even to make it somehow a property as it has property semantics. However, this would be different from the regular properties which cannot have type arguments.
  2. Redness of type parameters usage. Code is usually written from left to right, so when you write context(A<B>) fun <B> ..., you first type context(A<B>) which is red because <B> is unresolved. There are two proper solutions as I see:
    • Do not mark <B> as red in the IDE if the function is not fully defined, but mark it as gray instead. It can be even used by an IDE to suggest name and bounds for the type parameters when they are typed.
    • Support defining type parameters on context keyword as well, making them visible in both context part and the rest. . But this would lead to multiple ways to do the only thing which is not great. However, we already have where which leads to multiple ways to define type parameter bounds.
  3. context keyword is used both to specify required contexts and to specify that this function itself must be called when the receiver is contextual. This is unobvious then if the class A is required to be passed as a context receiver or not in the following snippet. And have to do the second option then?
    class A {
        context(Int) fun f() = Unit
    }
    fun main() {
        context(3, A()) {
            f() // is it ok?
            // if not, then how to make it ok? `context context(Int) fun f()`?
            // if it is, how to escape such an exposure
        }
    }
  4. As for a DI, it might be a good idea to try to mimic the existing DI functionality and find if the current design is ok for the task. DIs have many things such as joining several configurations, extending ones, etc.; so it is worth trying to express them with the context receivers design.
gpopides commented 4 months ago

Based on the example as far as i understand, summon will give you access to a resource that normally you dont have access to. Sounds like borrowing to me

borrow<Logger>().thing() which to me reads like you borrow the identity of Logger for an operation and then go back to normal.

One more note is about the DI. I can't understand how would this help with DI. DI is more like "here are the tools you need to do your job" while context parameters read (to me) like "i have the tools, you can borrow them to do what you need to "

Nevertheless, i like the progress.

JakeWharton commented 4 months ago

You aren't borrowing them from anywhere. You already own the references. They were explicitly supplied to the function, just contextually rather than nominally (like a normal parameter). A borrow also deeply implies a lifetime to the reference which doesn't really apply (at least no more than one would think of a named parameter as a borrowed reference).

xxfast commented 4 months ago

Some of the DI use cases are already doable with just primary receiver without using any context parameter/receivers,

For example,

class ApplicationScope(
  private val store: Store,
  private val httpClient: HttpClient
)

class Logger {
  fun ApplicationScope.log(message: String) {
    this@ApplicationScope.store.add(message)
  }
}

Here the scope can be provided with with

with(application){
  logger.log(message)
}

logger.log(message) // will fail to compile 

I guess context parameters makes this ApplicationScope anonymous, personally I prefer it being more explicit

bitspittle commented 4 months ago

Has there been any discussion of simply disallowing unnamed context parameters? Unless I'm mistaken, this would eliminate...

  1. The awkward summon function
  2. The need for a new empty context visibility modifier.

I'm guessing everyone commenting on this issue is going to be an experienced Kotlin user -- who else is reading a technically complex KEEP and then feeling like they can express their opinion about it -- but I am trying to think about this feature from someone who is brand new to Kotlin.

Attempting to wear the newbie hat, I think if I ran across the following code, I'd find it very unsightly and confusing:

interface Logger {
    context fun log(message: String) 
    fun thing()
}

To explain this code to that new user, you'd have to explain a lot to them at once -- what context parameters are, why some are unnamed, and how method visibility / propagation works for these unnamed context parameters (which is unique and different from the rest of the language). "Well you see, yeah, here "thing" is kind of public, but then "log" is really public. I mean, for "thing", you need to summon it..."

The implication of this small code snippet worries me too:

value class ApplicationContext(
  val app: Application
) {
  context fun get() = app.get()
  // copy of every function, but with context on front
}

How much boilerplate are codebases going to need to introduce if they want to support unnamed context parameters?


Maybe someone can clue me into a use-case where unnamed context parameters are really important. Are they really worth propagating so much noise through the rest of the language for them? Will this be a lot of burden on library developers, who now have to think about every class they write in the context of being used as an unnamed context parameter?

Or maybe I'm being totally obtuse, and you'd still need to support this syntax even if every context parameter were named?

JavierSegoviaCordoba commented 4 months ago

Even though I see the value and flexibility in the contextual visibility section, its complexity is really over the rest of the features of the language for me.

With a little knowledge of programming (basic level) and English, you can understand Kotlin code more or less easily, but the contextual visibility will drop this "readability" a lot.

I prefer the simpler approach it had before in which if you are in a specific context, you have access to all of its public properties/functions, not the only ones marked as context. If this feature grows in popularity, I am afraid almost everyone will just add context to all public declarations.

serras commented 4 months ago

Some answers and clarifications:

serras commented 4 months ago

Maybe this was not entirely clear from the document, but the contextual visibility only affects to context receivers, not named context parameters, which work like a regular parameter in terms of accessibility. We think that the default mode of use of this feature should actually be named context parameters, which are must simpler to explain, pose no problem for visibility, and do not require using summon.

It's even debatable whether Logger is a good example of context receiver, and shouldn't just be a named context parameter. In that case, the regular declaration suffices, no need for context.

interface Logger {
    fun log(message: String) 
}

// how to use it
context(logger: Logger) fun User.doSomething() = ...

Our intention is in fact to make the library author think what is the intended mode of use of the type being defined. Not every class is suitable to be thrown as context receiver directly, since it (potentially) pollutes the scope with lots of implicitly-available functions.

Why have context receivers, then? There are several (interrelated) reasons, which in many cases boil down to the same distinction between value parameters and extension receivers we already have in Kotlin.

If this feature grows in popularity, I am afraid almost everyone will just add context to all public declarations.

We took this concern quite seriously during the design. Our point of view, however, is that this problem is not that big.

  1. As mentioned above, the goal is for named context parameters to serve the main needs, and those need no context.
  2. If you "extend" a type using a context,

    context(a: A) blah(): String = ...

    this function is also available when A is used as context receiver, since it declares a context.

bitspittle commented 4 months ago

Having processed my thoughts a bit more, and talking with some friends, I can see the value in an unnamed context parameter, so I partially retract my previous comment. (For example, a class can be used as a control scope; that can be pretty neat.)

I am even growing to support the consume method (although I prefer the suggested receiver rename proposed above).

I still think context functions are a misstep. It feels like a bunch of complexity which opens up a lot of confusing design decisions for code authors and can produce code that just looks confusing to read and wrap your head around (a class with a mix of context and non-context methods makes it harder to understand IMO). I'm still not sure what the benefit of that complexity buys the language.

At this point, I'd rather that unnamed context parameters did not populate the current method scope at all. If you want to use a method on an unnamed context parameter, then just summon it:

context(Session, Users, logger: Logger)
fun User.delete() {
   val users = summon<Users>()
   users.remove(this)
   logger.log("User removed: ${user.id}")

   // Or use `with(summon<Users>) { ... }` if you really want to populate the scope with its methods
}

// Or maybe just name the `Users` parameter  instead?

Wouldn't this approach prevent the need for context funs? I'm worried I'm missing something obvious, so please feel free to correct me.

serras commented 4 months ago

This is unobvious then if the class A is required to be passed as a context receiver or not in the following snippet. And have to do the second option then?

class A {
    context(Int) fun f() = Unit
}
fun main() {
    context(3, A()) {
        f() // is it ok?
        // if not, then how to make it ok? `context context(Int) fun f()`?
        // if it is, how to escape such an exposure
    }
}

Here f is a member of A which requires Int in the context to be called. The "member" part means that we must call it using the usual call syntax, either explicit with dot, or implicit if A is the defining class or an extension receiver.

fun main() {
  val a = A()
  context(3) { a.f() }
}

fun A.g() = context(1) { f() }
quickstep24 commented 4 months ago

First of all, I really appreciate the introduction of named context parameters. I am still trying to realize the other changes.

As I understand, the context modifier now serves two quite different purposes:

a) it marks a callable as accessible from an unnamed context receiver b) if non-empty, it specifies that the callable requires a context when called

To me, these two purposes look completely unrelated and it feels like they should be separated. As is, there is no way to specify that a callable requires a context without making it accessible by receiver. A fix, though more verbose, would be to use context for (a) and context(..) for (b):

class Users {
    context(Logger) 
    fun connect() {}                // requires context but should not be accessible from context

    context(Transaction) 
    context fun setUser() {}        // requires context and should be accessible from context

    context fun getUser() {}        // callable from context but does not require a context
}
Mishkun commented 4 months ago

Why holding to the design with separate context(A) argument list when using named context parameters? Wouldn't it be clearer to use normal parameter list just with some keyword preceding context parameters. e.g.

// here with is a keyword (a hommage to the typeclasses KEEP)
fun myAwesomeContextFunction(x: Int, with logger: Logger) {
  logger.log(x)
}

with(Logger()) {
  myAwesomeContextFunction(1) // log: 1
  myAwesomeContextFunction(2, OtherLogger()) // other_log: 2
}
myAwesomeContextFunction(1) // err: no context
myAwesomeContextFunction(2, OtherLogger()) // other_log: 2
quickstep24 commented 4 months ago

It's even debatable whether Logger is a good example of context receiver, and shouldn't just be a named context parameter. In that case, the regular declaration suffices, no need for context.

interface Logger {
    fun log(message: String) 
}

This is a good example of the problems that library users will face when accessibility by receiver requires marking by the library author. The author may believe that it is better to use as a context parameter, but the library user may prefer an unnamed receiver (for whatever reason). Wrapping every callable with with (logger) {...} is not a very attractive alternative.

pablisco commented 4 months ago

I wonder how confusing, or complicated, would be to overload the with keyword. In terms of semantics it would be quite nice 🙂

with<NetworkContext>().fetchUser()
quickstep24 commented 4 months ago

Can someone shed some light on §E.2 for me.

// do not do this
context(ConsoleLogger(), ConnectionPool(2)) {
  context(DbUserService()) {
    ...
  }
}

// better be explicit about object creation
val logger = ConsoleLogger()
val pool = ConnectionPool(2)
val userService = DbUserService(logger, pool)
// and then inject everything you need in one go
context(logger, userService) {
   ...
}

It looks like context is used as a replacement for with here.

serras commented 4 months ago

Some more answers to the comments below. But before, a general comment: there are definitely cases which are difficult or impossible to express in this design. In the process leading to it we've tried to ponder the likeliness of every scenario; things which we don't think are going to be done, or we think should be done in other way, get less priority.

As I understand, the context modifier now serves two quite different purposes [...] To me, these two purposes look completely unrelated and it feels like they should be separated.

You're actually correct, and for some time we discussed a similar design with two different keywords. However, at some point we realized that adding context parameters to a function is already a marker for "opting in to the feature", so context context(A, B) wouldn't be necessary. In other words, the scenarios where you would have context parameters but not mark the function as context were really slim. From there we tried to uniformize the treatment of context(A, B) and context, leading to the current design of "context is a shorthand for context()".

Why holding to the design with separate context(A) argument list when using named context parameters?

One important design decision was that we did not want to interleave context and value parameters. That complicates resolution too much; since value parameters have additional powers like default values or varargs that wouldn't fit in context parameters.

This gave us two options: put all the parameters at the front or at the end. From those, the front seemed the best place because some parameters are receivers, and receivers are conventially written at the beginning in Kotlin.

Finally, the requirement to write the context before the fun keyword context(A, B) fun is needed for backwards compatibility. If not, it wouldn't be possible to know if fun context(A, B) is the beginning of a declaration of function context, or a function with a context.

Other potential syntactic options are explored in the previous iteration of context receivers.

This is a good example of the problems that library users will face when accessibility by receiver requires marking by the library author. The author may believe that it is better to use as a context parameter, but the library user may prefer an unnamed receiver (for whatever reason).

That is the crux of the issue. After consideration, we as a team considered that the author of the API has primacy in this case over the user, since it designs the API as a whole, but also any documentation or educational material which explains the intended mode of use. We found no compelling reasons, in most cases, to override the library author design; hence the "escape hatches" are not particularly simple.

At this point, I'd rather that unnamed context parameters did not populate the current method scope at all. If you want to use a method on an unnamed context parameter, then just summon it.

I honestly think the two options here are either have receivers which populate in some capacity the implicit scope, or just get rid of them completely and have only named context parameters (this is what Scala does, for example).

serras commented 4 months ago

Can someone shed some light on §E.2 for me. [..] It looks like context is used as a replacement for with here.

In some sense, context is a "version" of with, except that the arguments enter the implicit scope as context receivers, so the methods available in the block are only the contextually visible. Another way to see the difference is with the types:

fun <A, R> with(x: A, block: A.() -> R): R
fun <A, R> context(x: A, block: context(A) () -> R): R

I wonder how confusing, or complicated, would be to overload the with keyword.

The problem is that with<(A) -> B> { ... } would become confusing (or give a resolution error). Note also that with is a regular function right now, not a built-in keyword.

serras commented 4 months ago

"Well you see, yeah, here "thing" is kind of public, but then "log" is really public. I mean, for "thing", you need to summon it..."

This is not a bad thing! Keeping with the example of a Logger, there might be some functions to configure the logger which you don't want to make available when using it as context.

class ConsoleLogger {
  context fun log(message: String)
  var outputStream: OutputStream
}

fun main() {
  // when we create the logger, we use "explicit" syntax
  val logger = ConsoleLogger()
  logger.outputStream = STDERR
  // and now we use this built logger contextually
  context(logger) {
    // outputStream is not accessible
    log("Hello")
  }
}
SPC-code commented 4 months ago

summon is terrible. Otherwise, I see this proposal as a direct continuation of the previous one. My main concern is the handling of lambda functions with contexts. In general, it will look like this:

context(MyScope) fun doSomethingInScope(block: context(MyScope) suspend Container.()->Unit){}

It looks cumbersome. I guess it is not that bad since functions like this will exist only in libraries.

One of the primary problems with previous prototype was ambiguity caused by mixing context and dispatch receivers in lambdas. It seems like introduction of context modifier will improve this situation a bit. You can't use this to refer to context receiver and it is the only way you can refer to the dispatch receiver. But it still does not make a lot of sense.

While dispatch receiver could be (or could not) important for inheritance, it does not make sense for lambdas. I can't see difference between dispatch and context receivers for lambdas and function types.

It is not that bad as it is right now, but I think it would be better to revisit one of the initial suggestions: [MyScope, Container].()->Unit for function types only. Obviously, it is not compatible with regular dispatch receiver notation because it would be not clear, which of receivers is designated by this. But both notations could exist simultaneously.

kyay10 commented 4 months ago

An alternative name for summon that I've used for a while is given, but that might be even more confusing. I think something like context<Foo>() is likely sufficient. I like receiver too. Otherwise, great proposal!

One unaddressed thing is perhaps some way to bring in contexts without nesting. It would need language level support, but it would allow for more complicated features later on like bringing in a context inside a class, or bringing in a context perhaps in a file, or even for a whole module eventually (similar to how typeclasses work). It would also mean that explicit config for contexts isn't needed, and instead one can do something like:

context(DbConfig(), Logger()):
context(App()):
appContextualFunction()

But perhaps this should be a wider proposal for kotlin to support some syntax to define a lambda to the end of the current scope.

quickstep24 commented 4 months ago

Maybe someone can clue me into a use-case where unnamed context parameters are really important. Are they really worth propagating so much noise through the rest of the language for them? Will this be a lot of burden on library developers, who now have to think about every class they write in the context of being used as an unnamed context parameter?

DSLs (§C) make heavy use of context receivers and if you want to extend an existing DSL you need more than one receiver. Type classes (§D) need more than one receiver if combined. They more or less have to be unnamed.

quickstep24 commented 4 months ago

Do you plan a grace period, where we can use unmarked callables from a context, perhaps with some opt-in? Libraries will not add context modifiers until this feature is stable and todays experimental users would have to change their existing code until they do.

serras commented 4 months ago

One unaddressed thing is perhaps some way to bring in contexts without nesting. [...] But perhaps this should be a wider proposal for kotlin to support some syntax to define a lambda to the end of the current scope.

Indeed! There are many possibilities, and from those we think with properties are the best one. Alas, this would require an additional proposal, since the impact in the language would be bigger. In addition, during our interviews with users it seemed that providing a context function with several arguments would be enough in most of the cases.

Do you plan a grace period, where we can use unmarked callables from a context, perhaps with some opt-in?

No, the idea is to enforce this rule since the beginning. Any feature we provide without restriction is difficult to restrict after the fact, since we would break some code.

However, we'll work closely with the rest of the ecosystem to provide these annotations as soon as possible. We have already investigated which are the libraries more apt for this conversion, and talked informally to some of the maintainers.

CLOVIS-AI commented 4 months ago

Thanks for the update, it's great to see how this is moving.

First, as multiple people have mentioned before, I'm not a fan of the summon name, and would be happier with something more "down-to-earth" like receiver.

After thinking about it for some time, I think the contextual visibility is a good idea. One of the worries with context receivers would be that they would be abused in application code (in the same way that regular receivers are sometimes abused, but worse since there are fewer restrictions). However, since this visibility does not apply to regular receivers, I'm worried this may make the language harder to learn.

However, I'm not convinced by marking functions with this visibility. As the KEEP explains (emphasis mine):

§A: (implicit use case) […] A Repository class for a particular entity or a Logger are good examples of this mode of use.

§B (scope use case): In this case we use the context parameter as a marker of being inside a particular scope, which unlocks additional abilities. A prime example is CoroutineScope, which adds launch, async, and so on.

§C (DSLs use case): In this case, contexts are used to provide new members available in a domain-specific language.

Also, as @serras said:

Our intention is in fact to make the library author think what is the intended mode of use of the type being defined. Not every class is suitable to be thrown as context receiver directly, since it (potentially) pollutes the scope with lots of implicitly-available functions.

The ecosystem already uses extension receivers to describe "contexts", like CoroutineScope or Ktor's Application. For those it makes sense to expose them as contextual, since they allow new extensibility modes.

In all of these sentences, "it makes sense to use something in a contextual manner" is said about a type, not about an operation. I believe the ability to use something as an unnamed context receiver is a property of the type itself.

context interface HtmlScope {
    fun body(block: context(HtmlScope) () -> Unit)
}

In this interpretation, the parameter-less version of context is used to annotate types, not methods. I believe this is much more intuitive, which makes it easier to learn:

This is also a much simpler rule to explain to users, when inevitably someone will add a type that is not meant for it as a context receiver, and will not understand why IDEA autocompletes some methods but not others.

As a library author, I'm having a hard time finding a use case in which some methods of a class would be allowed in contexts but not others—either the class is meant as a DSL, and all its methods are written with this use case in mind, or it's not, in which case none are.

I believe having an explicit way to definitely mark which types were written with scope pollution in mind, and which were not, would be a great feature.

(This would also potentially allow context typealias to enable contextual usage for external types, e.g. those coming from Java, but I don't know if this is a good idea).

(If there is a chance that contextual classes are introduced later, I would prefer contextual class Bar so context(Foo) class Bar is still available; I believe context(Foo) contextual class Bar is clear that this is a class that can be used as context which itself requires a context to be instantiated).

serras commented 4 months ago

In all of these sentences, "it makes sense to use something in a contextual manner" is said about a type, not about an operation. I believe the ability to use something as an unnamed context receiver is a property of the type itself.

We've also toyed with this idea, but it actually doesn't solve the problem of scope pollution. How do you ensure that functions like toString or equals are not in scope? You need a more fine-grained control at the level of members.

Nevertheless, if it turns out that marking every function with context is required, one option from the top of my head is writing a plug-in similar to all-open that just marks every member with it.

As a library author, I'm having a hard time finding a use case in which some methods of a class would be allowed in contexts but not others.

As mentioned above, think about members which are inherited from Any, or functions like let or apply. You don't really want those to result in ambiguity just because you added a context receiver.

In some sense, this feature is tricky because we tend to think on interfaces for this feature, which have a very clear set of operations. However, we cannot just say "context visibility is only for interfaces", because many of the useful types in the Kotlin ecosystem are nowadays defined as abstract classes.

This would also potentially allow context typealias to enable contextual usage for external types, e.g. those coming from Java, but I don't know if this is a good idea

Adding modifiers to typealias which somehow change the semantics of the replacement break the expectation of "typelias are intechangeable with their expansion". There's already some problems with this in actual typealias, my colleague @nikitabobko has written a great summary of those in this YouTrack issue.

Mishkun commented 4 months ago

One obstacle on harnessing context receivers for dependency injection in general is the ability to partially provide context in some generic way

                                   subtyping is not allowed
    fun <X, Y> provide(lambda: context(X, Y) () -> Unit, a: Y): context(X) () -> Unit {
        // some code using X
        val newLambda: context(X) () -> Unit = {
            val x = this // ??? how to reference X here?
           // new proposal: val x = summon<X>() or use named context parameter
            with(a) {
                lambda(x, a)
            }
        }
        return newLambda
    }

    class A
    class B

    context(A, B) fun someFun() {
        // some code using A and B
    }
   val someFunWithA = provide(::someFun, A())

As far as I understand the proposal, we could lift one obstacle with referencing the context parameters in lambdas. But what with the generic named parameters?

damianw commented 4 months ago

Looking over the proposal, one thing that isn't clear to me is how the resolution works within the body of a class - particularly ones which aren't originally designed to work with context parameters (or may even be written in Java, JS, etc).

The original KT-10468 issue provides this example as a motivation for the original context receivers design:

context(View)
val Float.dp get() = this * resources.displayMetrics.density

context(View)
val Int.dp get() = this.toFloat().dp

With this, one would be able to have something like:

class MyView : View {
    val height = 8.dp
}
  1. Does MyView's (or any View's) class body qualify as a context parameter?
  2. In this example, resources is a member of View, which is both not context, nor is it even written in Kotlin. Is it even accessible in this way?

It's not clear to me whether the use-case described in the original issue is solved by the new proposal. If it still is, then great! If not, what solution (if any) is there to achieve the desired effect?

bitspittle commented 4 months ago

@serras

I honestly think the two options here are either have receivers which populate in some capacity the implicit scope, or just get rid of them completely and have only named context parameters (this is what Scala does, for example).

I think this is a really important decision. Can the feature designers consider highlighting it somewhere in the original feature proposal?

My current understanding: One way (unnamed with implicit scope) adds complexity to the language for some scoping safety, while the other way avoids this complexity at the cost of requiring a bit more typing in the (rare?) case that someone wants to use methods directly from an unnamed context parameter.

@serras

This is not a bad thing! Keeping with the example of a Logger, there might be some functions to configure the logger which you don't want to make available when using it as context.

We face this all the time already when writing Kotlin -- I have a class (or interface) which exposes more functionality to the user than I want. At that point, it's normal to add a new interface or wrapping class and give them that instead. It's easy enough to do, the mental model for understanding each class is easy, and this is already supported by Kotlin today without any changes.

@CLOVIS-AI

I believe having an explicit way to definitely mark which types were written with scope pollution in mind, and which were not, would be a great feature.

I'm OK with this suggestion. I can tell the feature designers want users to carefully consider what they use for unnamed context parameters, and they don't want them to just slap anything in there. In that case, marking the type with context and saying "Look, I wrote this explicitly with context parameters in mind" seems like a reasonable decision with minimal language impact.

I might even wonder if it's worth thinking through only allowing this context modifier to only work on interfaces at first ("context interfaces"?), to avoid concerns with populating the scope with ambiguity for inherited methods like toString (since this keeps getting brought up).

@quickstep24

DSLs (§C) make heavy use of context receivers and if you want to extend an existing DSL you need more than one receiver. Type classes (§D) need more than one receiver if combined. They more or less have to be unnamed.

I actually made a comment later that I'm OK with unnamed context parameters, but I still don't follow "They [...] have to be unnamed." Maybe you can share a code snippet example?

@serras

You're actually correct, and for some time we discussed a similar design with two different keywords.

I think this is a great example of the invisible technical complexity being added by this feature that will additionally make it really hard for new learners of Kotlin to understand it. Here, two concepts are subtly different but mostly overlap so it sounds like we've compromised by wrapping them behind a single keyword.

I'd be more comfortable with the compromise if it couldn't be helped, if this was the only way forward to provide some functionality. But we can also sidestep this for now and not support implicit scoping and still accomplish everything the original feature design set out to do, right?


However the context parameter feature gets added to the language is a forever decision. If we do it one way now and think it's a mistake later or notice our assumptions are wrong (e.g. is " In other words, the scenarios where you would have context parameters but not mark the function as context were really slim." really true?), it can't be undone.

The single responsibility principle pushes us to design classes that are focused and simple, with clear purpose. context funs push developers to design classes that behave two different ways depending on the context in which they are used. I'm worried this is a misstep.

I think limiting context parameters to never add to the implicit scope is the simplest way forward that still lets everyone do what the original proposal sets out to accomplish. Given the complexity of the language is going to increase quite a bit otherwise, I would really really love for someone to show me a concrete usecase where unnamed context parameters + implicit scope is significantly better than just using summon.

serras commented 4 months ago

The single responsibility principle pushes us to design classes that are focused and simple, with clear purpose. context funs push developers to design classes that behave two different ways depending on the context in which they are used.

I don't think this is the case. Each type still has a (preferred) mode of use. You can see it even in current Kotlin, a type like CoroutineScope is thought to be used implicitly (as extension receiver, since there are no contexts yet), and only in a handful of cases like scope in ViewModel this is done differently. But we don't say that CoroutineScope has too modes of use.

Given the complexity of the language is going to increase quite a bit otherwise, I would really really love for someone to show me a concrete usecase where unnamed context parameters + implicit scope is significantly better than just using summon.

I am a bit confused by this. If you don't want the implicit scope, you can just use named context parameters. summon is really a last resource, which we need to provide as language designers (we need to provide access to every value introduced by the user, unless explicitly ignored by using something like _), but shouldn't be used in most cases in code.

Does MyView's (or any View's) class body qualify as a context parameter?

There are two answers to this:

TadeasKriz commented 4 months ago

@bitspittle

Given the complexity of the language is going to increase quite a bit otherwise, I would really really love for someone to show me a concrete usecase where unnamed context parameters + implicit scope is significantly better than just using summon.

If I understand you correctly, then this wouldn't be possible and would be really unfortunate to use:

interface ViewBuilder {
    context fun label(title: String)

    context fun vstack(block: context(ViewBuilder) () -> Unit)
}

context(ViewBuilder)
fun buildSomeView() {
    vstack {
        label("Hello")
        label("World")
    }
}

Instead it'd require:

context(ViewBuilder)
fun buildSomeView() {
    summon<ViewBuilder>().vstack {
        summon<ViewBuilder>().label("Hello")
        summon<ViewBuilder>().label("World")
    }
}

or

context(ViewBuilder)
fun buildSomeView() = with(summon<ViewBuilder>()) {
    vstack {
        with(summon<ViewBuilder>()) {
            label("Hello")
            label("World")
        }
    }
}

But I may be misunderstanding what you mean, so please let me know :)

mgroth0 commented 4 months ago

Thank you for this update and for the opportunity to comment on it. I fully support and welcome most of these changes. To keep this short, I will only comment below on some of the changes that I have issues with

Removal of this@Type syntax, introduction of summon()

I fully support the removal of this@Type syntax.

However, as many others have commented, I strongly dislike the choice of the word summon. I much prefer the suggestion by @alllex above:

Another alternative could be to overload the new context() function. All versions that take arguments would be used to introduce context, as in context(foo) { ... }. While the parameterless overload would be used to extract the context as in val t = context().

If the overload of context is not possible, my second choice would be the other choice by @alllex of receiver.

I like how @CLOVIS-AI put it: we need something more "down-to-earth."

I want an even more principled approach to deciding which word to use. Here is my suggestion for principle number 1: Do not use a verb. Instead, use a noun.

Suggestions such as summon, materialize, borrow or identify are all verbs. But using a verb here is wrong because this function is not doing anything. Instead, it is purely syntax. It is a part of the language for referring to an object, not an action we perform. That is why I strongly prefer context or receiver (most preferably context<A>). context is very legible, as it reads as a noun: reference to context A .

Context Visibility

I fully support context visibility restrictions.

I understand the idea from @CLOVIS-AI to eventually allow context classes which will automatically make all of their members context-visible, or context typealias. However, I am currently leaning against these ideas in favor of being restricted to more fine-grained control. After all, as the new proposal mentions, when all class members are context-visible, you get ambiguity with shared members such as toString().

No subtyping check between different context parameters

I fully support this, but only if the compiler throws ambiguity errors.

Disallowing of contexts in constructors

I am satisfied with the reasons provided in the updated proposal, namely:

  • Constructors in Kotlin are already restricted from using suspend
  • A constructor with contexts can be faked using the invoke operator in a companion object

Most importantly, as the update said, this feature can be added in the future.

Dropping of Context-in-classes

Eventually, a feature that solves this problem will be needed. But I welcome introducing context receiver features in phases and understand that this, or something like this, is likely best added in a later phase.

TadeasKriz commented 4 months ago

First of all, thanks for this, looks like an overall improvement (expect for the summon, that'll hopefully change to something like receiver<> or even this<>).

I do wish the constructor and class contexts were included. But since that functionality is currently buggy anyway, it's better to be reintroduced in the future. Hopefully not a far too distant one class contexts are perfect for dependency injection and class factories.

The context visibility is nice, but I have to "vote" for having an explicit way for a user-site override. One thing I've seen over the years is that us library authors never foresee new ways to use our API. On the user site it's often frustrating having to write boilerplate like the with(summon<Something>()) { ... }.

My main worry would be utility DSL, which is essentially written in the user code, but uses contexts like fun doSomething(block: context(JavaType, OtherJavaType) () -> Unit) or fun doSomething(block: context(3rdPartyType, Other3rdPartyType) where we don't have control over the type and it might not have been thought of as a scope even (any interface mostly).

quickstep24 commented 4 months ago

@bitspittle

Maybe you can share a code snippet example?

See example from §D

context(comparatorT: Comparator<T>) fun <T> max(x: T, y: T) =
  with (comparatorT) { if (x.compareTo(y) > 0) x else y }

You need with or run scope for every call, which is very far from the original concept of type classes.

bitspittle commented 4 months ago

@TadeasKriz Thank you for your comment.

For your suggestion, I would do one of these to avoid context fun:

// 1, normal Kotlin + summon

interface VStackScope {
   fun label(title: String)
}

class ViewBuilder {
    fun vstack(block: VStackScope.() -> Unit) {
      ViewStackScope().block() 
   }
}

context(ViewBuilder)
fun buildSomeView() = with(summon<ViewBuilder>()) {
   vstack {
      label("Hello")
      label("World")
   }
}

// 2, context interfaces as suggested by CLOVIS

context interface VStackScope {
   fun label(title: String)
}

context interface ViewBuilder {
   context(VStackScope) fun vstack(block: () -> Unit)
}

context(ViewBuilder)
fun buildSomeView() {
   vstack {
       label("Hello")
       label("World")
   }
}

Let me know if I missed some nuance though.

TadeasKriz commented 4 months ago

@bitspittle In my example, I wanted to keep it simple, so I just used one scope, so you were able to partially work around it. However, it breaks down if you have more than one context, which for DSLs is not uncommon. The issue with context interface is that it doesn't allow you to have something that you don't want implicitly visible and opens the question about what to do with declarations inherited from Any.

Also, the need to do with(summon<ViewBuilder>()) at all is an unnecessary boilerplate when using DSLs.

bitspittle commented 4 months ago

@TadeasKriz

context interface would imply you designed an interface where everything should be intentionally public to the context.

By restricting the keyword to interfaces and not classes, you shouldn't have to worry about inherited methods. If the context interface developer inherits from another interface, then they are opting into surfacing those methods to the context.

TadeasKriz commented 4 months ago

One thing I may have missed in the proposal is how the inheritance sorts out something like this:

interface ScopeA {
  fun hello()
}

interface ScopeB { 
  fun world()
}

class ABImpl: ScopeA, ScopeB {
  override fun hello() {
    println("Hello")
  }

  override fun world() {
    println("World")
  }
}

context(a: ScopeA, b: ScopeB) fun helloworld() {
  a.hello()
  b.world()
}

Do I call it like this:

context(ABImpl()) {
  helloworld()
}

or would I have to do something like:

val impl = ABImpl()
context(a = impl, b = impl) {
  helloworld()
}

And if so, how would it work if both were unnamed?

TadeasKriz commented 4 months ago

@bitspittle

context interface would imply you designed an interface where everything should be intentionally public to the context.

By restricting the keyword to interfaces and not classes, you shouldn't have to worry about inherited methods. If the context interface developer inherits from another interface, then they are opting into surfacing those methods to the context.

AFAIK all interfaces inherit implicitly from Any and have toString() don't they? Or do you mean that when you inherit from an interface that isn't context interface then those wouldn't be visible?

And it unfortunately still doesn't solve the problem I mentioned and also creates a couple new ones. Like:

context interface ThisIsScope {
  fun directlyCallable()
}

interface ExtendsScope: ThisIsScope {
  fun notDirectlyCallable()
}

In this case, what happens when I add ExtendsScope as an unnamed context? Can I call directlyCallable implicitly, or only through summon? What about notDirectlyCallable?

And then there's the use case where you want something to configure the scope. There was the example with logger, where you could have something that configures it that's only available through summon (damn I hope that name's gonna change, it sounds like it's dynamically getting it from somewhere).

damianw commented 4 months ago

@serras

But it's part of the implicit scope, so it is available during context resolution (it can "fill in" a context parameter). So in the case above you can use the .dp without further complication.

Is there an exception made for resources not being marked with context in this case?

bitspittle commented 4 months ago

@TadeasKriz You would not be able to use anything but context interfaces as unnamed context parameters.

And yes, the implication would be only the methods explicitly inherited are exposed to the context. No Any stuff.

context interface ThisIsScope {
  fun directlyCallable()
}

interface ExtendsScope: ThisIsScope {
  fun notDirectlyCallable()
}

context(ThisIsScope)
fun fine()

context(ExtendsScope)
fun compileError()
quickstep24 commented 4 months ago
context(ViewBuilder)
fun buildSomeView() = with(summon<ViewBuilder>()) {
   vstack {
      label("Hello")
      label("World")
   }
}

@bitspittle That's not the same. label call is using the outer context from buildSomeView, not the inner one from vstack.

bitspittle commented 4 months ago

@quickstep24 For the user, it's the same experience. Meanwhile, this avoids the need for context fun, and the class design in my approach is more tightly aligned with the single responsibility principle. Actually reviewing the original example I see what you're saying. Thinking it through.

I'm still thinking through the multi-scope case, because it's clear that label will want to be called from more contexts than just vstack. But I'm still guessing there's a way to design the code in a way that works without needing context fun.

zhelenskiy commented 4 months ago

If I have an ambiguity, I would like to be able to exclude some of the contexts. How is it possible?

bitspittle commented 4 months ago

@serras

I don't think this is the case. Each type still has a (preferred) mode of use.

I'm thinking of something like this in the wild:

class LongClassThatExtendsAcrossPages {
   fun a()
   fun b()
   context fun c()
   fun d()
   fun e()
   context fun f()   
   fun g()
   fun h()
   fun i()
   context fun j()
}

This class is "a-j" normally or "c, f, j" if used in a context.

Imagine instead:

context interface ContextRelevant {
   fun c()
   fun f()
   fun j()
}

class LongClassThatExtendsAcrossPages : ContextRelevant {
   fun a()
   fun b()
   override fun c()
   fun d()
   fun e()
   override fun f()   
   fun g()
   fun h()
   fun i()
   override fun j()
}

ContextRelevant here is now embracing SRP - it's a thing that exists for a clear purpose, which is to be used as an unnamed context parameter. You can add header docs to it if needed.

I hope this shows how the first class is really the second example interface+class combined into one (so, not SRP anymore).

quickstep24 commented 4 months ago

I am with @bitspittle here. If a class is intended to be used as a context receiver but would polute the scope, the author should provide a reasonable interface. However, I see no need to mark this as a context interface. It should be obvious from the name (LoggerScope) or from the documentation. This leaves the choice to the user (at her own risk) and would not introduce compatibility issues with Java/JS/...

I have been using context receivers for some time now and I have not faced any problems with scope polution so far. toString et al. from Any will not polute your scope because the local receiver has higher priority. Do the interviews with other customers indicate different insights? If it is really required, maybe there could be an annotation to hide a callable from receiver scope?

In short, I would be very happy if named context parameters were added but unname context receivers kept as they are in the prototype today.