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

Add support for context-receivers #1233

Merged
merged 22 commits into from Apr 18, 2022

Conversation

DRSchlaubi
Copy link
Contributor

@DRSchlaubi DRSchlaubi commented Apr 12, 2022

This PR aims at implementing support for context receivers (#1232)

It currently only tries to implement a possible design for builders and specs and has no code generation logic

@OptIn(ContextReceivers::class)
public fun main() {
  val contextFunction = FunSpec.Builder("withContext")
    .contextReceiver(InputStream::class)
    .build()

  val contextLambda = LambdaTypeName.get(
    receiver = STRING,
    contextReceivers = listOf(InputStream::class.asTypeName()),
    returnType = STRING
  )
}

Fixes #1232

Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

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

Are you open to implementing the code gen part of it? I don't think it would be too difficult.

General design looks ok, left comments where there need to be changes.

In the future, please don't PR unrelated formatting changes that deviate from the code style we use, it makes the PR quite noisy.

@DRSchlaubi
Copy link
Contributor Author

DRSchlaubi commented Apr 12, 2022

Are you open to implementing the code gen part of it? I don't think it would be too difficult.

I was going to propose an implementation with this design as well; however, I didn't I ran into an issue, which is why I just proposed a design, once that is finished I would be open to implement the code gen

In the future, please don't PR unrelated formatting changes that deviate from the code style we use, it makes the PR quite noisy.

I didn't mean to do that, looks like IntelliJ just ignored your .editorconfig

@DRSchlaubi
Copy link
Contributor Author

I hope I caught all the code style violations?

DRSchlaubi and others added 5 commits April 13, 2022 01:32
- Use proper experimental annotation
- Remove not needed overloads
- Move contextReceivers parameter to the end
Co-authored-by: Zac Sweers <pandanomic@gmail.com>
- Add more tests
- Add new-line after context()
Copy link
Collaborator

@Egorand Egorand left a comment

Choose a reason for hiding this comment

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

Sweet! Added a few comments, some of them (non-nit ones) would be nice to address before merging.

@Egorand
Copy link
Collaborator

Egorand commented Apr 14, 2022

@DRSchlaubi also please sign our CLA before we can merge you PR.

@DRSchlaubi
Copy link
Contributor Author

Done

@Egorand
Copy link
Collaborator

Egorand commented Apr 18, 2022

Think it just needs a ./gradlew :kotlinpoet:spotlessApply.


assertThat(funSpec.toString()).isEqualTo(
"""
|context(@com.squareup.kotlinpoet.FunSpecTest.TestAnnotation kotlin.String)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you confirm this is legal syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using it in IntelliJ crashes intellij, however there is no real rules in the KEEP for how to use annotations and the compiler accepts it

DRSchlaubi and others added 2 commits April 18, 2022 17:53
Co-authored-by: Egor Andreevich <github@egorand.dev>
@Egorand Egorand merged commit 6d919af into square:master Apr 18, 2022
@Egorand
Copy link
Collaborator

Egorand commented Apr 18, 2022

Merged, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for context receivers
3 participants