Distinguish if a package comes from a foreign module#84
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new app-model module with two data classes (ThirdPartyModel, ThirdPartyModelStable) and wires it into the app. Implements cross-module/project dependency awareness across the Compose Stability Analyzer pipeline (Gradle plugin → compiler plugin → IR gen extension → IR transformer → K2/IDE inferencer) to consider external modules and Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Gradle as Gradle Plugin
participant Compiler as Compiler Plugin
participant IRGen as IR Generation Extension
participant Transformer as IR Transformer
participant Inferencer as K2/IDE Inferencer
Gradle->>Gradle: collectProjectDependencies() → List<String> (CSV)
Gradle->>Compiler: pass OPTION_PROJECT_DEPENDENCIES (CSV)
Compiler->>IRGen: register extension(stabilityOutputDir, projectDependencies)
IRGen->>IRGen: parse CSV → dependencyModules (List)
IRGen->>Transformer: instantiate Transformer(pluginContext, stabilityCollector, projectDependencies)
Transformer->>Inferencer: analyzeTypeStability(type)
Inferencer->>Inferencer: isFromDifferentModule? (origin/module/annotations)
alt External & no stability annotation
Inferencer-->>Transformer: UNSTABLE
else `@StabilityInferred` or local
Inferencer->>Inferencer: continue property-based analysis
Inferencer-->>Transformer: stability result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
app-model/build.gradle.kts (1)
16-23: Plugin configuration may be inconsistent with AndroidManifest.xml.This module uses the
kotlin.jvmplugin, but an AndroidManifest.xml file exists inapp-model/src/main/AndroidManifest.xml. If this is intended as an Android library module, consider switching to the Android library plugin.
🧹 Nitpick comments (4)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt (1)
1037-1049: Fragile heuristic for module detection.The
isFromDifferentModuleimplementation relies on parsingcontainingDeclaration.toString()for keywords like "library", "external", or "compiled". This is fragile because:
toString()output is not part of any stable API contract- The string representation may vary across Kotlin compiler versions
- False positives/negatives are likely for edge cases
Compare with the more robust implementations in:
KtStabilityInferencer.kt(lines 825-852): Usesorigin.toString()with "LIBRARY"/"SOURCE" checks plusProjectFileIndex.getModuleForFileStabilityAnalyzerTransformer.kt(lines 889-914): UsesIrDeclarationOriginenum comparisonConsider aligning this implementation with the K2 approach for consistency, or at minimum document the heuristic nature.
Additionally, the swallowed exception (line 1046) loses diagnostic context. Consider logging at debug level:
} catch (e: Exception) { + // Log at debug level for troubleshooting if needed false }stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt (2)
317-337: Potential Gradle configuration-time resolution issues.Iterating
project.configurationsand accessingdependency.dependencyProjectduringapplyToCompilation(which runs at configuration time) may inadvertently trigger dependency resolution. This can cause:
- Build performance degradation
- Configuration resolution errors in some Gradle setups
- Issues with configuration cache
Consider using lazy evaluation or deferring to execution time:
private fun collectProjectDependencies(project: Project): List<String> { return try { val dependencies = mutableSetOf<String>() - project.configurations.forEach { config -> + // Only check resolvable configurations to avoid resolution issues + project.configurations.filter { it.isCanBeResolved }.forEach { config -> config.dependencies.forEach { dependency ->Also, the swallowed exception (line 334) loses diagnostic context. Consider logging:
} catch (e: Exception) { + project.logger.debug("Failed to collect project dependencies: ${e.message}") emptyList() }
373-388: Optimize source directory traversal.
walkTopDown()traverses the entire source tree before filtering. For large projects, consider limiting the search:private fun findPackageFromSourceDir(dir: java.io.File): String { return try { dir.walkTopDown() + .maxDepth(10) // Limit traversal depth for performance .filter { it.extension == "kt" } .firstOrNull()The swallowed exception here and in
extractPackageName(line 365) could benefit from debug logging for troubleshooting cross-module detection issues.stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (1)
912-913: Exception handling could mask debugging issues.The caught exception is silently swallowed without logging, which could make debugging difficult if the module detection logic fails unexpectedly.
Consider adding minimal logging or documentation:
} catch (e: Exception) { + // Return false (assume same module) if detection fails + // This ensures stability analysis continues even if module detection encounters errors false }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app-model/.gitignore(1 hunks)app-model/api/app-model.api(1 hunks)app-model/build.gradle.kts(1 hunks)app-model/src/main/AndroidManifest.xml(1 hunks)app-model/src/main/kotlin/com/skydoves/myapplication/model/ThirdPartyModel.kt(1 hunks)app/build.gradle.kts(1 hunks)app/src/main/kotlin/com/skydoves/myapplication/MainActivity.kt(3 hunks)compose-stability-analyzer-idea/CHANGELOG.md(1 hunks)compose-stability-analyzer-idea/RELEASE_NOTES.md(0 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(4 hunks)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/StabilityAnalyzerK2.kt(2 hunks)settings.gradle.kts(1 hunks)stability-compiler/api/stability-compiler.api(3 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerCommandLineProcessor.kt(3 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerIrGenerationExtension.kt(2 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerPluginRegistrar.kt(1 hunks)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt(7 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt(4 hunks)
💤 Files with no reviewable changes (1)
- compose-stability-analyzer-idea/RELEASE_NOTES.md
🧰 Additional context used
🧬 Code graph analysis (2)
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 (2)
analyzeValueClass(981-1012)isFromDifferentModule(1037-1049)compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (2)
analyzeValueClass(691-724)isFromDifferentModule(826-853)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (1)
isFromDifferentModule(826-853)stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (1)
isFromDifferentModule(890-915)
🪛 detekt (1.23.8)
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
[warning] 334-334: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 365-365: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 385-385: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt
[warning] 912-912: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt
[warning] 1046-1046: 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). (10)
- GitHub Check: Lint Module Tests
- GitHub Check: Compiler Tests (FIR/IR Dumps)
- GitHub Check: Gradle Plugin Tests
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Compiler Plugin Tests
- GitHub Check: Runtime Module Tests
- GitHub Check: Test IntelliJ Plugin
- GitHub Check: Spotless check
- GitHub Check: API check
- GitHub Check: Build and Tests
🔇 Additional comments (23)
app-model/.gitignore (1)
1-1: LGTM!Standard practice to ignore build artifacts in Gradle modules.
settings.gradle.kts (1)
30-30: LGTM!The app-model module is correctly added to the project includes.
compose-stability-analyzer-idea/CHANGELOG.md (1)
304-304: LGTM!Minor formatting cleanup with no functional impact.
app/build.gradle.kts (1)
62-62: LGTM!The app-model module dependency is correctly declared.
app-model/src/main/kotlin/com/skydoves/myapplication/model/ThirdPartyModel.kt (1)
18-21: Verify the absence of stability annotations is intentional.The
ThirdPartyModellacks@Stableor@Immutableannotations. Given that this PR introduces cross-module stability detection, composables in theappmodule that use this model will be marked as unstable.If this is intentional for testing/demonstration purposes, consider adding a comment explaining the design choice. Otherwise, consider annotating this data class appropriately:
+import androidx.compose.runtime.Immutable + +@Immutable data class ThirdPartyModel( val name: String, val count: Int, )Note: Kotlin
data classwith only primitive immutable properties (String,Int) are inherently stable, but the analyzer requires explicit annotations for cross-module types without@StabilityInferred(parameters=0).stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerPluginRegistrar.kt (2)
43-46: LGTM!Correctly retrieves the project dependencies configuration following the same pattern as
stabilityOutputDir.
55-58: LGTM!The named parameters improve clarity and correctly pass both
stabilityOutputDirandprojectDependenciesto the IR generation extension.app/src/main/kotlin/com/skydoves/myapplication/MainActivity.kt (1)
50-50: LGTM!The import update and Preview annotation change are appropriate for the new cross-module integration with
ThirdPartyModel.Also applies to: 378-379
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/StabilityAnalyzer.kt (1)
574-592: LGTM on cross-module stability logic.The flow correctly:
- First checks for
@Stable/@Immutableannotations (handled earlier in lines 561-573)- Then checks if from different module
- Falls back to
@StabilityInferredcheck for external classes- Returns
UNSTABLEonly if external class lacks any stability annotationThe comment on line 590-591 correctly notes that
@StabilityInferredwould need parameter value inspection for complete analysis.app-model/api/app-model.api (1)
1-12: LGTM!The API surface correctly reflects a Kotlin data class with standard conventions. This provides a good test case for cross-module stability analysis, as
ThirdPartyModelfromapp-modelmodule will be used in theappmodule'sThirdPartyCardcomposable.stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt (1)
155-169: LGTM on plugin option wiring.The project dependencies are correctly collected and passed to the compiler plugin via
SubpluginOption. The comma-separated format aligns with the parsing logic inStabilityAnalyzerIrGenerationExtension.stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerIrGenerationExtension.kt (1)
39-51: LGTM!The parsing of comma-separated
projectDependenciesis correctly implemented with proper handling of:
- Empty strings →
emptyList()- Whitespace trimming
- Empty segment filtering
The named parameters (
pluginContext =,projectDependencies =) improve code clarity.compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/StabilityAnalyzerK2.kt (1)
76-82: Null module handling is properly guarded.
getModuleForFilecan returnnullif the file isn't associated with any module. InKtStabilityInferencer.kt, theusageSiteModuleparameter is declared as nullable (Module? = null) and is protected by an explicit null check before use:if (classFile != null && project != null && usageSiteModule != null). WhenusageSiteModuleis null, the cross-module detection logic is safely skipped. All code paths handle the null case appropriately.stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/StabilityAnalyzerCommandLineProcessor.kt (3)
31-33: LGTM!The new configuration key follows the established pattern and is correctly typed as
Stringto accept comma-separated module names.
56-61: LGTM!The CLI option is well-defined with clear descriptions and appropriate settings. The
required = falsedefault is sensible for backward compatibility.
69-69: LGTM!The option is correctly integrated into the plugin options collection and properly handled in the
processOptionmethod, consistent with the existing options pattern.Also applies to: 88-91
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt (2)
24-24: LGTM!The constructor signature changes support the cross-module detection feature. The
projectDependenciesparameter with an empty default ensures backward compatibility.Also applies to: 44-48
344-346: LGTM!Extracting the
clazzvariable reduces code duplication and improves readability. This is a good refactoring that aligns with the DRY principle.Also applies to: 380-382, 385-386
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt (3)
18-18: LGTM!The constructor changes enable module-aware stability analysis. The
usageSiteModuleparameter with null default maintains backward compatibility while supporting cross-module detection when the context is available.Also applies to: 20-20, 62-65
470-491: LGTM!This cross-module check correctly handles all stability annotations including
@StabilityInferred. The logic properly distinguishes between:
- No annotation → UNSTABLE (classes without explicit stability guarantees)
@StabilityInferred(parameters > 0)→ RUNTIME (unstable parameters require runtime checks)@StabilityInferred(parameters = 0)→ Continue analysis (all parameters stable)This is the correct pattern that should be mirrored in the compiler plugin.
821-853: LGTM!The module detection logic is well-structured with two complementary checks:
- Library detection via origin inspection - catches external JARs/AARs
- Module comparison via
ProjectFileIndex- accurately detects cross-module references within the projectThe module-based approach is more precise than package matching and avoids false positives in nested module structures. The exception handling ensures analysis continues even if detection fails.
stability-compiler/api/stability-compiler.api (2)
95-95: LGTM!The new public API members for project dependencies configuration are correctly exposed in the API file.
Also applies to: 102-102
111-111: Constructor signature changes are breaking changes.The updated constructor signatures for
StabilityAnalyzerIrGenerationExtensionandStabilityAnalyzerTransformerare breaking API changes. The synthetic constructor with defaults (line 183) provides some backward compatibility, but direct constructor calls with positional arguments will break.This is acceptable for a feature addition, but should be noted in release notes or migration guide.
Consider documenting these breaking changes in the PR description or release notes to help users migrate.
Also applies to: 182-183
| <?xml version="1.0" encoding="utf-8"?> | ||
| <!-- | ||
| Designed and developed by 2025 skydoves (Jaewoong Eum) | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| --> | ||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android"> | ||
|
|
||
| </manifest> No newline at end of file |
There was a problem hiding this comment.
Verify Android module configuration consistency.
The app-model module contains an AndroidManifest.xml (indicating it's an Android module), but app-model/build.gradle.kts uses the kotlin.jvm plugin instead of com.android.library. Android modules typically require the Android Gradle plugin to properly process manifest files and Android-specific resources.
If app-model is intended to be an Android library module, consider using:
plugins {
alias(libs.plugins.android.library)
alias(libs.plugins.kotlin.android)
}If it's meant to be a pure Kotlin/JVM module, the AndroidManifest.xml may not be necessary.
🤖 Prompt for AI Agents
In app-model/src/main/AndroidManifest.xml lines 1-19: the module contains an
AndroidManifest.xml but the build script uses the kotlin.jvm plugin, causing a
mismatch; either convert the module to an Android library by replacing the
kotlin.jvm plugin with the Android library and Kotlin Android plugins (ensure
the module uses the Android Gradle plugin alias entries and that
namespace/applicationId and SDK settings are configured in build.gradle.kts), or
if the module is intended as a pure JVM library remove the AndroidManifest.xml
(and any Android resources) and keep kotlin.jvm; update project configuration
and module docs accordingly to keep build plugins and presence of
AndroidManifest.xml consistent.
| @Composable | ||
| fun ThirdPartyCard(thirdPartyModel: ThirdPartyModel) { | ||
| Card { | ||
| Column(modifier = Modifier.padding(16.dp)) { | ||
| Text("Stable User") | ||
| Text("Name: ${thirdPartyModel.name}") | ||
| Text("Age: ${thirdPartyModel.count}") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor UI text inconsistencies.
The hardcoded text "Stable User" and "Age" labels don't match the ThirdPartyModel semantics:
- Line 171: "Stable User" should likely be "Third Party Model" or similar
- Line 173: "Age" should be "Count" to match the property name
This appears to be copy-paste from StableUserCard. Consider updating for clarity:
@Composable
fun ThirdPartyCard(thirdPartyModel: ThirdPartyModel) {
Card {
Column(modifier = Modifier.padding(16.dp)) {
- Text("Stable User")
+ Text("Third Party Model")
Text("Name: ${thirdPartyModel.name}")
- Text("Age: ${thirdPartyModel.count}")
+ Text("Count: ${thirdPartyModel.count}")
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Composable | |
| fun ThirdPartyCard(thirdPartyModel: ThirdPartyModel) { | |
| Card { | |
| Column(modifier = Modifier.padding(16.dp)) { | |
| Text("Stable User") | |
| Text("Name: ${thirdPartyModel.name}") | |
| Text("Age: ${thirdPartyModel.count}") | |
| } | |
| } | |
| } | |
| @Composable | |
| fun ThirdPartyCard(thirdPartyModel: ThirdPartyModel) { | |
| Card { | |
| Column(modifier = Modifier.padding(16.dp)) { | |
| Text("Third Party Model") | |
| Text("Name: ${thirdPartyModel.name}") | |
| Text("Count: ${thirdPartyModel.count}") | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/kotlin/com/skydoves/myapplication/MainActivity.kt around lines
167 to 176, the card UI uses hardcoded labels from StableUserCard; change
"Stable User" to a label that reflects the model (e.g., "Third Party Model") and
change "Age" to "Count" so the displayed labels match ThirdPartyModel.name and
ThirdPartyModel.count; update the two Text strings accordingly to keep UI
semantics consistent.
| // 17. Cross-module types require explicit @Stable/@Immutable/@StabilityInferred | ||
| if (isFromDifferentModule(clazz) && !type.hasStableAnnotation()) { | ||
| return ParameterStability.UNSTABLE | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "KtStabilityInferencer.kt" | head -20Repository: skydoves/compose-stability-analyzer
Length of output: 192
🏁 Script executed:
find . -type f -name "StabilityAnalyzerTransformer.kt" | head -20Repository: skydoves/compose-stability-analyzer
Length of output: 193
🏁 Script executed:
cat -n ./compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/k2/KtStabilityInferencer.kt | sed -n '470,495p'Repository: skydoves/compose-stability-analyzer
Length of output: 1631
🏁 Script executed:
cat -n ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | sed -n '330,470p'Repository: skydoves/compose-stability-analyzer
Length of output: 5786
🏁 Script executed:
# Check what hasStableAnnotation checks for in the compiler plugin
rg -n "fun.*hasStableAnnotation" stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/ -A 5Repository: skydoves/compose-stability-analyzer
Length of output: 1013
🏁 Script executed:
# Check how getStabilityInferredParameters is implemented in compiler plugin
rg -n "fun.*getStabilityInferredParameters" stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/ -A 10Repository: skydoves/compose-stability-analyzer
Length of output: 1739
🏁 Script executed:
# Search for any implementation of getStabilityInferredParameters in the entire codebase
rg -r "getStabilityInferredParameters" . --type kotlinRepository: skydoves/compose-stability-analyzer
Length of output: 50394
🏁 Script executed:
# Search for the definition and usage of getStabilityInferredParameters more precisely
find stability-compiler -name "*.kt" -type f -exec grep -l "getStabilityInferredParameters" {} \;Repository: skydoves/compose-stability-analyzer
Length of output: 191
🏁 Script executed:
# Check StabilityAnalyzerTransformer.kt to see full context of how getStabilityInferredParameters is used
wc -l ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.ktRepository: skydoves/compose-stability-analyzer
Length of output: 197
🏁 Script executed:
cat -n ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | sed -n '585,615p'Repository: skydoves/compose-stability-analyzer
Length of output: 1375
🏁 Script executed:
cat -n ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | sed -n '438,468p'Repository: skydoves/compose-stability-analyzer
Length of output: 1484
🏁 Script executed:
rg -n "StabilityInferred" ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | head -20Repository: skydoves/compose-stability-analyzer
Length of output: 716
🏁 Script executed:
# Check what the hasStableAnnotation check includes - does it check for StabilityInferred?
cat -n ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | sed -n '575,590p'Repository: skydoves/compose-stability-analyzer
Length of output: 747
🏁 Script executed:
# Check if hasStableAnnotation includes checking for StabilityInferred
rg -n "hasStableAnnotation\|hasStabilityInferredAnnotation" ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.ktRepository: skydoves/compose-stability-analyzer
Length of output: 61
🏁 Script executed:
# Final verification - check if there are any other calls to hasStabilityInferredAnnotation
rg -n "hasStabilityInferredAnnotation" ./stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/Repository: skydoves/compose-stability-analyzer
Length of output: 262
@StabilityInferred is not checked for cross-module types at the compiler plugin level, creating inconsistency with IDE plugin behavior.
The cross-module check at line 446 only validates @Stable and @Immutable annotations via type.hasStableAnnotation(), but ignores @StabilityInferred. This differs from the IDE plugin (KtStabilityInferencer.kt:474-491), which properly checks @StabilityInferred parameters for external classes before marking them unstable.
The compiler plugin already has hasStabilityInferredAnnotation() defined (line 582) but never uses it for cross-module validation. Additionally, getStabilityInferredParameters() (line 594) is currently a stub returning null due to the TODO note about deprecated IR APIs.
Minimum fix: Check hasStabilityInferredAnnotation() at line 446 to prevent incorrectly marking cross-module classes with @StabilityInferred as unstable. However, a complete solution requires implementing getStabilityInferredParameters() to handle the parameters=0 case properly, which is currently blocked by deprecated IR API limitations.
| /** | ||
| * Checks if a class is from a different module or external library. | ||
| * Detects: (1) External JARs/AARs via IR origins, (2) Other Gradle modules via package matching | ||
| */ | ||
| private fun isFromDifferentModule(clazz: IrClass): Boolean { | ||
| return try { | ||
| val classFqName = clazz.kotlinFqName.asString() | ||
|
|
||
| // Check 1: External library classes (from compiled JARs/AARs) | ||
| val origin = clazz.origin | ||
| if (origin == IrDeclarationOrigin.IR_EXTERNAL_DECLARATION_STUB || | ||
| origin == IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| // Check 2: Multi-module project dependencies (via package matching) | ||
| if (projectDependencies.isNotEmpty()) { | ||
| for (dependencyModule in projectDependencies) { | ||
| if (classFqName.startsWith("$dependencyModule.")) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| false | ||
| } catch (e: Exception) { | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how projectDependencies is populated in the Gradle plugin
rg -n -C 5 "projectDependencies|OPTION_PROJECT_DEPENDENCIES" stability-gradle/ stability-compiler/Repository: skydoves/compose-stability-analyzer
Length of output: 24416
🏁 Script executed:
#!/bin/bash
# Find the collectProjectDependencies function implementation
rg -n -A 20 "fun collectProjectDependencies" stability-gradle/Repository: skydoves/compose-stability-analyzer
Length of output: 3011
🏁 Script executed:
#!/bin/bash
# Find the extractPackageName function implementation
rg -n -A 15 "fun extractPackageName" stability-gradle/Repository: skydoves/compose-stability-analyzer
Length of output: 2404
🏁 Script executed:
#!/bin/bash
# Check if there's any additional logic around the package matching or if current module is excluded somewhere
rg -n -B 5 -A 10 "isFromDifferentModule" stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt | head -60Repository: skydoves/compose-stability-analyzer
Length of output: 1479
Package-based module detection can cause false positives in nested module structures.
The logic at line 905 uses classFqName.startsWith("$dependencyModule.") where dependencyModule contains package/group identifiers extracted from dependency projects. This creates a risk of incorrectly flagging classes as external.
Example: If a dependency module has group com.example.app and the current module contains classes in com.example.app.feature, those classes would match the startsWith check and be incorrectly marked as external, forcing unnecessary stability annotations or worse, being marked as UNSTABLE.
The issue stems from collectProjectDependencies() (Gradle plugin, line 325) which calls extractPackageName() to populate the list with group identifiers. These are namespace prefixes, not precise module boundaries. Consider whether:
- The check should account for package-level boundaries (e.g., match only when followed by a
.and ensure it's not a prefix collision) - The current module's package should be explicitly excluded from
projectDependencies - Module identifiers should be used instead of package names for more precise detection
🧰 Tools
🪛 detekt (1.23.8)
[warning] 912-912: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
stability-compiler/src/main/kotlin/com/skydoves/compose/stability/compiler/lower/StabilityAnalyzerTransformer.kt
around lines 886 to 915, the package-based module detection using
classFqName.startsWith("$dependencyModule.") can produce false positives for
nested packages; change the check to only treat a class as from the dependency
when the classFqName equals the dependency package or starts with dependency +
'.' (i.e., boundary-aware match), and also exclude the current module's own
package(s) from projectDependencies before testing (or explicitly skip matches
that belong to the current module package). Additionally, consider switching to
module identifiers returned by collectProjectDependencies() instead of raw
group/package names or refine collectProjectDependencies()/extractPackageName()
to supply precise module boundaries so the analyzer can reliably distinguish
true external/module dependencies.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/stability/app.stability(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). (7)
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Runtime Module Tests
- GitHub Check: Compiler Tests (FIR/IR Dumps)
- GitHub Check: Test IntelliJ Plugin
- GitHub Check: Build and Tests
- GitHub Check: Spotless check
- GitHub Check: API check
1 participant
Distinguish if a package comes from a foreign module (#18)
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.