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.

Sign up for GitHub

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

Support context receivers #400

Closed
wants to merge 7 commits into from
Closed

Support context receivers #400

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8993576
Support context receivers
bddckr Apr 29, 2023
14e5f1d
Test with annotation and superfluous newlines instead
bddckr Apr 29, 2023
9c3d852
Fix context receiver on class not parsing
bddckr Apr 29, 2023
0694e1e
Remove blank lines after context receiver
bddckr Apr 30, 2023
d9aeecd
Use `visitEachCommaSeparated`'s prefix & postfix for enclosing params
bddckr Jun 19, 2023
8502fd5
Ensure comments can be inside `context`'s parentheses
bddckr Jun 19, 2023
e2fe9bd
Don't break before closing parenthesis in non-Google styles
bddckr Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

<properties>
<dokka.version>0.10.1</dokka.version>
<kotlin.version>1.6.10</kotlin.version>
<kotlin.version>1.6.20-M1</kotlin.version>
<kotlin.compiler.incremental>true</kotlin.compiler.incremental>
<kotlin.compiler.jvmTarget>1.8</kotlin.compiler.jvmTarget>
<main.class>com.facebook.ktfmt.cli.Main</main.class>
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

I may have missed it, but there don't seem to be changes for context receivers on lambda parameters:

fun foo(cb: context(String) (Int) -> Unit) {
  cb("", 0)
}

Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
import org.jetbrains.kotlin.psi.KtConstantExpression
import org.jetbrains.kotlin.psi.KtConstructorDelegationCall
import org.jetbrains.kotlin.psi.KtContainerNode
import org.jetbrains.kotlin.psi.KtContextReceiverList
import org.jetbrains.kotlin.psi.KtContinueExpression
import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
Expand Down Expand Up @@ -124,6 +125,7 @@ import org.jetbrains.kotlin.psi.psiUtil.children
import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespace
import org.jetbrains.kotlin.psi.psiUtil.startOffset
import org.jetbrains.kotlin.psi.psiUtil.startsWithComment
import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes

/** An AST visitor that builds a stream of {@link Op}s to format. */
class KotlinInputAstVisitor(
Expand Down Expand Up @@ -162,6 +164,7 @@ class KotlinInputAstVisitor(
builder.sync(function)
builder.block(ZERO) {
visitFunctionLikeExpression(
function.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST),
function.modifierList,
"fun",
function.typeParameterList,
Expand Down Expand Up @@ -282,6 +285,7 @@ class KotlinInputAstVisitor(
* list of supertypes.
*/
private fun visitFunctionLikeExpression(
contextReceiverList: KtContextReceiverList?,
modifierList: KtModifierList?,
keyword: String,
typeParameters: KtTypeParameterList?,
Expand All @@ -294,6 +298,9 @@ class KotlinInputAstVisitor(
typeOrDelegationCall: KtElement?,
) {
builder.block(ZERO) {
if (contextReceiverList != null) {
visitContextReceiverList(contextReceiverList)
}
if (modifierList != null) {
visitModifierList(modifierList)
}
Expand Down Expand Up @@ -1372,6 +1379,7 @@ class KotlinInputAstVisitor(

builder.block(ZERO) {
visitFunctionLikeExpression(
null,
accessor.modifierList,
accessor.namePlaceholder.text,
null,
Expand Down Expand Up @@ -1533,6 +1541,7 @@ class KotlinInputAstVisitor(

val delegationCall = constructor.getDelegationCall()
visitFunctionLikeExpression(
constructor.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST),
constructor.modifierList,
"constructor",
null,
Expand Down Expand Up @@ -1638,6 +1647,16 @@ class KotlinInputAstVisitor(
builder.forcedBreak()
}

/** Example `context(Logger, Raise<Error>)` */
override fun visitContextReceiverList(contextReceiverList: KtContextReceiverList) {
builder.sync(contextReceiverList)
builder.token("context")
builder.token("(")
visitEachCommaSeparated(contextReceiverList.contextReceivers())
Copy link
Contributor

Choose a reason for hiding this comment

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

visitEachCommaSeparated has parameters for enclosing parens, to handle some weird indentation cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a commit to use that, thank you for pointing that out!

builder.token(")")
builder.forcedBreak()
}

/** For example `@Magic private final` */
override fun visitModifierList(list: KtModifierList) {
builder.sync(list)
Expand Down
11 changes: 11 additions & 0 deletions core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6674,6 +6674,17 @@ class FormatterTest {
assertThatFormatting(code).isEqualTo(expected)
}

@Test
fun `context receivers`() =
assertFormatted(
"""
|class A {
| context(Logger, Raise<Error>)
| fun test() {}
|}
|"""
.trimMargin())

companion object {
/** Triple quotes, useful to use within triple-quoted strings. */
private const val TQ = "\"\"\""
Expand Down
21 changes: 21 additions & 0 deletions core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,25 @@ class TokenizerTest {
.containsExactly(0, -1, 1, 2, 3, -1, 4, -1, 5, 6, 7)
.inOrder()
}

@Test
fun `Context receivers are parsed correctly`() {
val code = """
|class A {
| context(Logger, Raise<Error>)
Copy link
Contributor

@nreid260 nreid260 May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What happens if the list of receivers is longer than the max line length?

What happens if there's a trailing comma?

What happens to comments inside the parens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What happens if there's a trailing comma?

That's a compile error giving error: Type expected. Correct me if I'm wrong, but ktfmt should fail in that case and not try to parse/format when a compile error is encountered.

What happens if the list of receivers is longer than the max line length?
What happens to comments inside the parens?

I've changed the test to include a comment, which also shows that the regular wrapping logic works as it goes over the test's max line length.

Copy link
Contributor

@nreid260 nreid260 Jun 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks. It's very weird to me that trailing commas aren't accepted in the context receiver list. I suspect that's a bug in kotlinc, but nothing we can do right now.

https://youtrack.jetbrains.com/issue/KT-59506/Trailing-commas-are-illegal-in-context-receiver-lists

| fun test() {}
|}
|""".trimMargin().trimMargin()

val file = Parser.parse(code)
val tokenizer = Tokenizer(code, file)
file.accept(tokenizer)

assertThat(tokenizer.toks.map { it.originalText })
.containsExactly("class", " ", "A", " ", "{", "\n", " ", "context", "(", "Logger", ",", " ", "Raise", "<", "Error", ">", ")", "\n", " ", "fun", " ", "test", "(", ")", " ", "{", "}", "\n", "}")
.inOrder()
assertThat(tokenizer.toks.map { it.index })
.containsExactly(0, -1, 1, -1, 2, -1, -1, 3, 4, 5, 6, -1, 7, 8, 9, 10, 11, -1, -1, 12, -1, 13, 14, 15, -1, 16, 17, -1, 18)
.inOrder()
}
}