Fix #2386 - withOptions leaves stale index entries on qualifier/secondary change

[arnaudgiuliani]

Problem

withOptions mutates a BeanDefinition in place, then re-indexes from the mutated state — but never removes the entry registered under the previous coordinates. So:

single<Foo>(named("a")) { Foo() } withOptions { qualifier = named("b") }

leaves both Foo:a (stale) and Foo:b in Module.mappings, pointing at the same factory — get<Foo>(named("a")) still resolves even though the definition's qualifier is now b. With secondaryTypes it gets worse: secondaries are indexed only under the new qualifier, so Foo is reachable via old+new but Bar only via new (asymmetry). Fixes #2386.

Fix

Snapshot the previous qualifier + secondaryTypes before applying the options; when either changes, drop the stale mappings entries (old primary + old secondaries under the old qualifier) before re-indexing with the new coordinates:

val previousQualifier = def.qualifier
val previousSecondaryTypes = def.secondaryTypes.toList()
def.also(options)
if (def.qualifier != previousQualifier || def.secondaryTypes != previousSecondaryTypes) {
    module.mappings.remove(indexKey(def.primaryType, previousQualifier, def.scopeQualifier))
    previousSecondaryTypes.forEach { module.mappings.remove(indexKey(it, previousQualifier, def.scopeQualifier)) }
    module.indexPrimaryType(factory)
    if (def.secondaryTypes.isNotEmpty()) module.indexSecondaryTypes(factory)
}

No-op when nothing index-relevant changed.

Behavioral note (resolution semantics)

A definition is no longer reachable under a qualifier it had before withOptions changed it — this removes unintended/undocumented reachability (the dual-index). Resolution by the definition's current qualifier is unchanged.

Tests

• WithOptionsStaleIndexTest (commonTest, RED→GREEN): qualifier change drops the stale primary index; primary + secondary reachability stay consistent.
• NewDSLTest: corrected three mappings.size expectations that were counting the stale entry (3→2, 3→2, 4→3) — they encoded the bug.

Verification

• :core:koin-core:jvmTest green; wasmJs green except the documented pre-existing createdAtStart reds; native validated via JVM+wasmJs (native run hits the known pre-existing ServiceMessagesParser backtick-reporter crash, unrelated).
• ./gradlew apiCheck — pass (no public API change; mappings is @KoinInternalApi).

🤖 Generated with Claude Code

[arnaudgiuliani]

⚠️ Converting to draft — this fix regresses koin-android DSLExtendedTest.android dsl.

Root cause of the regression: with two same-type definitions —

viewModelOf(::MyViewModel)               // (1) indexes MyViewModel:null
viewModelOf(::MyViewModel){ named("bis") } // (2) indexes MyViewModel:null at creation (overwrites (1)!), then withOptions moves it to :bis

— the *Of(ctor){options} builders index under the null qualifier first and only change it afterward, so (2) transiently collides with (1) at MyViewModel:null. On main the leftover stale :null entry is what keeps getOrNull<MyViewModel>() non-null; removing it (the #2386 fix) exposes that (1) was already clobbered → unqualified resolves to null.

Implication: the clean fix for #2386's single-definition cases is correct (verified by WithOptionsStaleIndexTest), but a complete fix must also stop *Of(ctor){options} from indexing under the pre-options qualifier — i.e. apply options before the initial indexPrimaryType. That's a broader change to the *Of builders (Factory/Single/Scoped/ViewModelOf). Holding until that's designed.

[arnaudgiuliani]

Closing — #2386 is deferred to 4.3.0 (needs the deeper *Of-builder indexing-order fix, not a contained patch change). Investigation, root cause, and the single-definition fix + WithOptionsStaleIndexTest are recorded here and on #2386 for whoever picks it up.