-
-
Notifications
You must be signed in to change notification settings - Fork 24
Fix extracting raw FQ name from the property's getter #69
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 change adds property getter detection logic to the StabilityAnalyzerTransformer. When analyzing function names, the code now identifies getter patterns like "", extracts the actual property name, and corrects the fully-qualified name accordingly before proceeding with annotation and stability analysis. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 (1)
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (1)
64-86: Getter name/FQN normalization looks correct; consider IR-based detection and minor cleanupsThe new logic correctly:
- Detects getter functions via
"<get-"/">"name check.- Extracts the property name with
substring(5, rawFunctionName.length - 1).- Rewrites the FQN by stripping
<get-...>soqualifiedNameandsimpleNamenow reflect the property instead of the accessor, which should fix the original bug for@Composableproperty getters and improve TraceRecomposition tagging.Two optional refinements to consider:
- Prefer IR metadata over string patterns for robustness: e.g.,
declaration.correspondingPropertySymbol?.owner?.name?.asString()(or equivalent in your Kotlin/IR version) to get the property name instead of parsing"<get-...>". That avoids assumptions about synthetic name shapes and should stay stable across compiler changes.- Hoist the regex to a private constant to avoid re-instantiating it for every function:
private val getterNameRegex = Regex("<get-([^>]+)>") ... val propertyFqName = if (rawFqName.contains("<get-")) { rawFqName.replace(getterNameRegex, "$1") } else { rawFqName }You could also slightly simplify the mapping with destructuring:
val (functionName, fqName) = if (rawFunctionName.startsWith("<get-") && rawFunctionName.endsWith(">")) { ... } else { rawFunctionName to rawFqName }Please also verify this behaves as expected for:
- Extension property getters annotated with
@Composable.- Any cases where the compiler might change the synthetic getter naming scheme in future Kotlin versions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt(1 hunks)
Fix extracting raw FQ name from the property's getter. (#67)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.