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

nested java class resolves to error with kotlin compile testing 1.3.5 #263

Open
yigit opened this issue Jan 21, 2021 · 9 comments
Open

nested java class resolves to error with kotlin compile testing 1.3.5 #263

yigit opened this issue Jan 21, 2021 · 9 comments
Labels
P3 desirable

Comments

@yigit
Copy link
Collaborator

yigit commented Jan 21, 2021

I couldn't yet reproduce this in KSP so might not be a KSP issue but after updating room to use kotlin compile testing 1.3.5 (where there was a bug fix to pass down java files as root files), a room test started failing:

Test input:

            import java.util.List;
            class Baz {
                int intField;
                List<Integer> listOfInts;
                List incompleteGeneric;
                Nested nested;
                static class Nested {
                }
            }

Resolving the nested field's type returns error.

override fun resolve(): KSType {
        val resolvedType = ResolverImpl.instance.resolveUserType(this)
        return if ((resolvedType.declaration as? KSClassDeclarationDescriptorImpl)?.descriptor is NotFoundClasses.MockClassDescriptor) {
            KSErrorType
        } else resolvedType
    }

Another change was to update kotlin to 1.4.21-2, that might also be related.
Will investigate further.

@yigit
Copy link
Collaborator Author

yigit commented Jan 21, 2021

can reproduce w/ kotlin compile testing w/o room.
still investigating.
yigit/kotlin-compile-testing@2e35c09

@yigit
Copy link
Collaborator Author

yigit commented Jan 21, 2021

and it works fine if i don't add it as a java source roots:

https://github.com/google/ksp/blob/master/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingPlugin.kt#L200

maybe our testing infra does not do that?

@yigit
Copy link
Collaborator Author

yigit commented Jan 21, 2021

in kotlin compile testing, resolvedType.declaration is KSClassDeclarationDescriptorImpl while in KSP test infra, it is KSClassDeclarationJavaImpl.

@yigit
Copy link
Collaborator Author

yigit commented Jan 21, 2021

moved to kotlin compile testing as i cannot reproduce this in a sample project.
tschuchortdev/kotlin-compile-testing#105

@yigit yigit closed this as completed Jan 21, 2021
@yigit
Copy link
Collaborator Author

yigit commented Jan 21, 2021

i can reproduce this little bit easier by finding the nested class directly and calling asStarType because it fails with NPE in:

private val descriptor: ClassDescriptor? by lazy {
        ResolverImpl.moduleClassResolver.resolveClass(JavaClassImpl(psi))
    }

but again, works fine in ksp testing infra, fails in kotlin compile testing.

@yigit yigit reopened this Jan 22, 2021
@yigit
Copy link
Collaborator Author

yigit commented Jan 22, 2021

i have some level of repro for this:
yigit@8b15edb

still trying to isolate and not sure why i couldn't repro in my test. similarly it happens in a java source class but it is actually a root class, not a nested one.

@yigit
Copy link
Collaborator Author

yigit commented Jan 22, 2021

to keep the bug updated, it happens in KSP test infra when the file path does not match the class's package name.

This is something IDE complains about in a regular project but nevertheless, javac compiles it w/o any issues (and kotlin compiler is fine w/ that too).

I'm still not sure how/why we create the PSI file triggers this, not i'm not sure how to find the correct source root for kotlin compile testing.

@yigit
Copy link
Collaborator Author

yigit commented Jan 22, 2021

so tl;dr;
I created a sample project where i have:

package foo.bar;
class JavaSrc {
}
package foo.baz;
class KotlinSrc {
}

This code compiles fine.
But if I change KotlinSrc class to reference JavaSrc, compilation fails.

Seems like the tl;dr; is that this is OK for java but not ok from kotlin.
Moreover, if I move that JavaSrc to a dependent module, it again works fine from kotlin (because it is a class file).

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Jan 26, 2021
This version includes a fix where java sources were not
provided to KSP, which means getSymbolsAnnotatedWith didn't
work.

Unfortunately, this update hit a bug in KSP/KCT (not sure yet) where KSP
cannot find java sources. After a long investigation (see github
issues), I figured it is the lack of javaSourceRoots argument being
passed into KotlinCompilation that confuses KSP (but regular kotlin
compilation works fine). KCT does not set it and cannot easily set it
since its public API receives any file as an argument (so it cannot
calculate a root). For this reason, now Room's abstraction passes that
argument directly.

As part of that change, I also changed how SourceFile's are converted.
Now we always build a "proper" folder structure before calling KCT. I've
refactored all 3 places where we use KCT to use this common path.

tschuchortdev/kotlin-compile-testing#105
google/ksp#263

Test: existing tests
Change-Id: Ibd0d6fc46849c2a0489b2bb632a2c19862e31c9e
@ting-yuan
Copy link
Collaborator

Per offline discussion, the behavior is aligned to kotlinc when used through kotlin-gradle-plugin. Investigations are needed when calling directly.

@ting-yuan ting-yuan added the P3 desirable label Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 desirable
Projects
None yet
Development

No branches or pull requests

2 participants