ADFA-3365: include Kotlin analysis api as dependency#1098
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
f398287 to
4ca1e97
Compare
fixes duplicate class errors for org.antrl.v4.* classes Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
It is now included in the embeddable JAR (named UnsafeAndroid) with proper relocations. Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
651148e to
f1fe62f
Compare
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughRelease NotesNew Features
Dependency Updates
Build Configuration Improvements
Code Cleanup
|
| Cohort / File(s) | Summary |
|---|---|
Kotlin Compiler Configuration Updates annotation-processors/build.gradle.kts, lsp/kotlin-core/build.gradle.kts, composite-builds/build-logic/desugaring/build.gradle.kts, composite-builds/build-logic/plugins/build.gradle.kts |
Updated to new Kotlin DSL using compilerOptions instead of deprecated kotlinOptions, set API/language versions to Kotlin 2.1, and added FIR compiler flag (-Xuse-fir-lt=false). |
Version Upgrades and Gradle Optimization gradle/libs.versions.toml, gradle/wrapper/gradle-wrapper.properties, gradle.properties |
Bumped Kotlin to 2.3.0, KSP to 2.3.6, Room to 2.8.4, Auto-Value to 1.11.0; upgraded Gradle wrapper to 8.14.4; increased build parallelization from 2 to 30 workers, enabled parallel builds and VFS watching, increased heap sizes. |
Build Configuration Simplification git-core/build.gradle.kts, plugin-api/build.gradle.kts, plugin-manager/build.gradle.kts, llama-impl/build.gradle.kts |
Removed redundant Android build settings (compileSdk, defaultConfig.minSdk, compileOptions, kotlinOptions) while retaining essential namespace/lint configurations; reformatted indentation. |
New Kotlin Analysis API Module subprojects/kotlin-analysis-api/build.gradle.kts, subprojects/kotlin-analysis-api/.gitignore, subprojects/kotlin-analysis-api/proguard-rules.pro |
Added new Android library module with external asset configuration to download standalone embeddable Kotlin analysis API JAR from GitHub releases via SHA-256 pinned dependency. |
Project Structure and Dependencies settings.gradle.kts, app/build.gradle.kts, lsp/kotlin/build.gradle.kts |
Registered new :subprojects:kotlin-analysis-api module, normalized indentation; added kotlin-analysis-api dependency to app and lsp modules; updated app packaging rules with pickFirsts for kotlin\_builtins resources and excluded kotlin-android-extensions-runtime. |
Desugaring and Service Registration composite-builds/build-deps/java-compiler/src/main/java/javac/internal/jrtfs/JrtFileSystemProvider.java, composite-builds/build-deps/java-compiler/build.gradle.kts, composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt |
Added @AutoService annotation and dependencies for automatic FileSystemProvider registration; refactored DesugarClassVisitor to rewrite method/field descriptors and signatures before visitor wrapping, conditionally apply ClassRefReplacingMethodVisitor, and introduce lazy-computed slashClassReplacements map with helper functions. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- ADFA-3365: add ability to re-write class name references in desugar plugin #1097: Adds descriptor/signature rewriting to
DesugarClassVisitorwithvisitField, lazyslashClassReplacements, and conditionalClassRefReplacingMethodVisitorwrapping—the same core desugaring feature implemented in this PR. - ADFA-2878: Create git module #962: Modifies git-core module build files and structure, directly related to the git-core/build.gradle.kts simplification in this change.
Suggested reviewers
- Daniel-ADFA
- jatezzz
- jomen-adfa
Poem
🐰 The build grows swift with Kotlin bright,
Gradle 8.14, a pleasant sight,
New analysis-api joins the fold,
Descriptors rewritten, signatures retold,
Parallelization makes the tests take flight! ⚡
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: adding the Kotlin analysis API as a dependency to the project. |
| Description check | ✅ Passed | The description references the related Jira ticket and notes the dependency ordering requirement, which is relevant to the changeset involving dependency additions. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/ADFA-3365-include-analysis-api-as-dependency
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt (1)
63-65: Replace deprecatedvalues()withentriesproperty.The
values()method is deprecated in Kotlin 1.9+. Since the project uses Kotlin 2.3.0, use theentriesproperty instead, which returns aListwith better type safety than the deprecatedArray-based approach.Suggested change
fun find(insn: String): MethodOpcode? { - return MethodOpcode.values().find { it.insnName == insn } + return MethodOpcode.entries.find { it.insnName == insn } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt` around lines 63 - 65, Replace the deprecated MethodOpcode.values() usage in the MethodOpcode.find function with the Kotlin 1.9+ entries property: update the find implementation to search MethodOpcode.entries (not values()) for the matching insnName so it returns the correct MethodOpcode? while avoiding the deprecated Array-based call; reference the MethodOpcode.find function and the MethodOpcode.entries property when making the change.llama-impl/build.gradle.kts (1)
22-25: Remove no-op/empty CMake flag mutations.These lines add no value and
cppFlags("")may push an empty flag into native compilation.♻️ Proposed cleanup
cmake { arguments += "-DLLAMA_CURL=OFF" arguments += "-DLLAMA_BUILD_COMMON=ON" arguments += "-DGGML_LLAMAFILE=OFF" arguments += "-DCMAKE_BUILD_TYPE=Release" - cppFlags += listOf() - arguments += listOf() - - cppFlags("") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@llama-impl/build.gradle.kts` around lines 22 - 25, The diff shows no-op mutations to the CMake/Ninja flags—remove the empty list appends to cppFlags and arguments and delete the cppFlags("") call so you don't inject an empty flag into native compilation; specifically, remove the lines that do "cppFlags += listOf()", "arguments += listOf()", and the "cppFlags(\"\")" invocation (references: cppFlags and arguments) so only intentional flags are set.composite-builds/build-deps/java-compiler/build.gradle.kts (1)
28-29: UsecompileOnlyfor AutoService annotations.The annotations are compile-time metadata; using
compileOnlyavoids unnecessary runtime classpath surface.♻️ Proposed dependency-scope adjustment
dependencies { annotationProcessor(libs.google.auto.service) - implementation(libs.google.auto.service.annotations) + compileOnly(libs.google.auto.service.annotations) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-deps/java-compiler/build.gradle.kts` around lines 28 - 29, The AutoService annotation dependency is declared as implementation but should be compile-only; keep annotationProcessor(libs.google.auto.service) as-is and replace implementation(libs.google.auto.service.annotations) with compileOnly(libs.google.auto.service.annotations) so the annotations are available at compile time but not added to the runtime classpath (update the declaration in build.gradle.kts near the annotationProcessor line).plugin-api/build.gradle.kts (1)
22-22: Avoid hardcoding the plugin API jar version in task output.Deriving the filename from
project.versionprevents stale artifact names during releases.♻️ Proposed tweak
- rename { "plugin-api-1.0.0.jar" } + rename { "plugin-api-${project.version}.jar" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-api/build.gradle.kts` at line 22, The rename closure currently hardcodes "plugin-api-1.0.0.jar"; update it to derive the filename from the project version so releases don’t produce stale names. Locate the rename { "plugin-api-1.0.0.jar" } usage in the build script (the rename closure inside the Copy/task that produces the plugin API artifact) and replace the literal with a templated value using the project.version property (e.g., build the filename from "plugin-api" + project.version) so the produced jar name follows the current project.version.composite-builds/build-logic/desugaring/build.gradle.kts (1)
46-50: Confirm intentional Kotlin 2.1 language/API pinning.The desugaring build pins
apiVersionandlanguageVersiontoKOTLIN_2_1, which is older than the catalog version (2.3.0). Verify this is deliberate or consider updating to the catalog version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/build.gradle.kts` around lines 46 - 50, The build is pinning Kotlin API and language to KotlinVersion.KOTLIN_2_1 inside tasks.withType<KotlinCompile> -> compilerOptions (apiVersion and languageVersion), which is older than the project catalog (2.3.0); either confirm this pinning is intentional or update these settings to match the catalog (e.g., Kotlin 2.3) or remove the overrides so the project-wide/catalog version is used—locate the tasks.withType<KotlinCompile> block and change apiVersion.languageVersion from KotlinVersion.KOTLIN_2_1 to the catalog version (or delete the explicit settings) and run a quick build to verify no regressions.composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt (3)
60-62: ANEWARRAY with multi-dimensional arrays may not be fully handled.When
ANEWARRAYis used to create multi-dimensional arrays (e.g.,String[][]), thetypeparameter is an array descriptor like[Ljava/lang/String;, not a bare internal name. Thereplace()function does exact map lookup, so the embedded class name won't be matched.Consider using
replaceInDescriptor()for array descriptors:♻️ Proposed fix
override fun visitTypeInsn(opcode: Int, type: String) { - super.visitTypeInsn(opcode, replace(type)) + // ANEWARRAY can have array descriptors for multi-dimensional arrays + val replaced = if (type.startsWith("[")) { + replaceInDescriptor(type) + } else { + replace(type) + } + super.visitTypeInsn(opcode, replaced) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt` around lines 60 - 62, visitTypeInsn in ClassRefReplacingMethodVisitor currently calls replace(type) which fails for array descriptors used with ANEWARRAY (e.g., "[Ljava/lang/String;"); change it to use replaceInDescriptor(type) (or detect array descriptors and run the descriptor-aware replacement) and pass the resulting descriptor to super.visitTypeInsn so embedded class names in multi-dimensional array descriptors are correctly replaced.
64-74: Array class literals (Type.ARRAY) are not replaced.The current implementation only handles
Type.OBJECTclass literals. Array class literals likeString[].classhavesort == Type.ARRAYand will pass through unchanged.♻️ Proposed enhancement for array class literals
override fun visitLdcInsn(value: Any?) { // Replace class-literal constants: Foo.class → Bar.class - if (value is Type && value.sort == Type.OBJECT) { - val replaced = replace(value.internalName) - if (replaced !== value.internalName) { - super.visitLdcInsn(Type.getObjectType(replaced)) + if (value is Type) { + if (value.sort == Type.OBJECT) { + val replaced = replace(value.internalName) + if (replaced !== value.internalName) { + super.visitLdcInsn(Type.getObjectType(replaced)) + return + } + } else if (value.sort == Type.ARRAY) { + val replaced = replaceInDescriptor(value.descriptor) + if (replaced !== value.descriptor) { + super.visitLdcInsn(Type.getType(replaced)) + return + } - return } } super.visitLdcInsn(value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt` around lines 64 - 74, The visitLdcInsn in ClassRefReplacingMethodVisitor only replaces Type.OBJECT class literals; extend it to handle Type.ARRAY by extracting the array's element Type (use Type.getElementType or type.elementType), check if that element is an OBJECT, run replace(...) on element.internalName, and if changed reconstruct a new array Type with the same dimensions but the replaced element descriptor (e.g. build the descriptor with the correct number of '[' prefixes plus the element descriptor) and call super.visitLdcInsn with that new Type; keep the existing Type.OBJECT branch for non-array objects and fall back to super.visitLdcInsn(value) when nothing changed.
24-28: Verify if desugaring targets requirevisitFrameandvisitInvokeDynamicInsncoverage.The current implementation documents a specific scope of covered visit sites (lines 10-17). If your desugaring pipeline processes classes using invokedynamic instructions (lambdas, method references, string concatenation) or relies on JVM frame verification, consider adding:
visitFrame— Stack map frames contain type information that the JVM verifier checks. If frames reference old class names after transformation, class loading may fail withVerifyError.
visitInvokeDynamicInsn— The descriptor and bootstrap method arguments can contain class references that need updating during desugaring.If these are not encountered in your use case, the current scope is sufficient.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt` around lines 24 - 28, The visitor ClassRefReplacingMethodVisitor must also override visitFrame and visitInvokeDynamicInsn to update class name references during desugaring: add an override for visitFrame that walks all frame type entries (local and stack) and replaces any internal name strings using the classReplacements map before delegating to super.visitFrame, and add an override for visitInvokeDynamicInsn that updates the method descriptor and each bootstrap argument (especially Handle and String or Type entries) by replacing referenced internal names via classReplacements, then calls super.visitInvokeDynamicInsn with the updated descriptor and bootstrapArgs; ensure you preserve existing behavior for null/expanded frames and call through to the existing MethodVisitor (mv) after substitutions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt`:
- Around line 104-105: The replaceInSignature logic in DesugarClassVisitor
currently forwards to replaceInDescriptor which uses a regex and misses nested
generic type parameters; update replaceInSignature(signature: String?) to parse
and rewrite Java generic signatures using ASM's SignatureReader and
SignatureWriter: use SignatureReader(signature) and a custom SignatureVisitor
that delegates to a SignatureWriter while calling your existing type-replacement
routine (the logic in replaceInDescriptor) whenever visiting a type element,
then return the SignatureWriter.toString() (preserving null handling). Ensure
you update any callers of replaceInSignature to use the rewritten signature and
keep method name replaceInDescriptor for descriptor-only replacements.
- Around line 62-76: The method visitMethod currently rewrites descriptor and
signature but forwards the exceptions array unchanged; update it to rewrite
declared exception internal names as well by mapping exceptions through the same
replacement logic (e.g., create or reuse a helper like replaceInternalName(e:
String): String that applies the same class-name substitutions used by
replaceInDescriptor/replaceInSignature) and pass the transformed exceptions
array to super.visitMethod instead of the original exceptions. Ensure you call
this transformation on every non-null entry and preserve null-safety when
passing the resulting Array<String>? into the super.visitMethod invocation.
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.kt`:
- Around line 97-103: Validate and canonicalize class mappings in replaceClass:
reject descriptor-style or array inputs (strings starting with '[' or starting
with 'L' and ending with ';'), and reject primitive names (int, boolean, byte,
short, long, char, float, double, void) before doing the '/'→'.' conversion;
then detect cycles/overlaps by resolving the transitive target for the given
fromClass against the existing classReplacements map (follow mapping chains
classReplacements[src] until a stable non-mapped target or a cycle is found),
and if a cycle is detected or the resolved target would map back to any existing
source, throw an IllegalArgumentException; otherwise canonicalize by storing the
fully-resolved target in classReplacements[from] (use the normalized dotted
form) so all mappings are single-step and transitive lookups are consistent.
Also apply the same validation/canonicalization logic to the other analogous
overload/function at lines 110-117.
In `@settings.gradle.kts`:
- Around line 189-194: The project module list contains duplicate entries for
the modules ":llama-api" and ":llama-impl" in the settings.gradle.kts module
list; remove the duplicated occurrences so each module appears only once (keep a
single ":llama-api" and a single ":llama-impl" entry and remove the extra lines
near the second group), ensuring the resulting list still includes
":cv-image-to-xml" and ":compose-preview" exactly once.
In `@subprojects/kotlin-analysis-api/build.gradle.kts`:
- Around line 13-26: Add an explicit jarName to the external asset configuration
so the downloaded artifact's actual filename is preserved: inside the
jarDependency("kt-android") block in the externalAssets section, set jarName =
ktAndroidJarName (using the existing ktAndroidJarName variable) alongside the
existing configuration/source so the dependency uses the correct artifact name.
---
Nitpick comments:
In `@composite-builds/build-deps/java-compiler/build.gradle.kts`:
- Around line 28-29: The AutoService annotation dependency is declared as
implementation but should be compile-only; keep
annotationProcessor(libs.google.auto.service) as-is and replace
implementation(libs.google.auto.service.annotations) with
compileOnly(libs.google.auto.service.annotations) so the annotations are
available at compile time but not added to the runtime classpath (update the
declaration in build.gradle.kts near the annotationProcessor line).
In `@composite-builds/build-logic/desugaring/build.gradle.kts`:
- Around line 46-50: The build is pinning Kotlin API and language to
KotlinVersion.KOTLIN_2_1 inside tasks.withType<KotlinCompile> -> compilerOptions
(apiVersion and languageVersion), which is older than the project catalog
(2.3.0); either confirm this pinning is intentional or update these settings to
match the catalog (e.g., Kotlin 2.3) or remove the overrides so the
project-wide/catalog version is used—locate the tasks.withType<KotlinCompile>
block and change apiVersion.languageVersion from KotlinVersion.KOTLIN_2_1 to the
catalog version (or delete the explicit settings) and run a quick build to
verify no regressions.
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt`:
- Around line 60-62: visitTypeInsn in ClassRefReplacingMethodVisitor currently
calls replace(type) which fails for array descriptors used with ANEWARRAY (e.g.,
"[Ljava/lang/String;"); change it to use replaceInDescriptor(type) (or detect
array descriptors and run the descriptor-aware replacement) and pass the
resulting descriptor to super.visitTypeInsn so embedded class names in
multi-dimensional array descriptors are correctly replaced.
- Around line 64-74: The visitLdcInsn in ClassRefReplacingMethodVisitor only
replaces Type.OBJECT class literals; extend it to handle Type.ARRAY by
extracting the array's element Type (use Type.getElementType or
type.elementType), check if that element is an OBJECT, run replace(...) on
element.internalName, and if changed reconstruct a new array Type with the same
dimensions but the replaced element descriptor (e.g. build the descriptor with
the correct number of '[' prefixes plus the element descriptor) and call
super.visitLdcInsn with that new Type; keep the existing Type.OBJECT branch for
non-array objects and fall back to super.visitLdcInsn(value) when nothing
changed.
- Around line 24-28: The visitor ClassRefReplacingMethodVisitor must also
override visitFrame and visitInvokeDynamicInsn to update class name references
during desugaring: add an override for visitFrame that walks all frame type
entries (local and stack) and replaces any internal name strings using the
classReplacements map before delegating to super.visitFrame, and add an override
for visitInvokeDynamicInsn that updates the method descriptor and each bootstrap
argument (especially Handle and String or Type entries) by replacing referenced
internal names via classReplacements, then calls super.visitInvokeDynamicInsn
with the updated descriptor and bootstrapArgs; ensure you preserve existing
behavior for null/expanded frames and call through to the existing MethodVisitor
(mv) after substitutions.
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt`:
- Around line 63-65: Replace the deprecated MethodOpcode.values() usage in the
MethodOpcode.find function with the Kotlin 1.9+ entries property: update the
find implementation to search MethodOpcode.entries (not values()) for the
matching insnName so it returns the correct MethodOpcode? while avoiding the
deprecated Array-based call; reference the MethodOpcode.find function and the
MethodOpcode.entries property when making the change.
In `@llama-impl/build.gradle.kts`:
- Around line 22-25: The diff shows no-op mutations to the CMake/Ninja
flags—remove the empty list appends to cppFlags and arguments and delete the
cppFlags("") call so you don't inject an empty flag into native compilation;
specifically, remove the lines that do "cppFlags += listOf()", "arguments +=
listOf()", and the "cppFlags(\"\")" invocation (references: cppFlags and
arguments) so only intentional flags are set.
In `@plugin-api/build.gradle.kts`:
- Line 22: The rename closure currently hardcodes "plugin-api-1.0.0.jar"; update
it to derive the filename from the project version so releases don’t produce
stale names. Locate the rename { "plugin-api-1.0.0.jar" } usage in the build
script (the rename closure inside the Copy/task that produces the plugin API
artifact) and replace the literal with a templated value using the
project.version property (e.g., build the filename from "plugin-api" +
project.version) so the produced jar name follows the current project.version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16c71b19-f978-4dc3-9ec9-5166432f54d6
📒 Files selected for processing (29)
annotation-processors/build.gradle.ktsapp/build.gradle.ktscomposite-builds/build-deps/java-compiler/build.gradle.ktscomposite-builds/build-deps/java-compiler/src/main/java/javac/internal/jrtfs/JrtFileSystemProvider.javacomposite-builds/build-logic/desugaring/build.gradle.ktscomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitorFactory.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarParams.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsn.ktcomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsnKey.ktcomposite-builds/build-logic/plugins/build.gradle.ktsgit-core/build.gradle.ktsgradle.propertiesgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiesllama-impl/build.gradle.ktslsp/kotlin-core/build.gradle.ktslsp/kotlin/build.gradle.ktsplugin-api/build.gradle.ktsplugin-manager/build.gradle.ktssettings.gradle.ktssubprojects/kotlin-analysis-api/.gitignoresubprojects/kotlin-analysis-api/build.gradle.ktssubprojects/kotlin-analysis-api/consumer-rules.prosubprojects/kotlin-analysis-api/proguard-rules.pro
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt (1)
63-77:⚠️ Potential issue | 🟠 MajorRewrite declared
throwstypes when applying class replacements.This is still unresolved from prior feedback: Line 76 forwards
exceptionsunchanged. If a replaced type appears in declared exceptions, method metadata keeps the old internal name.Patch sketch
override fun visitMethod( access: Int, name: String?, descriptor: String?, signature: String?, exceptions: Array<out String>?, ): MethodVisitor { + val rewrittenExceptions = + exceptions?.map { slashClassReplacements[it] ?: it }?.toTypedArray() + // Rewrite the method's own descriptor/signature at the class-structure level. val base = super.visitMethod( access, name, descriptor?.let { replaceInDescriptor(it) }, replaceInSignature(signature), - exceptions, + rewrittenExceptions, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt` around lines 63 - 77, The visitMethod implementation currently rewrites the method descriptor/signature but leaves the declared throws (exceptions) array unchanged; update visitMethod so it maps the exceptions parameter through the same class-replacement logic before calling super.visitMethod (e.g., replace each entry in exceptions via the replacement helper used for internal names), e.g. transform exceptions with the appropriate replace function and pass the resulting Array<String>? to super.visitMethod instead of the original exceptions array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt`:
- Line 20: The unresolved import for DesugarParams in DesugarClassVisitor.kt
means the import is unqualified; fix it by replacing the bare import with the
correct fully-qualified type import (or remove the import if DesugarParams is
defined in the same package as DesugarClassVisitor). Locate the DesugarParams
declaration and update the import statement in DesugarClassVisitor.kt to
reference that package (e.g., import <actual.package>.DesugarParams) so the
compiler can resolve the symbol.
---
Duplicate comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt`:
- Around line 63-77: The visitMethod implementation currently rewrites the
method descriptor/signature but leaves the declared throws (exceptions) array
unchanged; update visitMethod so it maps the exceptions parameter through the
same class-replacement logic before calling super.visitMethod (e.g., replace
each entry in exceptions via the replacement helper used for internal names),
e.g. transform exceptions with the appropriate replace function and pass the
resulting Array<String>? to super.visitMethod instead of the original exceptions
array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5e0f253-f131-45f1-9fc0-ec9ab8057090
📒 Files selected for processing (4)
app/build.gradle.ktscomposite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.ktgit-core/build.gradle.ktsgradle/libs.versions.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- gradle/libs.versions.toml
- app/build.gradle.kts
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt (1)
67-76:⚠️ Potential issue | 🟠 MajorRewrite declared
exceptionsinvisitMethodas well.Line 75 still forwards
exceptionsunchanged. If a replaced class appears in declared throws, method metadata keeps stale internal names even when descriptors/signatures are rewritten.Patch suggestion
override fun visitMethod( access: Int, name: String?, descriptor: String?, signature: String?, exceptions: Array<out String>?, ): MethodVisitor { + val rewrittenExceptions = + exceptions?.map { slashClassReplacements[it] ?: it }?.toTypedArray() + // Rewrite the method's own descriptor/signature at the class-structure level. val base = super.visitMethod( access, name, descriptor?.let { replaceInDescriptor(it) }, replaceInSignature(signature), - exceptions, + rewrittenExceptions, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt` around lines 67 - 76, visitMethod currently rewrites the method descriptor and signature but forwards the declared exceptions array unchanged, leaving stale internal names in throws metadata; update the call in DesugarClassVisitor.visitMethod to map/transform the exceptions parameter using the same replacement logic (e.g., call replaceInInternalName or the existing replacement helper) before passing it to super.visitMethod so declared throws use replaced/internal names alongside replaceInDescriptor and replaceInSignature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt`:
- Around line 67-76: visitMethod currently rewrites the method descriptor and
signature but forwards the declared exceptions array unchanged, leaving stale
internal names in throws metadata; update the call in
DesugarClassVisitor.visitMethod to map/transform the exceptions parameter using
the same replacement logic (e.g., call replaceInInternalName or the existing
replacement helper) before passing it to super.visitMethod so declared throws
use replaced/internal names alongside replaceInDescriptor and
replaceInSignature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20510c13-52f8-42e7-b5eb-9536e9d0c2a5
📒 Files selected for processing (1)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt
See ADFA-3365 for more details.
Must be merged after #1097