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

Support qualifieres in multibinds map keys and set elements. #409

Open
evant opened this issue Jun 26, 2024 · 6 comments · May be fixed by #462
Open

Support qualifieres in multibinds map keys and set elements. #409

evant opened this issue Jun 26, 2024 · 6 comments · May be fixed by #462
Labels
enhancement New feature or request

Comments

@evant
Copy link
Owner

evant commented Jun 26, 2024

As part of #253 we expect to support placing a qualifier on a typealias as a replacement to using the typealias is a qualifier directly, ex: typealias MyString = @Named("name") String. Right now this causes and issue with multibinds as you can't use in the Map or Set type.

abstract val myStringMap: Map<MyString, String>

runs into

e: [ksp] Qualifier: @Named(value=MyString) can only be applied to the outer type

Since we treat these Map and Set here specially we can support the nested qualifier in these cases

@evant evant added the enhancement New feature or request label Jun 26, 2024
@sergeshustoff
Copy link
Contributor

Should it be applicable to both key and value?

@evant
Copy link
Owner Author

evant commented Jun 30, 2024

Good question, I don't actually know that the current behavior is for map multibinds with the same key type but different value types.

@evant
Copy link
Owner Author

evant commented Sep 16, 2024

update: same key but different value type is treated as distinct, I can see usecases for this so it's best to keep, which means that yes this should be applicable to both key and value

@sergeshustoff
Copy link
Contributor

I've researched a little and I see a few problems:

  1. It's kinda weird that generic type qualifiers are conditionally legal and it might be difficult to see if it should be legal or not. Even if it's simplified to only illegal for anything that @Provides, it complicates api

     @Component
     abstract class SetComponent {
         abstract val conditionallyLegal: Set<@Named("foo") String>
     
         @Provides
         @IntoSet
         fun makesFirstOneLegal(): @Named("foo") String = "1"
         
         @Provides
         fun butThisIsIllegalAndMakesFirstOneIllegal(): Set<@Named("foo") String> = setOf("1")
     }
    
  2. abstract val legal1: Set<@Named("foo1") String> and abstract val legal2: Set<@Named("foo2") String> would have equal TypeKey (but not SetKey).

To avoid both problems we would need to include generic's qualifiers (recursively?) into TypeKey comparison and that leads us to maybe supporting type qualifiers for generics in general rather than exclusively for @IntoSet and @IntoMap cases. Wdyt?

@evant
Copy link
Owner Author

evant commented Oct 7, 2024

To avoid both problems we would need to include generic's qualifiers (recursively?)

I started to take a stab at this and reached a similar conclusion.

@sergeshustoff
Copy link
Contributor

What do you think of separating type qualifiers from member qualifiers? It would make folowing 2 things work differently:

@Named("foo") val someting: String
val somethingElse: @Named("foo") String

In theory that might let us untie type qualifiers from everything else and handle them on the level of AstType. And it would be more closely aligned with how typealiases handled now, basically following code would not work:

// typealias Foo = @Named("foo") String
abstract val foo: @Named("foo") String // same as if we used Foo
@Provides @Named("foo") fun provide(): String

sergeshustoff added a commit to sergeshustoff/kotlin-inject that referenced this issue Jan 10, 2025
sergeshustoff added a commit to sergeshustoff/kotlin-inject that referenced this issue Jan 10, 2025
@sergeshustoff sergeshustoff linked a pull request Jan 10, 2025 that will close this issue
sergeshustoff added a commit to sergeshustoff/kotlin-inject that referenced this issue Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants