Aliased import ignored for nullable variants of that type

[RaoPrashanth]

• docs/changelog.md has been updated if applicable.
  • Changes not visible to library consumers, such as build script, documentation, or test code updates, don't need to
    be added to the changelog.
• CLA signed.

Closes #2020. Closes #2021.

[JakeWharton]

Seems fine? What did it do before, insert the regular import or create another automatic alias?

[RaoPrashanth]

Seems fine? What did it do before, insert the regular import or create another automatic alias?

Yes, It creates another automatic alias and becomes 2 member type imports (one for Object and another one for nullable Object)
Eventually, throws exception
Exception in thread "main" java.lang.IllegalArgumentException: Collection has more than one element.

[RaoPrashanth]

@JakeWharton Can you help me to merge the PR, please?

[Egorand]

I'd like to take a look as well and will merge today/tomorrow if no feedback!

[Egorand]

The proposed fix does not work that well if I change ClassName(packageName, "PersonId") in the test to be a nested class (ClassName(packageName, "Person", "Id")):

expected:
    package org.example
    
    import org.example.Person.Id as PID
    
    public fun pid(): PID? = PID()
    
but was:
    package org.example
    
    import org.example.Person as ExamplePID
    import org.example.Person.Id as PID
    
    public fun pid(): ExamplePID.Id? = ExamplePID.Id()

Produced code is correct, but it's super awkward and includes and doesn't actually use the import alias we explicitly passed in.

I think a better fix would be to ensure the type we're adding to importableTypes is always non-nullable. Interestingly, className.topLevelClassName() always returns a non-nullable ClassName (probably a separate bug), but for className we need to remove nullability manually:

  if (simpleName !in importableMembers) {
    // Maintain the inner class name if the alias exists.
    val newImportTypes = if (alias == null) {
      topLevelClassName
    } else {
-     className
+     className.copy(nullable = false) as ClassName
    }
    importableTypes[simpleName] = importableTypes.getValue(simpleName) + newImportTypes
}

I'll add it as a suggestion along with the tweaked test.

[RaoPrashanth]

@Egorand: Thank you very much for the good suggestion.
Now, PR is ready for the next step (re-review/merge)

[Egorand]

Looks good, thanks for addressing the feedback!

[Egorand]

@RaoPrashanth please run ./gradlew :kotlinpoet:spotlessApply to fix up code formatting violations.