Skip to content

Fix outer DSL qualifier propagation for create(::...) safety hints#48

Open
1250422131 wants to merge 1 commit into
InsertKoinIO:mainfrom
1250422131:fix/create-outer-qualifier
Open

Fix outer DSL qualifier propagation for create(::...) safety hints#48
1250422131 wants to merge 1 commit into
InsertKoinIO:mainfrom
1250422131:fix/create-outer-qualifier

Conversation

@1250422131

Copy link
Copy Markdown

The Pull Request

Related to #41.

This PR fixes qualifier collection for create(::...) registrations when the qualifier is declared on the enclosing DSL call.

At first, I thought the issue was that @Named on the constructor/class was not being collected correctly. After digging into it, I found that the actual missing piece was the outer DSL qualifier:

val appModule = module {
    single(qualifier = named("Demo1")) { create(::createDemoRoot) }
}

The runtime registration has a qualifier, but the compiler plugin did not carry that outer qualifier into the collected DSL definition used by compile-safety validation.

I understand that the annotation-first style is already supported and may be the more idiomatic style for the compiler plugin:

@Named("local")
class LocalDatabase : Database

@Named("remote")
class RemoteDatabase : Database

// Use @Named on parameters to specify which one to inject
class SyncService(
    @Named("local") val localDb: Database,
    @Named("remote") val remoteDb: Database
)

// DSL - plugin reads @Named from classes and parameters
val appModule = module {
    single<LocalDatabase>()
    single<RemoteDatabase>()
    single<SyncService>()
}

However, for users coming from classic Koin DSL, putting the qualifier on the registration site is also a natural style:

single(qualifier = named("local")) { create(::LocalDatabase) }
single(qualifier = named("remote")) { create(::RemoteDatabase) }

I am not fully sure whether supporting this style fits the intended design of the Koin compiler plugin. If this goes against the project direction, please feel free to close the PR.

With this change, both forms are supported:

  • qualifier declared on the outer DSL call
  • qualifier declared on the target class/function via @Named / @Qualifier

When both are present, the outer DSL qualifier takes priority, matching the actual DSL registration site.

Changes

  • Capture the qualifier from the enclosing DSL definition call.
  • Reuse that qualifier when collecting DslDef entries from create(::constructor) and create(::function).
  • Add a box regression test for single(qualifier = named(...)) { create(::function) }.

Verification

./gradlew :koin-compiler-plugin:test --tests org.jetbrains.kotlin.compiler.plugin.template.runners.JvmBoxTestGenerated\$Safety.testDsl_create_function_outer_qualifier

Capture the enclosing DSL qualifier in KoinDSLTransformer and reuse it when
collecting DslDef entries from create(::constructor) and create(::function).
This keeps compile-safety hints aligned with registrations such as
single(qualifier = named("Demo1")) { create(::createDemoRoot) }.

Add a box regression test covering outer qualifier + create(::function).
@aiKrice

aiKrice commented Jun 23, 2026

Copy link
Copy Markdown

@arnaudgiuliani Can you have a look in this PR ? Seems it fixes our issue

@arnaudgiuliani

arnaudgiuliani commented Jun 23, 2026

Copy link
Copy Markdown
Member

Yep, will look at it!

@aiKrice

aiKrice commented Jun 29, 2026

Copy link
Copy Markdown

@arnaudgiuliani Do you plan to do anything for this PR ?

@arnaudgiuliani

Copy link
Copy Markdown
Member

Got heavily delayed ... working on it this week 👍

@arnaudgiuliani arnaudgiuliani added this to the 1.0.2 milestone Jun 29, 2026
@arnaudgiuliani arnaudgiuliani self-requested a review June 29, 2026 10:31
@1250422131

Copy link
Copy Markdown
Author

Thanks for your attention on this PR.

Just to be transparent: I used AI assistance while preparing this change. My goal is to help make the Koin compiler plugin more robust, but I’m aware that AI-assisted code should be reviewed carefully. Please feel free to point out anything that looks wrong, fragile, or not aligned with the project.

@arnaudgiuliani

Copy link
Copy Markdown
Member

Thanks for this @1250422131 🙏 — the outer-DSL-qualifier propagation is exactly the right fix for #41, and we've incorporated it into the upcoming 1.0.2 release (with credit to you in the commit).

One thing we adjusted on top while integrating it, so you're aware: the new extractFromExpression() used the legacy positional accessors getValueArgument(0) / getTypeArgument(0). Those throw on the zero-argument named<T>() overload (the type-qualifier form your own KDoc lists), e.g.:

single(qualifier = named<MyQualifier>()) { create(::createDemoRoot) }
// -> IrGenerationExtensionException: No such value argument slot in IrCallImpl: 0 (total=0)

and they're also removed in Kotlin 2.4.0. We swapped all four sites to the plugin's bounds-safe compat shims (getRegularArgument / getTypeArgumentCompat), which both fixes the named<T>() crash (it now falls through to type-qualifier extraction) and keeps us 2.4.0-safe. We also added box tests for the named<T>() form and for outer-qualifier-vs-@Named priority.

Net: your change ships in 1.0.2. Much appreciated! 🎉

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 this pull request may close these issues.

3 participants