Skip to content

Fix 'no element found' issue in KtAnnotationEntry.qualifiedName#2914

Open
bcorso wants to merge 1 commit intogoogle:mainfrom
bcorso:bcorso/get-symbols-with-annotation-fix
Open

Fix 'no element found' issue in KtAnnotationEntry.qualifiedName#2914
bcorso wants to merge 1 commit intogoogle:mainfrom
bcorso:bcorso/get-symbols-with-annotation-fix

Conversation

@bcorso
Copy link
Copy Markdown
Collaborator

@bcorso bcorso commented May 1, 2026 •
edited by jaschdoc
Loading

This fixes a case where the experimentalPsiResolution strategy was failing due to an incorrect use-site on an annotation.

Fixes #2913

jaschdoc
jaschdoc reviewed May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jaschdoc jaschdoc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Just a minor comment about the test.

Comment thread kotlin-analysis-api/testData/getSymbolsWithAnnotation/annotationsWithUseSiteTargets.kt Outdated
Comment thread ...in-analysis-api/src/main/kotlin/com/google/devtools/ksp/common/impl/PsiResolutionStrategy.kt
@bcorso bcorso force-pushed the bcorso/get-symbols-with-annotation-fix branch from 52a5357 to 7820982 Compare May 5, 2026 20:55
@bcorso bcorso requested a review from jaschdoc May 5, 2026 20:59
@jaschdoc
Copy link
Copy Markdown
Collaborator

jaschdoc commented May 6, 2026

Also fixes the Psi implementation of #2912

jaschdoc
jaschdoc previously approved these changes May 6, 2026
View reviewed changes
@jaschdoc jaschdoc mentioned this pull request May 6, 2026
Open
@jaschdoc
Copy link
Copy Markdown
Collaborator

jaschdoc commented May 6, 2026

I will merge this when you have rebased on main :)

@bcorso bcorso force-pushed the bcorso/get-symbols-with-annotation-fix branch from 7820982 to 7a9f55c Compare May 6, 2026 15:33
@jaschdoc
Copy link
Copy Markdown
Collaborator

jaschdoc commented May 6, 2026

Okay, GitHub is having some issues right now, so I can't submit a review, but the Negative and Bug annotations can stay on the abstract function abstract fun testAllUseSiteTargetAppliedToAnnotationList() and also fun testFieldAndPropertyUseSiteTargetOnConstructorParameters() in KSPUnitTestSuite

@bcorso
Copy link
Copy Markdown
Collaborator Author

bcorso commented May 6, 2026 •
edited
Loading

Okay, GitHub is having some issues right now, so I can't submit a review, but the Negative and Bug annotations can stay on the abstract function abstract fun testAllUseSiteTargetAppliedToAnnotationList()

Done.

and also fun testFieldAndPropertyUseSiteTargetOnConstructorParameters() in KSPUnitTestSuite

Do we still need the Negative and Bug annotations on testFieldAndPropertyUseSiteTargetOnConstructorParameters? That bug is fixed by this PR so I thought we can remove them?

@bcorso bcorso force-pushed the bcorso/get-symbols-with-annotation-fix branch from 7a9f55c to 1ce2699 Compare May 6, 2026 16:42
jaschdoc
jaschdoc reviewed May 7, 2026
View reviewed changes
Comment thread kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt Outdated
@jaschdoc
Copy link
Copy Markdown
Collaborator

jaschdoc commented May 7, 2026 •
edited
Loading

Do we still need the Negative and Bug annotations on testFieldAndPropertyUseSiteTargetOnConstructorParameters? That bug is fixed by this PR so I thought we can remove them?

Yes, the idea is to have the issue directly linked in the code, so if the test breaks, you can look up the issue and see if it's a regression test or reproducing an unfixed bug. It gives an easy way to look up why a test is there. The Negative annotation is a marker annotation to show it's testing invalid input. There is not tooling for it (yet - that's maybe something to be explored in the future but not important right now), but it helps with issue tracking and testing / documentation.
There is also #2930 which extends the Bug annotation to show if it's fixed or not.

@bcorso
Fix 'no element found' issue in KtAnnotationEntry.qualifiedName
7514894 
This fixes a case where the experimentalPsiResolution strategy was
failing due to an incorrect use-site on an annotation.

BUG: google#2913

# Conflicts:
#	kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPUnitTestSuite.kt
@bcorso bcorso force-pushed the bcorso/get-symbols-with-annotation-fix branch from 1ce2699 to 7514894 Compare May 7, 2026 15:31
@bcorso bcorso requested a review from jaschdoc May 7, 2026 15:32
jaschdoc
jaschdoc approved these changes May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jaschdoc jaschdoc left a comment

Choose a reason for hiding this comment

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

Thank you for the changes. Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaschdoc jaschdoc jaschdoc approved these changes

@troelsbjerre troelsbjerre Awaiting requested review from troelsbjerre troelsbjerre is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

experimentalPsiResolution fails with "no element found"

2 participants

@bcorso @jaschdoc