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

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

Closed
ShikaSD opened this issue Jul 17, 2024 · 16 comments
Closed

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

ShikaSD opened this issue Jul 17, 2024 · 16 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@ShikaSD
Copy link

ShikaSD commented Jul 17, 2024

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
Copy link

liutikas commented Aug 8, 2024

It seems to have been introduced in #418 by @nreid260 as a workaround for a bug in 1.9.10.

Does this workaround make sense for 2.0.0+?

@nreid260
Copy link
Contributor

nreid260 commented Aug 8, 2024

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

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
Copy link

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
Copy link

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
Copy link

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

@hick209 hick209 added bug Something isn't working help wanted Extra attention is needed labels Aug 23, 2024
@jsjeon
Copy link
Contributor

jsjeon commented Aug 27, 2024

#513 passed smoke_test locally...

@jsjeon
Copy link
Contributor

jsjeon commented Aug 27, 2024

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

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Aug 27, 2024
This results in slowdown of ./gradlew ktCheck --rerun from 22s to 82s
as we end up creating a lot more classloaders.

https://ge.androidx.dev/s/6uo5wcowlhsay/performance/build
https://ge.androidx.dev/s/xa5e5cnnd54sw/performance/build

This is needed as a workaround for facebook/ktfmt#495
so we can keep using old kotlin compiler for ktfmt as we upgrade to KGP
2.0.20

Test: ./gradlew ktCheck
Change-Id: I1220bbef9e679e67d4ab7e0afaf4d4f7be7678d3
@hick209 hick209 linked a pull request Aug 28, 2024 that will close this issue
@Sercurio
Copy link

Same problem on 2.0.21

@WeisSebastian
Copy link

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
Copy link
Contributor

hick209 commented Oct 25, 2024

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

@hick209
Copy link
Contributor

hick209 commented Oct 26, 2024

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

@hick209
Copy link
Contributor

hick209 commented Oct 30, 2024

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

Should be available soon to everyone. Thanks for your patience

@hick209 hick209 closed this as completed Oct 30, 2024
@StefanLobbenmeierObjego
Copy link

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
Copy link

sureshg commented Nov 5, 2024

@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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants