search

facebook / ktfmt

A program that reformats Kotlin source code to comply with the common community standard for Kotlin code conventions.
https://facebook.github.io/ktfmt/
Apache License 2.0
933 stars 78 forks source link

Kotlin 2.0.20-Beta2: Exception when parsing a property accessor #495

Closed ShikaSD closed 1 month ago

ShikaSD commented 4 months ago

Exception is thrown when parsing the following code:

class FilteringExecutor(
    private val delegate: ExecutorService = Executors.newSingleThreadExecutor()
) : Executor {
    var filterFunction: (Runnable) -> Boolean = { true }
        // failing expression
        set(value) {
            field = value
            reEnqueueDeferred()
        }
        // failing expression

    private fun reEnqueueDeferred()  {}
}

It appears that accessor.stub can be null with 2.0.20.

Full exception:

org.jetbrains.kotlin.com.intellij.psi.PsiInvalidElementAccessException: Element: class org.jetbrains.kotlin.psi.KtParameterList; stub hierarchy is invalid: org.jetbrains.kotlin.com.intellij.psi.impl.source.SubstrateRef$StubRef@23756ebe (class org.jetbrains.kotlin.com.intellij.psi.impl.source.SubstrateRef$StubRef) has null containing file stub
        at org.jetbrains.kotlin.com.intellij.psi.impl.source.SubstrateRef$StubRef.getContainingFile(SubstrateRef.java:110)
        at org.jetbrains.kotlin.com.intellij.extapi.psi.StubBasedPsiElementBase.getContainingFile(StubBasedPsiElementBase.java:235)
        at org.jetbrains.kotlin.com.intellij.extapi.psi.StubBasedPsiElementBase.getNode(StubBasedPsiElementBase.java:119)
        at org.jetbrains.kotlin.com.intellij.psi.PsiInvalidElementAccessException.getPsiInvalidationTrace(PsiInvalidElementAccessException.java:98)
        at org.jetbrains.kotlin.com.intellij.psi.PsiInvalidElementAccessException.<init>(PsiInvalidElementAccessException.java:61)
        at org.jetbrains.kotlin.com.intellij.psi.stubs.StubBase.<init>(StubBase.java:31)
        at org.jetbrains.kotlin.psi.stubs.impl.KotlinStubBaseImpl.<init>(KotlinStubBaseImpl.kt:34)
        at org.jetbrains.kotlin.psi.stubs.impl.KotlinPlaceHolderStubImpl.<init>(KotlinPlaceHolderStubImpl.java:28)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.getParameterListWithBugFixes(KotlinInputAstVisitor.kt:1428)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.access$getParameterListWithBugFixes(KotlinInputAstVisitor.kt:133)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$declareOne$2$1.invoke(KotlinInputAstVisitor.kt:1401)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$declareOne$2$1.invoke(KotlinInputAstVisitor.kt:1393)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$declareOne$2.invoke(KotlinInputAstVisitor.kt:1393)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$declareOne$2.invoke(KotlinInputAstVisitor.kt:1387)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.declareOne(KotlinInputAstVisitor.kt:1387)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.access$declareOne(KotlinInputAstVisitor.kt:133)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitProperty$1.invoke(KotlinInputAstVisitor.kt:443)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitProperty$1.invoke(KotlinInputAstVisitor.kt:442)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visitProperty(KotlinInputAstVisitor.kt:442)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitProperty(KtVisitorVoid.java:497)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitProperty(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtProperty.accept(KtProperty.java:57)
        at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visit(KotlinInputAstVisitor.kt:2596)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.access$visit(KotlinInputAstVisitor.kt:133)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassBody$1$2.invoke(KotlinInputAstVisitor.kt:1965)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassBody$1$2.invoke(KotlinInputAstVisitor.kt:1965)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassBody$1.invoke(KotlinInputAstVisitor.kt:1965)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassBody$1.invoke(KotlinInputAstVisitor.kt:1924)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$emitBracedBlock$1.invoke(KotlinInputAstVisitor.kt:414)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$emitBracedBlock$1.invoke(KotlinInputAstVisitor.kt:411)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.emitBracedBlock(KotlinInputAstVisitor.kt:411)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visitClassBody(KotlinInputAstVisitor.kt:1924)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassBody(KtVisitorVoid.java:545)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassBody(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtClassBody.accept(KtClassBody.kt:43)
        at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visit(KotlinInputAstVisitor.kt:2596)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.access$visit(KotlinInputAstVisitor.kt:133)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassOrObject$1.invoke(KotlinInputAstVisitor.kt:1550)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor$visitClassOrObject$1.invoke(KotlinInputAstVisitor.kt:1514)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block(KotlinInputAstVisitor.kt:2568)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.block$default(KotlinInputAstVisitor.kt:2564)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visitClassOrObject(KotlinInputAstVisitor.kt:1514)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassOrObject(KtVisitorVoid.java:473)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassOrObject(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtVisitor.visitClass(KtVisitor.java:33)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:33)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:467)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtClass.accept(KtClass.kt:22)
        at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visit(KotlinInputAstVisitor.kt:2596)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visitScript(KotlinInputAstVisitor.kt:2517)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitScript(KtVisitorVoid.java:527)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitScript(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtScript.accept(KtScript.java:69)
        at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visit(KotlinInputAstVisitor.kt:2596)
        at com.facebook.ktfmt.format.KotlinInputAstVisitor.visitKtFile(KotlinInputAstVisitor.kt:2492)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtFile(KtVisitorVoid.java:521)
        at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtFile(KtVisitorVoid.java:21)
        at org.jetbrains.kotlin.psi.KtFile.accept(KtFile.kt:41)
        at org.jetbrains.kotlin.psi.KtCommonFile.accept(KtCommonFile.kt:244)
        at com.facebook.ktfmt.format.Formatter.prettyPrint(Formatter.kt:109)
        at com.facebook.ktfmt.format.Formatter.format(Formatter.kt:96)
        at androidx.build.BaseKtfmtTask.processFile(Ktfmt.kt:150)
        at androidx.build.BaseKtfmtTask.access$processFile(Ktfmt.kt:84)
        at androidx.build.BaseKtfmtTask$processInputFiles$2$1$1.invokeSuspend(Ktfmt.kt:144)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
        at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
        at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
liutikas commented 3 months ago

It seems to have been introduced in https://github.com/facebook/ktfmt/pull/418 by @nreid260 as a workaround for a bug in 1.9.10.

Does this workaround make sense for 2.0.0+?

nreid260 commented 3 months ago

There are tests covering the expected output. Feel free to try removing this stub, it's absolutely a hack. Otherwise, it's probably simple to expand the stub for the new syntax.

StefanLobbenmeierObjego commented 3 months ago

FYI the issue is still there with the release of kotlin 2.0.20 (so its not just kotlin beta)

I also wanted to share my slightly code snippet which has the same issue:

val Principal.userId: UUID
    get() = UUID.fromString(this.name)
liutikas commented 3 months ago

I looked this a bit more, upgrading kotlin version to 2.0.20 in the ktfmt pom.xml also makes a bunch of these tests fails with this exact stacktrace.

It seems that KotlinPlaceHolderStubImpl used to allow passing in null StubElement, whereas starting with 2.0.20 if throws an exception.

I tried replacing getParameterListWithBugFixes with accessor.parameterList, it stops crashing, but instead it fails tests with incorrect formatting.

liutikas commented 3 months ago

org.jetbrains.kotlin.com.intellij.psi.stubs.StubBase constructor now has

if (parent == null && !(this instanceof PsiFileStub)) {
            throw new PsiInvalidElementAccessException(this.getPsi(), "stub hierarchy is invalid: the parent of " + this + " (" + this.getClass() + ") is null, even though it's not a PsiFileStub", (Throwable)null);
        }

what did not exist before.

liutikas commented 3 months ago

Returning null from getParameterListWithBugFixes when accessor.stub is null also breaks tests.

jsjeon commented 3 months ago

https://github.com/facebook/ktfmt/pull/513 passed smoke_test locally...

jsjeon commented 3 months ago

Just for the record, I'm also trying to fix https://youtrack.jetbrains.com/issue/KT-70922. Once it's done, we won't need getParameterListWithBugFixes at all. cc @nreid260

Sercurio commented 1 month ago

Same problem on 2.0.21

WeisSebastian commented 1 month ago

This currently blocks us from upgrading or kotlin version. @hick209 Is there any plan to release a new ktfmt version that contains the merged PR (#513 )?

hick209 commented 1 month ago

@WeisSebastian, sounds fair to me. I'll try to get a release out today

hick209 commented 1 month ago

I had to fight 3 SEVs today, so I could not get to this, but I will get this shipped next week

hick209 commented 1 month ago

I've just published https://github.com/facebook/ktfmt/releases/tag/v0.53

Should be available soon to everyone. Thanks for your patience

StefanLobbenmeierObjego commented 3 weeks ago

I suspect some time will pass until the next spotless version includes the new ktfmt release (last minor was v6.25 in january and there are now multiple betas for v7), so I will just leave this snippet here:

configure<SpotlessExtension> {
    val ktfmtVersionOverride = "0.53"
    if (KtfmtStep.defaultVersion() >= ktfmtVersionOverride) {
        throw Exception("Remember to remove explicit version!")
    }
    kotlin { ktfmt(ktfmtVersionOverride).kotlinlangStyle() }
    kotlinGradle { ktfmt(ktfmtVersionOverride).kotlinlangStyle() }
}
sureshg commented 3 weeks ago

@StefanLobbenmeierObjego isn't it better to the latest using val ktfmtVersion = maxOf(KtfmtStep.defaultVersion(), libs.versions.ktfmt.get()) instead of throwing error and breaking the build?

StefanLobbenmeierObjego commented 3 weeks ago

Both work, I prefer breaking the build because I want to remove the explicit version when the version finally arrives, so the override only stays in the code for as long as I need it. When renovate / dependabot bumps that version the CI will fail and I will revert that change