Skip to content

ADFA-3365: add ability to re-write class name references in desugar plugin#1097

Open
itsaky-adfa wants to merge 2 commits intostagefrom
fix/ADFA-3365-allow-class-ref-desugaring
Open

ADFA-3365: add ability to re-write class name references in desugar plugin#1097
itsaky-adfa wants to merge 2 commits intostagefrom
fix/ADFA-3365-allow-class-ref-desugaring

Conversation

@itsaky-adfa
Copy link
Contributor

@itsaky-adfa itsaky-adfa commented Mar 19, 2026

See ADFA-3365 for more details.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5eaf5d95-873e-4907-8db9-f21b441243ee

📥 Commits

Reviewing files that changed from the base of the PR and between 1b069cb and 4af1c30.

📒 Files selected for processing (9)
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitorFactory.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarParams.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsn.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsnKey.kt
✅ Files skipped from review due to trivial changes (4)
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsn.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsnKey.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitorFactory.kt
🚧 Files skipped from review as they are similar to previous changes (4)
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarParams.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.kt

📝 Walkthrough

Release Notes: Class Reference Rewriting in Desugar Plugin

Features

  • New class reference rewriting capability: Introduced ClassRefReplacingMethodVisitor to rewrite bytecode references to specified classes within method bodies

    • Supports rewriting across multiple bytecode instruction types:
      • Method invocation owners and descriptors
      • Field access owners and descriptors
      • Type instructions and operands
      • Class literals loaded via LDC instructions
      • Local variable descriptors and generic signatures
      • Multi-dimensional array descriptors
      • Exception handler type definitions
  • DSL enhancements: Extended desugaring replacement configuration with new class-level replacement methods

    • replaceClass(fromClass: String, toClass: String) - dot-notation class names
    • replaceClass(fromClass: Class<*>, toClass: Class<*>) - Java Class objects
  • New configuration type: ReplaceClassRef data class for representing class replacement mappings with automatic conversion between dot and slash notation

  • Enhanced DesugarParams interface: Added classReplacements map property for configurable class-to-class mappings

  • Layered bytecode transformation: Proper ordering of transformations where class reference replacement is applied before method call replacement

Risks & Considerations

  • ⚠️ Bytecode manipulation complexity: The solution modifies multiple bytecode instruction types (descriptors, signatures, type operands). Any errors in descriptor/signature manipulation could cause class loading failures or runtime errors. Comprehensive bytecode verification testing is critical.

  • ⚠️ Descriptor and signature string replacement: The implementation uses string-based replacement of L<className>; patterns in descriptors and signatures. Edge cases in descriptor parsing (nested generic types, array dimensions) should be thoroughly tested.

  • ⚠️ Transformation layering: Class reference replacement is layered before method call replacement. The order is significant and requires validation that both layers interact correctly without duplicate replacements or missed edge cases.

  • Input validation scope: While the DSL validates blank inputs and rejects primitive/array classes, ensure all downstream code properly handles invalid or malformed class name mappings.

  • Precedence semantics: When both class-level and method-level replacements target the same instruction, method-level takes precedence. This behavior should be clearly documented and tested.

  • No test files detected: The changes appear to lack corresponding test coverage visible in the repository structure. New tests should verify descriptor/signature correctness and transformation layer interactions.

Walkthrough

Adds class-level bytecode reference replacement: new ClassRefReplacingMethodVisitor rewrites class refs inside method bodies; DesugarClassVisitor now rewrites field/method descriptors and composes the new visitor with DesugarMethodVisitor; DSL and params gain classReplacements and replaceClass(...) APIs.

Changes

Cohort / File(s) Summary
Class reference rewrite core
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt, composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
Added ClassRefReplacingMethodVisitor to rewrite bytecode class references (invokes, fields, type instructions, LDC class constants, local var descriptors, signatures, try/catch types) using a slash-form map. Added ReplaceClassRef data class modeling from/to class names with internal (slash) conversions.
Class visitor & factory
.../DesugarClassVisitor.kt, .../DesugarClassVisitorFactory.kt
Expanded DesugarClassVisitor to rewrite field and method descriptors/signatures and compose method visitors so ClassRefReplacingMethodVisitor is applied before DesugarMethodVisitor. DesugarClassVisitorFactory centralizes params resolution and changes instrumentability/filtering to bypass included-package filtering when class replacements exist.
Configuration & DSL
.../DesugarParams.kt, .../DesugarReplacementsContainer.kt
DesugarParams gains classReplacements: MapProperty<String,String> and wiring from the extension. DesugarReplacementsContainer adds replaceClass(from: String, to: String) and replaceClass(from: Class<*>, to: Class<*>), input validation, normalization to dot-notation storage, and internal classReplacements map.
Formatting / minor refactors
.../dsl/ReplaceMethodInsn.kt, .../dsl/ReplaceMethodInsnKey.kt, .../dsl/MethodOpcode.kt
Formatting and indentation changes (enum and interface refactors, tabs/spaces) with no behavioral or API-signature changes.
Helpers added
(helpers within DesugarClassVisitor file) replaceInDescriptor, replaceInSignature
New string-based descriptor/signature replacement helpers performing L<from>;L<to>; substitutions for slash-form class names.

Sequence Diagram

sequenceDiagram
    participant Client
    participant DesugarClassVisitor
    participant ClassRefReplacingMethodVisitor
    participant DesugarMethodVisitor
    participant MethodBody

    Client->>DesugarClassVisitor: visitMethod(name, descriptor, signature)
    activate DesugarClassVisitor
    DesugarClassVisitor->>DesugarClassVisitor: rewrite method descriptor & signature
    DesugarClassVisitor->>ClassRefReplacingMethodVisitor: create with classReplacements
    activate ClassRefReplacingMethodVisitor
    DesugarClassVisitor->>DesugarMethodVisitor: create wrapping ClassRefReplacingMethodVisitor
    activate DesugarMethodVisitor
    MethodBody->>ClassRefReplacingMethodVisitor: emit instructions (NEW, CHECKCAST, LDC, field/method insns, local vars, try/catch)
    ClassRefReplacingMethodVisitor->>DesugarMethodVisitor: emit rewritten instructions
    DesugarMethodVisitor->>Client: return transformed method visitor
    deactivate DesugarMethodVisitor
    deactivate ClassRefReplacingMethodVisitor
    deactivate DesugarClassVisitor
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 A nibble, a hop through bytecode so neat,

I swap class names where instructions meet,
Visitors layered, dancing in line,
Descriptors rewritten, references align,
Hooray for hops that make bytecode complete!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% 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 accurately describes the main change: introducing the ability to rewrite class name references in the desugar plugin, which aligns with the new ClassRefReplacingMethodVisitor, updated DesugarClassVisitor, and DSL additions for class replacements.
Description check ✅ Passed The description references ADFA-3365 issue which relates to adding class name rewriting ability in the desugar plugin, directly connecting to the changeset's objective of enabling class reference desugaring.

✏️ 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-allow-class-ref-desugaring
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt (1)

24-105: Consider overriding visitInvokeDynamicInsn for complete coverage.

The visitor covers many bytecode sites but invokedynamic instructions (used by lambdas and method references) contain bootstrap method handles and descriptors that may also reference the replaced classes. Without handling this, lambda captures or method references involving the replaced class may not be rewritten.

Suggested addition
override fun visitInvokeDynamicInsn(
    name: String,
    descriptor: String,
    bootstrapMethodHandle: Handle,
    vararg bootstrapMethodArguments: Any?,
) {
    val newHandle = Handle(
        bootstrapMethodHandle.tag,
        replace(bootstrapMethodHandle.owner),
        bootstrapMethodHandle.name,
        replaceInDescriptor(bootstrapMethodHandle.desc),
        bootstrapMethodHandle.isInterface,
    )
    val newArgs = bootstrapMethodArguments.map { arg ->
        when (arg) {
            is Type -> when (arg.sort) {
                Type.OBJECT -> Type.getObjectType(replace(arg.internalName))
                Type.METHOD -> Type.getMethodType(replaceInDescriptor(arg.descriptor))
                else -> arg
            }
            is Handle -> Handle(
                arg.tag,
                replace(arg.owner),
                arg.name,
                replaceInDescriptor(arg.desc),
                arg.isInterface,
            )
            else -> arg
        }
    }.toTypedArray()
    super.visitInvokeDynamicInsn(name, replaceInDescriptor(descriptor), newHandle, *newArgs)
}

Note: Requires importing org.objectweb.asm.Handle.

🤖 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 - 105, Add an override of visitInvokeDynamicInsn in
ClassRefReplacingMethodVisitor to rewrite bootstrap method handles and bootstrap
arguments that reference replaced classes: create a new Handle from the incoming
bootstrapMethodHandle with owner/name/desc rewritten via replace(...) and
replaceInDescriptor(...), map bootstrapMethodArguments to rewrite Type (OBJECT
and METHOD kinds) and nested Handle entries similarly, and call
super.visitInvokeDynamicInsn with the rewritten descriptor
(replaceInDescriptor), new Handle, and rewritten args; also import
org.objectweb.asm.Handle. Ensure you use the existing replace(...) and
replaceInDescriptor(...) helpers and mirror how other visit* methods perform
replacements so invokedynamic lambdas and method refs are updated.
🤖 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 44-89: The current visitor rewrites descriptors but leaves raw
internal-name slots unmapped — add remapping for superName and interfaces by
overriding visit(classVersion, access, name, signature, superName, interfaces)
and remap the superName and each interfaces entry (e.g., via a new helper
replaceInternalName(name) or by delegating to an ASM Remapper), and also remap
the exceptions array inside visitMethod by mapping each exceptions[i] before
passing to super.visitMethod; keep existing
replaceInDescriptor/replaceInSignature, reuse the same helper across visit,
visitField, and visitMethod, and ensure
ClassRefReplacingMethodVisitor/DesugarMethodVisitor still wrap the returned
MethodVisitor.
- Around line 37-41: The current dot-to-slash conversion in
slashClassReplacements assumes input strings are already binary names and fails
for canonical names with nested classes (e.g., java.util.Map.Entry); update the
mapping code (where params.classReplacements.get().entries are processed into
slashClassReplacements) to first normalize canonical nested-class separators to
binary '$' separators and then convert dots to slashes, or alternatively reject
non-binary input; for example, before doing from.replace('.', '/') and
to.replace('.', '/'), run a small normalization that replaces occurrences of
".<Identifier>" that denote nested types with "$<Identifier>" (apply iteratively
until no change) so nested classes become binary names, and keep or document the
safer overload replaceClass(Class<*>, Class<*>) as preferred.

In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.kt`:
- Around line 105-118: KDoc for DesugarReplacementsContainer.replaceClass
currently states it throws UnsupportedOperationException but the implementation
uses require(...) which throws IllegalArgumentException; either update the KDoc
to document IllegalArgumentException (or remove the `@throws`) or change the
checks in replaceClass(fromClass: Class<*>, toClass: Class<*>) to explicitly
throw UnsupportedOperationException with the same error message (matching the
ReplaceMethodInsn.Builder.fromClass/toClass pattern) so the documented exception
and actual behavior are consistent.

In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsnKey.kt`:
- Around line 32-33: Move the instance-level declaration of serialVersionUID
into the class's companion object so it becomes a static field; in
ReplaceMethodInsnKey (the class containing the current `@JvmField` val
serialVersionUID = 1L), remove or relocate that line and add it inside the
companion object (matching the pattern used in ReplaceClassRef) as `@JvmField` val
serialVersionUID = 1L to ensure proper Java serialization behavior.

---

Nitpick comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt`:
- Around line 24-105: Add an override of visitInvokeDynamicInsn in
ClassRefReplacingMethodVisitor to rewrite bootstrap method handles and bootstrap
arguments that reference replaced classes: create a new Handle from the incoming
bootstrapMethodHandle with owner/name/desc rewritten via replace(...) and
replaceInDescriptor(...), map bootstrapMethodArguments to rewrite Type (OBJECT
and METHOD kinds) and nested Handle entries similarly, and call
super.visitInvokeDynamicInsn with the rewritten descriptor
(replaceInDescriptor), new Handle, and rewritten args; also import
org.objectweb.asm.Handle. Ensure you use the existing replace(...) and
replaceInDescriptor(...) helpers and mirror how other visit* methods perform
replacements so invokedynamic lambdas and method refs are updated.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 038a1430-53af-4e78-afe6-f3741b178f1b

📥 Commits

Reviewing files that changed from the base of the PR and between bfd285b and 1b069cb.

📒 Files selected for processing (9)
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/ClassRefReplacingMethodVisitor.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitorFactory.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarParams.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/DesugarReplacementsContainer.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/MethodOpcode.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsn.kt
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceMethodInsnKey.kt

Comment on lines +37 to +41
private val slashClassReplacements: Map<String, String> by lazy {
params.classReplacements.get()
.entries.associate { (from, to) ->
from.replace('.', '/') to to.replace('.', '/')
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd 'ReplaceClassRef\.kt|DesugarReplacementsContainer\.kt|DesugarParams\.kt' composite-builds/build-logic/desugaring/src/main/java -t f \
  -x sh -c 'printf "\n=== %s ===\n" "$1"; sed -n "1,220p" "$1"' sh {}

rg -n --type=kt -C2 'qualifiedName|canonicalName|replaceClass\(' composite-builds/build-logic/desugaring/src/main/java

Repository: appdevforall/CodeOnTheGo

Length of output: 7696


Use replaceClass(Class<*>, Class<*>) or validate that replaceClass(String, String) receives binary names, not canonical names.

Line 37–41 performs a simple dot-to-slash conversion that assumes from and to already use binary notation with $ for nested classes. If callers pass canonical names like java.util.Map.Entry, the conversion produces java/util/Map/Entry instead of the correct internal name java/util/Map$Entry, causing the class replacement to never match.

The replaceClass(Class<*>, Class<*>) overload is safe because it uses Class.name (binary names), but the replaceClass(String, String) overload has no validation. Either add validation to reject or normalize canonical names, or update the documentation to explicitly require binary notation.

🤖 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 37 - 41, The current dot-to-slash conversion in
slashClassReplacements assumes input strings are already binary names and fails
for canonical names with nested classes (e.g., java.util.Map.Entry); update the
mapping code (where params.classReplacements.get().entries are processed into
slashClassReplacements) to first normalize canonical nested-class separators to
binary '$' separators and then convert dots to slashes, or alternatively reject
non-binary input; for example, before doing from.replace('.', '/') and
to.replace('.', '/'), run a small normalization that replaces occurrences of
".<Identifier>" that denote nested types with "$<Identifier>" (apply iteratively
until no change) so nested classes become binary names, and keep or document the
safer overload replaceClass(Class<*>, Class<*>) as preferred.

Comment on lines +44 to +89
// -------------------------------------------------------------------------
// Class-structure level: rewrite descriptors in field / method declarations
// -------------------------------------------------------------------------

override fun visitField(
access: Int,
name: String,
descriptor: String,
signature: String?,
value: Any?,
): FieldVisitor? = super.visitField(
access,
name,
replaceInDescriptor(descriptor),
replaceInSignature(signature),
value,
)

override fun visitMethod(
access: Int,
name: String?,
descriptor: String?,
signature: String?,
exceptions: Array<out String>?,
): MethodVisitor {
// 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,
)

// Layer 1 — class-reference replacement inside the method body.
// Skip instantiation entirely when there are no class replacements.
val withClassRefs: MethodVisitor = when {
slashClassReplacements.isNotEmpty() ->
ClassRefReplacingMethodVisitor(api, base, slashClassReplacements)
else -> base
}

// Layer 2 — fine-grained method-call replacement.
// Runs first; any instruction it emits flows through withClassRefs.
return DesugarMethodVisitor(params, classContext, api, withClassRefs)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In ASM's ClassVisitor, which callbacks expose type references as raw internal names rather than descriptors? Specifically, are superName/interfacessupplied tovisit(...), and are declared checked exceptions supplied as a separate array in visitMethod(...)?

💡 Result:

Yes, in ASM's ClassVisitor: - The superName and interfaces parameters in visit(int version, int access, String name, String signature, String superName, String[] interfaces) are supplied as raw internal names (e.g., "java/lang/Object"), explicitly documented as such via Type.getInternalName. - Declared checked exceptions are supplied as a separate String[] exceptions array in visitMethod(int access, String name, String descriptor, String signature, String[] exceptions), where each element is the raw internal name of an exception class. These are type references provided as internal names rather than full type descriptors (which would include primitives like "I" or object references like "Ljava/lang/Object;"). Internal names are specifically for class/interface types (fully qualified name with '/' separators), distinguishing them from general descriptors used for fields (descriptor) and method parameters/returns (descriptor).

Citations:


🏁 Script executed:

fd -t f "DesugarClassVisitor.kt" --type f

Repository: appdevforall/CodeOnTheGo

Length of output: 177


🏁 Script executed:

rg -A 200 "class DesugarClassVisitor" --type kotlin

Repository: appdevforall/CodeOnTheGo

Length of output: 18205


🏁 Script executed:

cat -n composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt | sed -n '96,120p'

Repository: appdevforall/CodeOnTheGo

Length of output: 456


🏁 Script executed:

rg "fun replaceInDescriptor|fun replaceInSignature" composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/DesugarClassVisitor.kt -A 10

Repository: appdevforall/CodeOnTheGo

Length of output: 442


Remap superName, interfaces, and exceptions — all internal-name slots.

replaceInDescriptor(...) only covers descriptor format (L...;). The superName and interfaces parameters in a missing visit(...) override, plus the exceptions array in visitMethod(...), are raw internal names and remain unmapped. A class extending, implementing, or declaring a replaced type will carry the old reference after this pass. Extract a shared internal-name remapping helper or use ASM's Remapper to handle both formats uniformly.

🤖 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 44 - 89, The current visitor rewrites descriptors but leaves raw
internal-name slots unmapped — add remapping for superName and interfaces by
overriding visit(classVersion, access, name, signature, superName, interfaces)
and remap the superName and each interfaces entry (e.g., via a new helper
replaceInternalName(name) or by delegating to an ASM Remapper), and also remap
the exceptions array inside visitMethod by mapping each exceptions[i] before
passing to super.visitMethod; keep existing
replaceInDescriptor/replaceInSignature, reuse the same helper across visit,
visitField, and visitMethod, and ensure
ClassRefReplacingMethodVisitor/DesugarMethodVisitor still wrap the returned
MethodVisitor.

Comment on lines +105 to +118
/**
* Replaces every bytecode reference to [fromClass] with [toClass].
*
* @throws UnsupportedOperationException for array or primitive types.
*/
fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
require(!fromClass.isArray && !fromClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
require(!toClass.isArray && !toClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
replaceClass(fromClass.name, toClass.name)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

KDoc @throws clause doesn't match implementation.

The KDoc states @throws UnsupportedOperationException but require throws IllegalArgumentException. Either update the KDoc or use throw UnsupportedOperationException(...) if that's the intended exception type.

Option 1: Fix KDoc
 	/**
 	 * Replaces every bytecode reference to [fromClass] with [toClass].
 	 *
-	 * `@throws` UnsupportedOperationException for array or primitive types.
+	 * `@throws` IllegalArgumentException for array or primitive types.
 	 */
Option 2: Match existing ReplaceMethodInsn pattern

If UnsupportedOperationException is preferred (consistent with ReplaceMethodInsn.Builder.fromClass/toClass), change to explicit throws:

 	fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
-		require(!fromClass.isArray && !fromClass.isPrimitive) {
-			"Array and primitive types are not supported for class replacement."
-		}
-		require(!toClass.isArray && !toClass.isPrimitive) {
-			"Array and primitive types are not supported for class replacement."
-		}
+		if (fromClass.isArray || fromClass.isPrimitive) {
+			throw UnsupportedOperationException(
+				"Array and primitive types are not supported for class replacement."
+			)
+		}
+		if (toClass.isArray || toClass.isPrimitive) {
+			throw UnsupportedOperationException(
+				"Array and primitive types are not supported for class replacement."
+			)
+		}
 		replaceClass(fromClass.name, toClass.name)
 	}
📝 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.

Suggested change
/**
* Replaces every bytecode reference to [fromClass] with [toClass].
*
* @throws UnsupportedOperationException for array or primitive types.
*/
fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
require(!fromClass.isArray && !fromClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
require(!toClass.isArray && !toClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
replaceClass(fromClass.name, toClass.name)
}
/**
* Replaces every bytecode reference to [fromClass] with [toClass].
*
* `@throws` IllegalArgumentException for array or primitive types.
*/
fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
require(!fromClass.isArray && !fromClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
require(!toClass.isArray && !toClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
replaceClass(fromClass.name, toClass.name)
}
Suggested change
/**
* Replaces every bytecode reference to [fromClass] with [toClass].
*
* @throws UnsupportedOperationException for array or primitive types.
*/
fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
require(!fromClass.isArray && !fromClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
require(!toClass.isArray && !toClass.isPrimitive) {
"Array and primitive types are not supported for class replacement."
}
replaceClass(fromClass.name, toClass.name)
}
/**
* Replaces every bytecode reference to [fromClass] with [toClass].
*
* `@throws` UnsupportedOperationException for array or primitive types.
*/
fun replaceClass(fromClass: Class<*>, toClass: Class<*>) {
if (fromClass.isArray || fromClass.isPrimitive) {
throw UnsupportedOperationException(
"Array and primitive types are not supported for class replacement."
)
}
if (toClass.isArray || toClass.isPrimitive) {
throw UnsupportedOperationException(
"Array and primitive types are not supported for class replacement."
)
}
replaceClass(fromClass.name, toClass.name)
}
🤖 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/DesugarReplacementsContainer.kt`
around lines 105 - 118, KDoc for DesugarReplacementsContainer.replaceClass
currently states it throws UnsupportedOperationException but the implementation
uses require(...) which throws IllegalArgumentException; either update the KDoc
to document IllegalArgumentException (or remove the `@throws`) or change the
checks in replaceClass(fromClass: Class<*>, toClass: Class<*>) to explicitly
throw UnsupportedOperationException with the same error message (matching the
ReplaceMethodInsn.Builder.fromClass/toClass pattern) so the documented exception
and actual behavior are consistent.

Comment on lines +32 to +33
@JvmField
val serialVersionUID = 1L
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

serialVersionUID should be in a companion object.

The serialVersionUID is declared as an instance field, but Java serialization requires it to be a static field to be recognized. Compare with ReplaceClassRef.kt which correctly places it in a companion object.

Proposed fix
 ) : Serializable {
-	`@JvmField`
-	val serialVersionUID = 1L
+	companion object {
+		private const val serialVersionUID = 1L
+	}
 }
🤖 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/ReplaceMethodInsnKey.kt`
around lines 32 - 33, Move the instance-level declaration of serialVersionUID
into the class's companion object so it becomes a static field; in
ReplaceMethodInsnKey (the class containing the current `@JvmField` val
serialVersionUID = 1L), remove or relocate that line and add it inside the
companion object (matching the pattern used in ReplaceClassRef) as `@JvmField` val
serialVersionUID = 1L to ensure proper Java serialization behavior.

@itsaky-adfa itsaky-adfa force-pushed the fix/ADFA-3365-allow-class-ref-desugaring branch from 1b069cb to 6a5b20a Compare March 20, 2026 08:49
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa force-pushed the fix/ADFA-3365-allow-class-ref-desugaring branch from 6a5b20a to 4af1c30 Compare March 20, 2026 08:51
@@ -0,0 +1,31 @@
package com.itsaky.androidide.desugaring.dsl
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this file dead code?

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa requested a review from jatezzz March 23, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants