refactor: improve the addInstructions extension method further more

src/main/kotlin/app/revanced/patcher/extensions/Extensions.kt

@@ -2,6 +2,7 @@ package app.revanced.patcher.extensions
 
 import app.revanced.patcher.util.proxy.mutableTypes.MutableMethod
 import app.revanced.patcher.util.proxy.mutableTypes.MutableMethod.Companion.toMutable
+import app.revanced.patcher.util.smali.ExternalLabel
 import app.revanced.patcher.util.smali.toInstruction
 import app.revanced.patcher.util.smali.toInstructions
 import org.jf.dexlib2.AccessFlags
@@ -11,6 +12,7 @@ import org.jf.dexlib2.builder.Label
 import org.jf.dexlib2.builder.MutableMethodImplementation
 import org.jf.dexlib2.builder.instruction.*
 import org.jf.dexlib2.iface.Method
+import org.jf.dexlib2.iface.instruction.Instruction
 import org.jf.dexlib2.iface.reference.MethodReference
 import org.jf.dexlib2.immutable.ImmutableMethod
 import org.jf.dexlib2.immutable.ImmutableMethodImplementation
@@ -50,8 +52,10 @@ fun MutableMethodImplementation.removeInstructions(index: Int, count: Int) {
  * @return True if the methods match given the conditions.
  */
 fun Method.softCompareTo(otherMethod: MethodReference): Boolean {
-    if (MethodUtil.isConstructor(this) && !parametersEqual(this.parameterTypes, otherMethod.parameterTypes))
+    if (MethodUtil.isConstructor(this) && !parametersEqual(
-        return false
+            this.parameterTypes, otherMethod.parameterTypes
+        )
+    ) return false
     return this.name == otherMethod.name
 }
 
@@ -71,14 +75,7 @@ internal fun Method.clone(registerCount: Int = 0): ImmutableMethod {
         )
     }
     return ImmutableMethod(
-        returnType,
+        returnType, name, parameters, returnType, accessFlags, annotations, hiddenApiRestrictions, clonedImplementation
-        name,
-        parameters,
-        returnType,
-        accessFlags,
-        annotations,
-        hiddenApiRestrictions,
-        clonedImplementation
     )
 }
 
@@ -109,46 +106,88 @@ fun MutableMethod.replaceInstruction(index: Int, instruction: String) =
  * Remove a smali instruction within the method.
  * @param index The index to delete the instruction at.
  */
-fun MutableMethod.removeInstruction(index: Int) =
+fun MutableMethod.removeInstruction(index: Int) = this.implementation!!.removeInstruction(index)
-    this.implementation!!.removeInstruction(index)
+
+/**
+ * Create a label for the instruction at given index in the method's implementation.
+ * @param index The index to create the label for the instruction at.
+ * @return The label.
+ */
+fun MutableMethod.label(index: Int) = this.implementation!!.newLabelForIndex(index)
+
+/**
+ * Get the instruction at given index in the method's implementation.
+ * @param index The index to get the instruction at.
+ * @return The instruction.
+ */
+fun MutableMethod.instruction(index: Int): BuilderInstruction = this.implementation!!.instructions[index]
 
 /**
  * Add smali instructions to the method.
  * @param index The index to insert the instructions at.
  * @param smali The smali instructions to add.
+ * @param externalLabels A list of [ExternalLabel] representing a list of labels for instructions which are not in the method to compile.
  */
-fun MutableMethod.addInstructions(index: Int, smali: String, labels: List<Pair<String, Label>> = emptyList()) {
-    var code = smali
-    for ((name, _) in labels) {
-        code += "\n :$name \n nop"
-    }
-    val instructions = code.toInstructions(this).toMutableList()
-    var fixedInstructions: List<Int>? = null // find a better way to do this.
 
-    if (labels.isNotEmpty()) {
+fun MutableMethod.addInstructions(index: Int, smali: String, externalLabels: List<ExternalLabel> = emptyList()) {
-        val labelRange = instructions.size - labels.size..instructions.size
+    // create reference dummy instructions for the instructions.
-        fixedInstructions = mutableListOf()
+    val nopedSmali = StringBuilder(smali).also { builder ->
-        for (instructionIndex in 0 until instructions.size - labels.size) {
+        externalLabels.forEach { (name, _) ->
-            val instruction = instructions[instructionIndex]
+            builder.append("\n:$name\nnop")
-            if (instruction !is BuilderOffsetInstruction || !instruction.target.isPlaced) continue
-            val fakeIndex = instruction.target.location.index
-            if (!labelRange.contains(fakeIndex)) continue
-            instructions[instructionIndex] = replaceOffset(instruction, labels[labelRange.indexOf(fakeIndex)].second)
-            fixedInstructions.add(instructionIndex + index)
         }
-        // find a better way to drop the nop instructions.
+    }.toString()
-        instructions.subList(labelRange.first, labelRange.last).clear()
+
+    // compile the instructions with the dummy labels
+    val compiledInstructions = nopedSmali.toInstructions(this)
+
+    // add the compiled list of instructions to the method.
+    val methodImplementation = this.implementation!!
+    methodImplementation.addInstructions(index, compiledInstructions)
+
+    val methodInstructions = methodImplementation.instructions
+    methodInstructions.subList(index, index + compiledInstructions.size)
+        .forEachIndexed { compiledInstructionIndex, compiledInstruction ->
+            // If the compiled instruction is not an offset instruction, skip it.
+            if (compiledInstruction !is BuilderOffsetInstruction) return@forEachIndexed
+
+            /**
+             * Creates a new label for the instruction and replaces it with the label of the [compiledInstruction] at [compiledInstructionIndex].
+             */
+            fun Instruction.makeNewLabel() {
+                // create the final label.
+                val label = methodImplementation.newLabelForIndex(methodInstructions.indexOf(this))
+                // create the final instruction with the new label.
+                val newInstruction = replaceOffset(
+                    compiledInstruction, label
+                )
+                // replace the instruction pointing to the dummy label with the new instruction pointing to the real instruction.
+                methodImplementation.replaceInstruction(index + compiledInstructionIndex, newInstruction)
             }
 
-    this.implementation!!.addInstructions(index, instructions)
+            // If the compiled instruction targets its own instruction,
-    this.fixInstructions(index, instructions, fixedInstructions)
+            // which means it points to some of its own, simply an offset has to be applied.
+            val labelIndex = compiledInstruction.target.location.index
+            if (labelIndex < compiledInstructions.size - externalLabels.size) {
+                // get the targets index (insertion index + the index of the dummy instruction).
+                methodInstructions[index + labelIndex].makeNewLabel()
+                return@forEachIndexed
+            }
+
+            // since the compiled instruction points to a dummy instruction,
+            // we can find the real instruction which it was created for by calculation.
+
+            // get the index of the instruction in the externalLabels list which the dummy instruction was created for.
+            // this line works because we created the dummy instructions in the same order as the externalLabels list.
+            val (_, instruction) = externalLabels[(compiledInstructions.size - 1) - labelIndex]
+            instruction.makeNewLabel()
+        }
 }
 
 /**
  * Add smali instructions to the end of the method.
  * @param instructions The smali instructions to add.
  */
-fun MutableMethod.addInstructions(instructions: String, labels: List<Pair<String, Label>> = emptyList()) =
+fun MutableMethod.addInstructions(instructions: String, labels: List<ExternalLabel> = emptyList()) =
     this.addInstructions(this.implementation!!.instructions.size, instructions, labels)
 
 /**
@@ -164,27 +203,10 @@ fun MutableMethod.replaceInstructions(index: Int, instructions: String) =
  * @param index The index to remove the instructions at.
  * @param count The amount of instructions to remove.
  */
-fun MutableMethod.removeInstructions(index: Int, count: Int) =
+fun MutableMethod.removeInstructions(index: Int, count: Int) = this.implementation!!.removeInstructions(index, count)
-    this.implementation!!.removeInstructions(index, count)
-
-fun MutableMethod.label(index: Int) = this.implementation!!.newLabelForIndex(index)
-fun MutableMethod.instruction(index: Int): BuilderInstruction = this.implementation!!.instructions[index]
-
-private fun MutableMethod.fixInstructions(index: Int, instructions: List<BuilderInstruction>, skipInstructions: List<Int>?) {
-    for (instructionIndex in index until instructions.size + index) {
-        val instruction = this.implementation!!.instructions[instructionIndex]
-        if (instruction !is BuilderOffsetInstruction || !instruction.target.isPlaced) continue
-        if (skipInstructions?.contains(instructionIndex) == true) continue
-        val fakeIndex = instruction.target.location.index
-        val fixedIndex = fakeIndex + index
-        if (fakeIndex == fixedIndex) continue // no need to replace if the indexes are the same.
-        this.implementation!!.replaceInstruction(instructionIndex, replaceOffset(instruction, this.label(fixedIndex)))
-    }
-}
 
 private fun replaceOffset(
-    i: BuilderOffsetInstruction,
+    i: BuilderOffsetInstruction, label: Label
-    label: Label
 ): BuilderOffsetInstruction {
     return when (i) {
         is BuilderInstruction10t -> BuilderInstruction10t(i.opcode, label)
@@ -207,8 +229,7 @@ internal fun Method.cloneMutable(registerCount: Int = 0) = clone(registerCount).
 
 // FIXME: also check the order of parameters as different order equals different method overload
 internal fun parametersEqual(
-    parameters1: Iterable<CharSequence>,
+    parameters1: Iterable<CharSequence>, parameters2: Iterable<CharSequence>
-    parameters2: Iterable<CharSequence>
 ): Boolean {
     return parameters1.count() == parameters2.count() && parameters1.all { parameter ->
         parameters2.any {

src/main/kotlin/app/revanced/patcher/util/smali/ExternalLabel.kt

@@ -0,0 +1,10 @@
+package app.revanced.patcher.util.smali
+
+import org.jf.dexlib2.iface.instruction.Instruction
+
+/**
+ * A class that represents a label for an instruction.
+ * @param name The label name.
+ * @param instruction The instruction that this label is for.
+ */
+data class ExternalLabel(internal val name: String, internal val instruction: Instruction)

src/main/kotlin/app/revanced/patcher/util/smali/InlineSmaliCompiler.kt

@@ -45,7 +45,6 @@ class InlineSmaliCompiler {
             val tokens = CommonTokenStream(lexer as TokenSource)
             val parser = smaliParser(tokens)
             val result = parser.smali_file()
-            result.tree
             if (parser.numberOfSyntaxErrors > 0 || lexer.numberOfSyntaxErrors > 0) {
                 throw IllegalStateException(
                     "Encountered ${parser.numberOfSyntaxErrors} parser syntax errors and ${lexer.numberOfSyntaxErrors} lexer syntax errors!"

src/test/kotlin/app/revanced/patcher/util/smali/InlineSmaliCompilerTest.kt

@@ -57,7 +57,6 @@ internal class InlineSmaliCompilerTest {
         method.addInstructions(
             """
                 const/4 v0, 0x1
-                :test
                 const/4 v0, 0x0
             """
         )
@@ -70,7 +69,7 @@ internal class InlineSmaliCompilerTest {
                 if-eqz v0, :test
                 return-void
             """, listOf(
-                "test" to method.label(labelIndex)
+                ExternalLabel("test",method.instruction(1))
             )
         )