-
-
Notifications
You must be signed in to change notification settings - Fork 24
Handle StabilityInferred annotated composables #24
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 changes add support for reading Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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
🧹 Nitpick comments (2)
compose-stability-analyzer-idea/CHANGELOG.md (1)
10-10: Consolidate duplicate changelog entries for @StabilityInferred support.Lines 10 and 17–18 document the same feature—support for reading
@StabilityInferredannotation parameters. This duplication may confuse readers. Additionally, lines 17–18 describe improvements and behavior changes rather than fixes, so they may belong in an "Improved" section if kept separate.Recommendation: Consolidate into a single, comprehensive entry that covers both the capability (what was added) and the behavior (what it enables). Consider whether it belongs in "Added" or "Improved" based on the nature of the change.
Example consolidation:
### Added - New setting: "Show in test source sets" for gutter icons (Issue #21) - Gutter icons are now hidden in test directories by default (can be enabled in settings) -- Support for reading @StabilityInferred annotation parameters for cross-module stability detection (Issue #18) +- Support for reading @StabilityInferred annotation parameters for cross-module stability detection (Issue #18) + - Classes from other modules now correctly marked as UNSTABLE unless annotated with @Stable/@Immutable or @StabilityInferred(parameters=0) ### Fixed - Fixed typealias detection for Composable function types (Issue #16) - Typealiases like `typealias SettingsButtons = @Composable (PlayerUiState) -> Unit` now correctly expand to their underlying function types before stability analysis - Fixed ImmutableList/ImmutableSet/ImmutableMap showing as unstable in test code (Issue #21) - Added fallback type resolution by simple name for immutable collections when FQN resolution fails in test source sets -- Improved cross-module stability detection by reading @StabilityInferred(parameters) annotation (Issue #18) -- Classes from other modules now correctly marked as UNSTABLE unless annotated with @Stable/@Immutable or @StabilityInferred(parameters=0)Also applies to: 17-18
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (1)
600-639: Well-implemented annotation parsing with defensive error handling.The implementation correctly reads the
@StabilityInferredannotation and extracts theparametersvalue. The logic properly handles edge cases (missing annotation, missing argument, wrong type) by returning null, which is the correct fallback behavior.Optional: Address the detekt warning about the swallowed exception.
The try-catch at lines 624-638 intentionally swallows exceptions for defensive behavior, which is appropriate here. However, consider adding a brief comment explaining why exceptions are caught and ignored, to prevent future developers from "fixing" this:
// Extract the Int value from the constant expression return try { + // Defensive: return null if parsing fails due to API changes or unexpected format when (val expression = parametersArgument?.expression) {This will clarify that the exception swallowing is intentional rather than an oversight.
As per static analysis hints.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
compose-stability-analyzer-idea/CHANGELOG.md(1 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt
[warning] 636-636: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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). (7)
- GitHub Check: Compiler Tests (FIR/IR Dumps)
- GitHub Check: Runtime Module Tests
- GitHub Check: Test IntelliJ Plugin
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Build and Tests
- GitHub Check: API check
- GitHub Check: Spotless check
Handle StabilityInferred annotated composables.
Summary by CodeRabbit
New Features
Improvements