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

KSP processing does not see member injections from typealiased superclass in 2.50 #4199

Closed
ZacSweers opened this issue Dec 25, 2023 · 2 comments

Comments

@ZacSweers
Copy link

ZacSweers commented Dec 25, 2023

When testing with Dagger 2.50, it looks like its KSP implementation does not see through typealias'd superclasses. This seems to be a regression, as it works in 2.49.

Repro

abstract class ActualBase {
  @Inject lateinit var string: String
}

typealias Base = ActualBase

class InjectClass : Base() {
  @Inject lateinit var numbers: List<Int>

  override fun equals(other: Any?): Boolean {
    if (this === other) return true
    if (javaClass != other?.javaClass) return false

    other as InjectClass

    if (numbers != other.numbers) return false
    if (string != other.string) return false

    return true
  }

  override fun hashCode(): Int {
    var result = numbers.hashCode()
    result = 31 * result + string.hashCode()
    return result
  }
}

The generated MembersInjector class however is missing the base class's string member

package com.squareup.test;

import dagger.MembersInjector;
import dagger.internal.DaggerGenerated;
import dagger.internal.InjectedFieldSignature;
import dagger.internal.QualifierMetadata;
import java.util.List;
import javax.annotation.processing.Generated;
import javax.inject.Provider;

@QualifierMetadata
@DaggerGenerated
@Generated(
    value = "dagger.internal.codegen.ComponentProcessor",
    comments = "https://dagger.dev"
)
@SuppressWarnings({
    "unchecked",
    "rawtypes",
    "KotlinInternal",
    "KotlinInternalInJava"
})
public final class InjectClass_MembersInjector implements MembersInjector<InjectClass> {
  private final Provider<List<Integer>> numbersProvider;

  public InjectClass_MembersInjector(Provider<List<Integer>> numbersProvider) {
    this.numbersProvider = numbersProvider;
  }

  public static MembersInjector<InjectClass> create(Provider<List<Integer>> numbersProvider) {
    return new InjectClass_MembersInjector(numbersProvider);
  }

  @Override
  public void injectMembers(InjectClass instance) {
    injectNumbers(instance, numbersProvider.get());
  }

  @InjectedFieldSignature("com.squareup.test.InjectClass.numbers")
  public static void injectNumbers(InjectClass instance, List<Integer> numbers) {
    instance.numbers = numbers;
  }
}
@bcorso
Copy link

bcorso commented Dec 26, 2023

Thanks for reporting this!

My guess is that it's due to 5f8b76c, where we changed the implementation of XTypes#nonObjectSuperclass(). That should have been a non-functional change but this sounds like there's a bug in XProcessing. I'll take a look and report back once I have more information.

@bcorso
Copy link

bcorso commented Dec 27, 2023

Filed https://issuetracker.google.com/317818395, and I'll have the fix submitted this week.

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Dec 27, 2023
This CL fixes the issue in google/dagger#4199.

The bug was due to checking if the `KSDeclaration` was a
`KSClassDeclaration` without taking into consideration that the
declaration could be a `KSTypeAlias`. To fix this, I've replaced the
type aliases in the `KSType` before checking the declaration.

BUG: 317818395
Test: XTypeElementTest.kt
Change-Id: I110f6723fbb9d153a9c6d8dea74b7fbcf9146a74
copybara-service bot pushed a commit that referenced this issue Jan 9, 2024
The fix for #4199 is included in the new XProcessing jars (aosp/2891694), so this CL also adds a regression test for that issue.

Fixes #4199

RELNOTES=Fixed #4199
PiperOrigin-RevId: 597032576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants