Introduce KotlinCompilerPluginSupportPlugin and adjust StabilityInferred annotation check #4
Conversation
…red annotation check
WalkthroughThis pull request restructures stability analysis workflows, migrates the Gradle plugin to leverage Kotlin compiler plugin infrastructure, filters compiler-generated anonymous composables from exports, expands known-stable shape types, and updates function signatures for composable content parameters—shifting from composable-typed to explicit function types with adjusted nullability. Changes
Sequence DiagramsequenceDiagram
participant Old as Old Flow
participant New as New Flow
rect rgba(200, 220, 255, 0.2)
Note over Old: Previous Approach
Old->>Old: Check @StabilityInferred (early)
Old->>Old: Return RUNTIME if present
Old->>Old: Check other properties
end
rect rgba(200, 255, 220, 0.2)
Note over New: New Approach
New->>New: Analyze class properties first
alt Property analysis conclusive
New->>New: Return STABLE or UNSTABLE
else Property analysis inconclusive
New->>New: Check @StabilityInferred parameters
alt Parameters == 0
New->>New: Return STABLE
else Parameters > 0
New->>New: Return RUNTIME
end
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
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 |
skydoves
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
settings.gradle.kts (1)
3-7: Duplicate mavenLocal() in pluginManagement repositories.The
mavenLocal()repository is declared twice in thepluginManagement.repositoriesblock (lines 3 and 7). Remove the duplicate entry to avoid redundancy.Apply this diff to remove the duplicate:
pluginManagement { repositories { mavenLocal() gradlePluginPortal() google() mavenCentral() - mavenLocal() } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/stability/app.stability(4 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalysisConstants.kt(1 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt(2 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt(3 hunks)gradle.properties(1 hunks)gradle/libs.versions.toml(1 hunks)settings.gradle.kts(2 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityInfoCollector.kt(3 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt(3 hunks)stability-gradle/api/stability-gradle.api(1 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt(2 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityCheckTask.kt(1 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityDumpTask.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt (1)
analyzeClassProperties(886-944)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (1)
analyzeClassProperties(345-372)
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt (1)
analyzeClassProperties(886-944)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (1)
analyzeClassProperties(322-405)
⏰ 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). (1)
- GitHub Check: Build and Tests
🔇 Additional comments (14)
app/stability/app.stability (4)
16-27: Signature update reflects Compose IR types correctly.The nullable wrappers and the explicit
Function2/Function3encodings line up with the transformed Compose signatures, and the stability flags still look accurate for the analyzer output. Nicely done.
65-74: Icon export stays consistent with stability rules.The tighter
List<StableUser>typing and nullable elevation mirror the source API while keeping the correct runtime-versus-stable markings.
89-99: Nullable modifier keeps the right stability classification.
Modifier?remains treated as stable, so this export matches Compose expectations.
200-206: Function2 encoding forcontentlooks spot on.The analyzer now emits the lowered signature while keeping the stable classification, which is what we want.
gradle.properties (1)
54-54: LGTM!Version bump to 0.4.1 is consistent with the PR objectives and other version changes across the codebase.
gradle/libs.versions.toml (1)
15-15: LGTM!Version bump to 0.4.1 is consistent with the version changes in gradle.properties and the PR objectives.
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityCheckTask.kt (1)
140-143: LGTM!The anonymous composable filtering logic is correctly implemented and aligns with the same filtering approach used in
StabilityDumpTask.ktandStabilityInfoCollector.kt.stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityDumpTask.kt (1)
256-260: LGTM!The anonymous composable filtering is correctly placed before the
ignoredPackagesandignoredClassesfilters, ensuring compiler-generated entries are excluded early in the pipeline. This aligns with the filtering approach inStabilityCheckTask.ktandStabilityInfoCollector.kt.stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityInfoCollector.kt (2)
40-50: LGTM!The filtering of anonymous composables from the JSON export is correctly implemented and aligns with the filtering approach in the Gradle tasks. The emptiness check now operates on the filtered list, which is the correct behavior.
58-91: LGTM!All references to the composables collection have been correctly updated to use
filteredComposables, ensuring anonymous entries are excluded from the JSON output.stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (3)
313-339: LGTM - Two-phase stability analysis correctly implemented.The refactored logic correctly implements the two-phase approach:
- First, analyze class properties to determine if the class is definitively STABLE or UNSTABLE
- Only if properties yield RUNTIME, then check
@StabilityInferredparametersThis aligns with the updated flow in the K2 implementation shown in the relevant code snippets.
407-413: Verify the impact of the placeholder implementation.The
getStabilityInferredParameters()helper currently returnsnull, which means all classes with runtime-dependent property stability will be treated as RUNTIME regardless of their@StabilityInferredannotation. This is conservative but may lead to more RUNTIME classifications than necessary.Please verify that this placeholder implementation is acceptable for the 0.4.1 release, or if a working implementation should be prioritized. The TODO indicates this is intentional, but it's worth confirming the impact on stability analysis accuracy.
568-576: Fix package name typos for Compose Foundation shapes.Lines 571-574 contain typos:
androidxxshould beandroidx. These typos will prevent the stability analyzer from recognizing these types as stable. This is the same issue as inStabilityAnalysisConstants.kt.Apply this diff to fix the typos:
// Compose Foundation shapes "androidx.compose.foundation.shape.RoundedCornerShape", "androidx.compose.foundation.shape.CircleShape", - "androidxx.compose.foundation.shape.CutCornerShape", - "androidxx.compose.foundation.shape.CornerBasedShape", - "androidxx.compose.foundation.shape.AbsoluteRoundedCornerShape", - "androidxx.compose.foundation.shape.AbsoluteCutCornerShape", + "androidx.compose.foundation.shape.CutCornerShape", + "androidx.compose.foundation.shape.CornerBasedShape", + "androidx.compose.foundation.shape.AbsoluteRoundedCornerShape", + "androidx.compose.foundation.shape.AbsoluteCutCornerShape", "androidx.compose.ui.graphics.RectangleShape",Likely an incorrect or invalid review comment.
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalysisConstants.kt (1)
87-95: Fix package name typos for Compose Foundation shapes.Lines 90-93 contain typos:
androidxxshould beandroidx. These typos will prevent the stability analyzer from recognizing these types as stable.Apply this diff to fix the typos:
// Compose Foundation shapes "androidx.compose.foundation.shape.RoundedCornerShape", "androidx.compose.foundation.shape.CircleShape", - "androidxx.compose.foundation.shape.CutCornerShape", - "androidxx.compose.foundation.shape.CornerBasedShape", - "androidxx.compose.foundation.shape.AbsoluteRoundedCornerShape", - "androidxx.compose.foundation.shape.AbsoluteCutCornerShape", + "androidx.compose.foundation.shape.CutCornerShape", + "androidx.compose.foundation.shape.CornerBasedShape", + "androidx.compose.foundation.shape.AbsoluteRoundedCornerShape", + "androidx.compose.foundation.shape.AbsoluteCutCornerShape", "androidx.compose.ui.graphics.RectangleShape",Likely an incorrect or invalid review comment.
...radle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt (1)
57-61: Extension construction is missing required services
StabilityAnalyzerExtensionstill exposes a constructor(ProjectLayout, ObjectFactory, ProviderFactory). Callingextensions.createwith onlytarget.layoutmeans Gradle will try to invoke a non-existent(ProjectLayout)ctor and will throwNoSuchMethodExceptionwhen the plugin is applied. Please wire the missing services:val extension = target.extensions.create( "composeStabilityAnalyzer", StabilityAnalyzerExtension::class.java, - target.layout, + target.layout, + target.objects, + target.providers, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
settings.gradle.kts(1 hunks)stability-compiler/build.gradle.kts(1 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt(2 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: Test IntelliJ Plugin
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Build and Tests
- GitHub Check: API check
- GitHub Check: Spotless check
🔇 Additional comments (2)
settings.gradle.kts (1)
1-30: AI summary inconsistency detected.The AI-generated summary states: "Added mavenLocal() to... Removed the trailing mavenLocal() entries from both...". However, the provided code (final state) shows
mavenLocal()as added (marked with~) at lines 3 and 13, with no evidence of removals. This discrepancy suggests either the summary is outdated or the code state does not reflect what the summary describes.stability-compiler/build.gradle.kts (1)
32-32: No breaking API changes—dependency change is correct.The concern about public API exposure is invalid in this context. While
RecompositionIrBuilderis marked public, it and all other public classes in stability-compiler are compiler plugin infrastructure (extending or implementing Kotlin compiler plugin interfaces likeCompilerPluginRegistrar,FirExtensionRegistrar,IrGenerationExtension). These classes are not consumer-facing APIs—they are discovered and instantiated by the Kotlin compiler framework via reflection.RecompositionIrBuilderis only used internally byStabilityAnalyzerTransformerand is not intended for direct consumer usage.Changing the dependency scope from
apitoimplementationis correct and safe for this use case, as it accurately reflects the internal-only nature of the stability-runtime dependency.
Introduce KotlinCompilerPluginSupportPlugin and adjust StabilityInferred annotation check
Summary by CodeRabbit
Version 0.4.1 Release Notes
New Features
Bug Fixes
Improvements