1.6.20 Regression: Annotations on Java type parameters

[owengray-google]

Describe the bug
<T> void foo(@Nullable java.util.List<@NonNull T> previousList) creates a TypeParameter for T which has no annotations.

Expected behaviour
The created TypeParameter contains the @NonNull annotation.

(I'm not sure whether nullability is correct there, but it is not an error to put a TYPE_USE annotation there.)

Screenshots

This is a regression in 1.6.10-dev-146 -> 1.6.20-M1-dev-148

[IgnatBeresnev]

@owengray-google hi! Is there a test that fails or how did you find this? I'll try to narrow it down, but I need to find a way to reproduce this or check the fix.

I looked at the changes between builds 146 and 148 -- either it's the kotlin update itself or something very not obvious that was broken during refactoring, as other updates don't seem related to it.

[IgnatBeresnev]

Looks like I found the test here:

@Test // REGRESSION: go/dokka-upstream-bug/2388
fun `Full documentation parameters table has types with nullability annotations`() {

If I revert your changes in this test, it should reproduce the problem, right?

[IgnatBeresnev]

Unfortunately, it looks like this problem comes from #2326 (Refactor Ancestry Graphs).

@BarkingBad, any ideas off the top of your head?

How I reproduced it:

1. Clone devsite plugin, revert assert changes from this commit
2. Run the test (Full documentation parameters table has types with nullability annotations), make sure it fails
3. Open dokka, revert Refactor Ancestry Graphs #2326, make a local build and publish it to maven local
4. Use locally built dokka to run the devsite plugin test again, it should pass

[owengray-google]

That CL does have some changes to the part of the code that does this.

I can't see off the top of my head what could be responsible. At the moment, my only guess is that the cause is that the handling of PsiTypeParameter went from using type.annotations() to resolved.annotations(), but I would guess that resolving the type doesn't disturb the annotations.

[IgnatBeresnev]

At the moment, my only guess is that the cause is that the handling of PsiTypeParameter went from using type.annotations() to resolved.annotations(), but I would guess that resolving the type doesn't disturb the annotations

It seems you are right :) Using type.annotations() instead of resolved.annotations() fixes the problem

I've written identical tests for Kotlin and Java, and interestingly enough, this bug is only reproducible for Java. Moreover, there's another issue (or feature, depends on how you look at it), which is to be expected if you think about it.

Take this code:

public <@Nullable T> void foo(List<T> param) {}

• Before, if you looked at the parameter List<T> param, its TypeParameter#extra was empty.
• After - it's not, and it contains propagated @Nullable annotation.

Working on a fix now

[BarkingBad]

Hi, sorry for missing this conversation, I had a busy time recently.

Indeed, this regression was already fixed by me here https://github.com/Kotlin/dokka/pull/2353/files#r813308178

The PR is complete but there was the redundancy of code that I promised @vmishenev to look into that. Unfortunately, I am really short on time recently and I forgot to finish that, hopefully I will find some time this week

[BarkingBad]

I hope my fix will work, after all I tracked it down by comparing the changes and found out that the previous implementation was having this type as a parameter. Let me know if I could help you somehow @IgnatBeresnev

[IgnatBeresnev]

Oh, I see, thanks!

I think I'll fix it today or tomorrow just so that we can proceed with testing 1.6.20, and I'll also add some unit tests to cover these cases.

But you'll need to rebase and maybe fix some conflicts, hopefully that's OK with you. I'll let you know once it's done

[IgnatBeresnev]

Fixed in master and cherry-picked into the upcoming 1.6.20