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

Context receivers give ParseError #397

Closed
bddckr opened this issue Apr 23, 2023 · 2 comments
Closed

Context receivers give ParseError #397

bddckr opened this issue Apr 23, 2023 · 2 comments
Assignees

Comments

@bddckr
Copy link
Contributor

bddckr commented Apr 23, 2023

Using com.diffplug.spotless:spotless-plugin-gradle:6.18.0 configured to use ktfmt().kotlinlangStyle() fails to parse usage of context receivers.

Minimum example:

class A {
    context(String)
    fun test() {}
}

leads to

Step 'ktfmt' found problem in 'A.kt':
2:5: error: Expecting member declaration
com.facebook.ktfmt.format.ParseError: 2:5: error: Expecting member declaration
	at com.facebook.ktfmt.format.Parser.throwParseError(Parser.kt:71)
	at com.facebook.ktfmt.format.Parser.parse(Parser.kt:65)
	at com.facebook.ktfmt.format.Formatter.sortedAndDistinctImports(Formatter.kt:143)
	at com.facebook.ktfmt.format.Formatter.format(Formatter.kt:90)
	at com.diffplug.spotless.glue.ktfmt.KtfmtFormatterFunc.apply(KtfmtFormatterFunc.java:54)
	at com.diffplug.spotless.FormatterFunc.apply(FormatterFunc.java:32)
	at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:82)
	at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:88)
	at com.diffplug.spotless.Formatter.compute(Formatter.java:246)
	at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:203)
	at com.diffplug.gradle.spotless.IdeHook.performHook(IdeHook.java:58)
	at com.diffplug.gradle.spotless.SpotlessExtensionImpl.lambda$createFormatTasks$9(SpotlessExtensionImpl.java:86)

According to the context receivers proposal here, the earliest Kotlin release that supports them is 1.6.20-M1. However, ktfmt seems to be using 1.6.10 as seen here. Am I correct to assume that that's the issue?


#314 seems related, but according to the comments in that issue, ktfmt at one point worked when using context receivers. This issue here instead explains that ktfmt is basically crashing due to an issue parsing the source code.

@bddckr
Copy link
Contributor Author

bddckr commented Apr 24, 2023

Adding this to TokenizerTest shows the issue:

@Test
fun `Context receivers are parsed correctly`() {
  val code = """
    |class A {
    |  context(String)
    |  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", "(", "String", ")", "\n", "  ", "fun", " ", "test", "(", ")", " ", "{", "}", "\n", "}")
          .inOrder()
  assertThat(tokenizer.toks.map { it.index })
          .containsExactly(0, -1, 1, -1, 2, -1, -1, 3, 4, 5, 6, -1, -1, 7, -1, 8, 9, 10, -1, 11, 12, -1, 13)
          .inOrder()
}

Changing pom.xml to use <kotlin.version>1.6.20-M1</kotlin.version> makes that test pass.

However, this is where my knowledge ends 😅 Adding this to FormatterTest shows that further changes are required to support context receivers, just like #314 explains.

@Test
fun `context receivers`() =
  assertFormatted(
      """
    |class A {
    |  context(String)
    |  fun test() {}
    |}
    |"""
          .trimMargin())

@davidtorosyan
Copy link
Contributor

@bddckr I don't think #314 indicates that ktfmt used to support context receivers.

Rather, any situations in which they work right now are incidental. In that issue, they're parsed but not formatted correctly.

In your example, they're not parsed at all.


Normally I'd close this as a duplicate, but the fact that this causes a crash (instead of just bad formatting) means we should consider a mitigation at least (if a full fix is hard).

@strulovich has there been any investigation into this?

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 a pull request may close this issue.

2 participants