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

null value in annotation arguments if annotation originates from Java and argument is Kotlin's const val #839

Closed
Jeffset opened this issue Feb 13, 2022 · 6 comments · Fixed by #954
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Jeffset
Copy link
Contributor

Jeffset commented Feb 13, 2022

Assume we have a Kotlin source, named Constants.kt, as follows:

const val FOO = "foo"

And a Java source:

class Bar {
    @Named(ConstantsKt.FOO) void set(/* */) { /* */ }
}

When @Named annotation (KSAnnotationJavaImpl) is resolved, it yields ["value:null"] from its arguments property, with no way to retrieve the "foo" value.

Named = javax.inject.Named for completeness.

So, basically, Kotlin constants are not properly resolved in Java annotation arguments.

@Jeffset Jeffset changed the title null value in annotation arguments if annotation originates from Java and argument is Kotlin const val null value in annotation arguments if annotation originates from Java and argument is Kotlin's const val Feb 13, 2022
@Jeffset
Copy link
Contributor Author

Jeffset commented Feb 15, 2022

I've edited the description, as it indeed only fails to resolve, if const val is top-level.

@ting-yuan ting-yuan added the bug Something isn't working label Feb 16, 2022
@ting-yuan ting-yuan added this to the 1.0.5 milestone Mar 3, 2022
@Jeffset
Copy link
Contributor Author

Jeffset commented Mar 6, 2022

The issue is that JavaPsiFacade.findClass() yields null when resolving synthetic file facade classes (e. g. ConstantsKt), thus any references from Java code to Kotlin top-level properties/functions are virtually inaccessible for KSP.

I'm not sure how to correctly resolve the issue, for now it seems that PsiReference.resolve() (and PsiConstantEvaluationHelper.computeConstantExpression() in turn) for Java Psi Impls have no notion about Kotlin's facade classes; yet Kotlin plugin for intelliJ, which is backed by Kotlin compiler code, correctly resolves these (?) references from Java, obviously.
So should we develop a workaround on the KSP level, or apply some other Kotlin-aware approach for computing constant expressions, or is this an issue with core compiler code?

@Jeffset
Copy link
Contributor Author

Jeffset commented Mar 7, 2022

Also this leaves us with the fundamental problem - how to model synthetic file facades in KSP, if they are used as classes directly, like in @SomeAnnotation(clazz = ConstantsKt.class). For now we are getting ErrorType there.

@neetopia
Copy link
Collaborator

neetopia commented Mar 8, 2022

I am thinking about adding a synthetic implementation of KSFile for such file classes, still trying to recall why I did not have such classes in place.

@neetopia
Copy link
Collaborator

neetopia commented Mar 8, 2022

One problem I can think of is the name mangling, we might end up replicating the name mangling logic for file classes.

@Jeffset
Copy link
Contributor Author

Jeffset commented Apr 16, 2022

Thanks for the fix!

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

Successfully merging a pull request may close this issue.

3 participants