[dkhawk]

Summary

Applies upstream lint fixes, Compose deprecation removals, and loop optimizations:

Changes

1. Removed unused imports (fromInt, NO_MOVEMENT_YET) in CameraMoveStartedReason.kt.
2. Removed deprecated key parameter from rememberSaveable invocations in CameraPositionState.kt and Marker.kt.
3. Replaced .forEach with standard for loops and while-iterator pattern in ClusterRenderer.kt.

Reviewer

cc @LoyalAbbas

[googlemaps-bot]

Code Coverage

Overall Project	24.66%	❌

There is no coverage information present for the Files changed

[github-actions]

Coverage (unit tests)

Compared against 61c3cf0 on main (#1024).

Module	Line %	Change	Branch %	Change
maps-compose	0.00%	no change	0.00%	no change
maps-compose-utils	2.05%	🔺 +0.01	0.48%	🔻 -0.01
maps-compose-widgets	0.00%	no change	0.00%	no change
TOTAL	0.42%	no change	0.09%	no change

_{Line and branch coverage from unit test reports. History is recorded on the coverage-history branch after each merge to main.}

[kikoso]

This is inside the @Deprecated overload, so after this change key stays in the public signature and does nothing. Callers still compile, but their saved-state key flips from their string to currentCompositeKeyHashCode. Three consequences, none of which the compiler surfaces:

• State persisted under the old custom key isn't found after upgrade, so the camera resets once.
• Anyone sharing one key across two call sites silently loses that sharing.
• Anyone using key to distinguish iterations of the same call site (loop, pager) now collides, since positional scoping yields the same hash per iteration. The correct migration is wrapping in the key(...) composable, which nothing in our KDoc currently says.

I grepped every rememberSaveable call site in the repo: the non-deprecated path at line 54 is already keyless, and all three remaining keyed sites are inside @Deprecated declarations. So this change buys nothing on the supported surface, and only alters APIs already on their way out.

Suggestion: leave the body as it was and silence the warning instead, then delete the overload wholesale at the next major, where removing key is an honest feat!: with a migration note.

@Suppress("DEPRECATION")
@Composable
public inline fun rememberCameraPositionState(
    key: String? = null,
    crossinline init: CameraPositionState.() -> Unit = {}
): CameraPositionState = rememberSaveable(key = key, saver = CameraPositionState.Saver) {

If you'd rather keep it as written, that's defensible given the function is already deprecated, but it needs a release-note line pointing loop and pager users at key(...).

[kikoso]

Same as CameraPositionState.kt:75: key survives in the signature as a no-op.

One extra wrinkle here. The deprecation message on line 193 only talks about rememberUpdatedMarkerState and never mentions key, so a caller passing key gets no hint at all that its meaning changed. If we keep this change rather than suppressing, the message needs a sentence about it.

[kikoso]

Non-blocking, but while we're in here: this ReplaceWith can't actually be applied by the IDE. It's a multi-line statement (val markerState = ...) rather than an expression, and it carries no imports for rememberSaveable / Saver. It also contradicts the message right above it, which says to use rememberUpdatedMarkerState.

Either ReplaceWith("rememberUpdatedMarkerState(position)") or dropping replaceWith entirely would be more honest than what's there today.

[kikoso]

This is where the "optimize loops" in the title could actually land.

keys is a List, so key !in keys on line 164 is a linear scan run once per entry in keysToViews, making onClustersChanged O(n*m) on every cluster change. computeViewKeys() already returns a Set, so:

Suggested change

	val keys = clusters.flatMap { it.computeViewKeys() }
	val keys = clusters.flatMapTo(mutableSetOf()) { it.computeViewKeys() }

That makes both membership checks O(1) and dedupes safely (the for (key in keys) loop below adds into a map, so duplicates were never wanted). With a few hundred markers this is the difference that's actually measurable, unlike the forEach to for swaps.

[kikoso]

This hunk is a genuine improvement. The old with(keysToViews.iterator()) { forEach { ... remove() } } was fragile, relying on the interplay between Iterator.forEach and MutableIterator.remove. The explicit while (hasNext()) is clearer and obviously correct. 👍

I'd just reframe it in the PR description though: the forEach to for swaps elsewhere (lines 127 and 169) are readability only. Iterable.forEach and Map.forEach are inline, so the bytecode is effectively identical and there's no perf win to claim there.

[kikoso]

Anchoring here since the two removed lines only exist on the left side of the diff.

The removed fromInt and NO_MOVEMENT_YET imports aren't unused: both are referenced from the enum's KDoc just below, in "[NO_MOVEMENT_YET] is used as the initial state..." and "...an unsupported integer value is provided to [fromInt]". Compilation is fine either way, but could you check the Dokka output still resolves both links after this?

It's also inconsistent: this UNKNOWN import is kept, and it's equally KDoc-only. Its one code use, ?: return UNKNOWN inside the companion, resolves as an enum member without any import. So by the same criterion it would go too. I'd drop all three or keep all three.

[kikoso]

Minor comments @dkhawk , but looks good! Since key stops having an effect, should we call it out in the release notes?

[kikoso]

With this change key is still part of the public signature of rememberCameraPositionState, but it is silently ignored. Anyone passing it to keep separate saved states (e.g. several maps in a list) would get a different behaviour now. Should we deprecate the key parameter and mention in the KDoc that it has no effect, instead of dropping it silently? What do you think?

[kikoso]

Same here: key is now unused. Since rememberMarkerState is already deprecated this is less critical, but could we at least document that key is ignored?

[kikoso]

Just to understand it: were the removed imports only there for the KDoc links to fromInt and NO_MOVEMENT_YET? If so we might lose those links in the generated docs, or do I miss anything?