-
-
Notifications
You must be signed in to change notification settings - Fork 23
Mark runtime stability for composables #64
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
Conversation
WalkthroughThe pull request refactors the Compose stability analyzer's icon and tooltip generation by replacing discrete boolean parameters with a unified Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityLineMarkerProvider.kt (1)
133-235: Runtime icon vs tooltip/explanation are inconsistent for receiver‑only runtime cases.There’s a behavioral mismatch between how you classify runtime stability for the icon and how the tooltip header/explanation are computed:
getIcontreats both parameters and receivers as part ofallParams, so an unstable/runtime receiver influenceshasUnstable/hasRuntimeand thus the color.- In
buildTooltip,isRuntimeOnlyand the runtime explanation block are based only on parameter counts:val stableCount = analysis.parameters.count { it.stability == ParameterStability.STABLE } val unstableCount = analysis.parameters.count { it.stability == ParameterStability.UNSTABLE } val runtimeCount = analysis.parameters.count { it.stability == ParameterStability.RUNTIME } // ... val isRuntimeOnly = unstableCount == 0 && runtimeCount > 0 // ... if (isRuntimeOnly || runtimeCount > 0) { /* Runtime Stability block */ }Later you compute receiver counts (
runtimeReceiverCount, etc.), but they don’t feed back intoisRuntimeOnlyor the runtime explanation condition.Problematic scenario
- No parameters are runtime (
runtimeCount == 0), and no parameters are unstable.- At least one receiver is runtime (
runtimeReceiverCount > 0), and no receivers are unstable.Effect:
getIconseeshasRuntime == trueandhasUnstable == false→ yellow runtime icon.isRuntimeOnly == falseandruntimeCount == 0→ header will not say “Runtime Stability”, and the “🟡 Runtime Stability” block won’t be shown.So the gutter icon advertises runtime stability, but the tooltip never explains why for receiver‑only runtime.
Suggested fix: include receivers in runtime/unstable classification for the tooltip
You’re already computing receiver counts; reuse them when computing
isRuntimeOnlyand deciding whether to show the runtime block. One possible adjustment:- val stableCount = analysis.parameters.count { it.stability == ParameterStability.STABLE } - val unstableCount = analysis.parameters.count { it.stability == ParameterStability.UNSTABLE } - val runtimeCount = analysis.parameters.count { it.stability == ParameterStability.RUNTIME } + val stableCount = analysis.parameters.count { it.stability == ParameterStability.STABLE } + val unstableCount = analysis.parameters.count { it.stability == ParameterStability.UNSTABLE } + val runtimeCount = analysis.parameters.count { it.stability == ParameterStability.RUNTIME } val totalCount = analysis.parameters.size - - // Check if all non-stable parameters are runtime - val isRuntimeOnly = unstableCount == 0 && runtimeCount > 0 + // Receiver information + val stableReceiverCount = analysis.receivers.count { it.stability == ParameterStability.STABLE } + val unstableReceiverCount = analysis.receivers.count { it.stability == ParameterStability.UNSTABLE } + val runtimeReceiverCount = analysis.receivers.count { it.stability == ParameterStability.RUNTIME } + val totalReceiverCount = analysis.receivers.size + + // Check if all non-stable *parameters or receivers* are runtime + val hasAnyRuntime = runtimeCount > 0 || runtimeReceiverCount > 0 + val hasAnyUnstable = unstableCount > 0 || unstableReceiverCount > 0 + val isRuntimeOnly = hasAnyRuntime && !hasAnyUnstable @@ - isRuntimeOnly -> - "🟡 Runtime Stability (skippability determined at runtime)" + isRuntimeOnly -> + "🟡 Runtime Stability (skippability determined at runtime)" @@ - // Receiver information - val stableReceiverCount = analysis.receivers.count { - it.stability == ParameterStability.STABLE - } - val unstableReceiverCount = analysis.receivers.count { - it.stability == ParameterStability.UNSTABLE - } - val runtimeReceiverCount = analysis.receivers.count { - it.stability == ParameterStability.RUNTIME - } - val totalReceiverCount = analysis.receivers.size + // Receiver information (reuse counts above) @@ - // Runtime stability explanation - if (isRuntimeOnly || runtimeCount > 0) { + // Runtime stability explanation (trigger if any runtime param or receiver) + if (hasAnyRuntime) { append("\n\n🟡 Runtime Stability:") append("\nStability is determined at runtime based on") append("\nactual parameter values and their implementations.") append("\nSkippability may change between library versions") append("\nor when parameter implementations change.") }This keeps your parameter/receiver breakdown display as-is, but ensures:
- Yellow icon ⇔ runtime‑aware header and explanation, whether the runtime classification comes from parameters, receivers, or both.
- “Runtime Stability” label is consistent with the same combined notion of runtime/unstable you already use for icons.
🧹 Nitpick comments (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/settings/StabilitySettingsConfigurable.kt (1)
132-135: Runtime gutter color description looks good (small wording nit only).Text correctly explains that stability is runtime-determined and version-dependent. If you want to be extra precise, you could say “no unstable parameters and at least one runtime parameter” instead of “only runtime parameters,” but that’s optional and doesn’t affect behavior.
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityLineMarkerProvider.kt (1)
79-124: Align icon color logic with intent; simplifywhenbranches.The
getIconlogic is mostly sound, but there are two points worth tightening:
- Redundant
isSkippablebranchesCurrently:
val color = when { analysis.isSkippable && allStable -> Color(settings.stableGutterColorRGB) analysis.isSkippable -> Color(settings.stableGutterColorRGB) hasUnstable -> Color(settings.unstableGutterColorRGB) hasRuntime -> Color(settings.runtimeGutterColorRGB) else -> Color(settings.unstableGutterColorRGB) }The first two branches produce the same result, so
allStableis effectively unused. Either:
- Drop the first branch to reduce noise, or
- If you intend to only show green when everything is actually stable (and treat other skippable-but-not-fully-stable cases differently in the future), adjust the second branch to reflect that.
A minimal simplification:
- val color = when { - analysis.isSkippable && allStable -> Color(settings.stableGutterColorRGB) - analysis.isSkippable -> Color(settings.stableGutterColorRGB) - hasUnstable -> Color(settings.unstableGutterColorRGB) - hasRuntime -> Color(settings.runtimeGutterColorRGB) - else -> Color(settings.unstableGutterColorRGB) - } + val color = when { + analysis.isSkippable -> Color(settings.stableGutterColorRGB) + hasUnstable -> Color(settings.unstableGutterColorRGB) + hasRuntime -> Color(settings.runtimeGutterColorRGB) + else -> Color(settings.unstableGutterColorRGB) + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityLineMarkerProvider.kt(7 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/settings/StabilitySettingsConfigurable.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runtime Module Tests
- GitHub Check: Spotless check
- GitHub Check: API check
- GitHub Check: Test IntelliJ Plugin
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Build and Tests
Mark runtime stability for composables.
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.