FELIX-4219 Variables named not stored in bytecode for constructors during manipulation

Modify the constructor manipulation to keep the argument names.

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1520506 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
index 3967d5e..cced500 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
@@ -157,7 +157,7 @@
      * Check if the class was already manipulated.

      * @return true if the class is already manipulated.

      */

-    public boolean isalreadyManipulated() {

+    public boolean isAlreadyManipulated() {

         return m_isAlreadyManipulated;

     }

 

diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/Manipulator.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/Manipulator.java
index 9a15d55..a9d439b 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/Manipulator.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/Manipulator.java
@@ -97,7 +97,7 @@
         m_version = ck.getClassVersion();
 
         ClassWriter finalWriter = null;
-        if (!ck.isalreadyManipulated()) {
+        if (!ck.isAlreadyManipulated()) {
             // Manipulation ->
             // Add the _setComponentManager method
             // Instrument all fields
@@ -115,7 +115,7 @@
             finalWriter = cw0;
         }
         // The file is in the bundle
-        if (ck.isalreadyManipulated()) {
+        if (ck.isAlreadyManipulated()) {
             return origin;
         } else {
             return finalWriter.toByteArray();
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
index 8513ce1..ca5de8e 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
@@ -19,11 +19,7 @@
 

 package org.apache.felix.ipojo.manipulation;

 

-import java.util.ArrayList;

-import java.util.Iterator;

-import java.util.List;

-import java.util.Map;

-import java.util.Set;

+import java.util.*;

 

 import org.apache.felix.ipojo.manipulation.ClassChecker.AnnotationDescriptor;

 import org.objectweb.asm.ClassAdapter;

@@ -194,7 +190,8 @@
             Type[] args = Type.getArgumentTypes(desc);

 

             // TODO HERE ! => All constructor matches, no distinction between the different constructors.

-            generateConstructor(access, desc, signature, exceptions, md.getAnnotations(), md.getParameterAnnotations());

+            generateConstructor(access, desc, signature, exceptions, md.getAnnotations(),

+                    md.getParameterAnnotations(), md.getLocals());

 

             if (args.length == 0) {

                 m_foundSuitableConstructor = true;

@@ -222,7 +219,8 @@
         if (md == null) {

             generateMethodHeader(access, name, desc, signature, exceptions, null, null, null);

         } else {

-            generateMethodHeader(access, name, desc, signature, exceptions, md.getArgumentLocalVariables(), md.getAnnotations(), md.getParameterAnnotations());

+            generateMethodHeader(access, name, desc, signature, exceptions, md.getArgumentLocalVariables(),

+                    md.getAnnotations(), md.getParameterAnnotations());

         }

 

         String id = generateMethodFlag(name, desc);

@@ -308,8 +306,11 @@
      * @param signature : method signature

      * @param exceptions : declared exception

      * @param annotations : the annotations to move to this constructor.

+     * @param locals : the local variables from the original constructors.

      */

-    private void generateConstructor(int access, String descriptor, String signature, String[] exceptions, List<AnnotationDescriptor> annotations, Map<Integer, List<AnnotationDescriptor>> paramAnnotations) {

+    private void generateConstructor(int access, String descriptor, String signature, String[] exceptions,

+                                     List<AnnotationDescriptor> annotations, Map<Integer,

+            List<AnnotationDescriptor>> paramAnnotations, LinkedHashMap<Integer, LocalVariableNode> locals) {

          GeneratorAdapter mv = new GeneratorAdapter(

                  cv.visitMethod(access, "<init>", descriptor, signature, exceptions),

                  access, "<init>", descriptor);

@@ -318,11 +319,15 @@
          newDesc = "(Lorg/apache/felix/ipojo/InstanceManager;" + newDesc;

 

          mv.visitCode();

+         Label start = new Label();

+         mv.visitLabel(start);

          mv.visitVarInsn(ALOAD, 0);

          mv.visitInsn(ACONST_NULL);

          mv.loadArgs();

          mv.visitMethodInsn(INVOKESPECIAL, m_owner, "<init>", newDesc);

          mv.visitInsn(RETURN);

+         Label stop = new Label();

+         mv.visitLabel(stop);

 

          // Move annotations

          if (annotations != null) {

@@ -345,6 +350,16 @@
              }

          }

 

+         // Add local variables for the arguments.

+        for (Map.Entry<Integer, LocalVariableNode> local : locals.entrySet()) {

+            // Write the parameter name. Only write the local variable that are either `this` or parameters from the

+            // initial descriptor.

+            if (local.getValue().index <= Type.getArgumentTypes(descriptor).length) {

+                mv.visitLocalVariable(local.getValue().name, local.getValue().desc, local.getValue().signature, start,stop,

+                        local.getValue().index);

+            }

+        }

+

          mv.visitMaxs(0, 0);

          mv.visitEnd();

     }

@@ -362,7 +377,9 @@
      * @param annotations : the annotations to move to this method.

      * @param paramAnnotations : the parameter annotations to move to this method.

      */

-    private void generateMethodHeader(int access, String name, String desc, String signature, String[] exceptions, List<LocalVariableNode> localVariables, List<AnnotationDescriptor> annotations, Map<Integer, List<AnnotationDescriptor>> paramAnnotations) {

+    private void generateMethodHeader(int access, String name, String desc, String signature, String[] exceptions,

+                                      List<LocalVariableNode> localVariables, List<AnnotationDescriptor> annotations,

+                                      Map<Integer, List<AnnotationDescriptor>> paramAnnotations) {

         GeneratorAdapter mv = new GeneratorAdapter(cv.visitMethod(access, name, desc, signature, exceptions), access, name, desc);

 

         // If we have variables, we wraps the code within labels. The `lifetime` of the variables are bound to those

diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodDescriptor.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodDescriptor.java
index a477560..fe0890c 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodDescriptor.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodDescriptor.java
@@ -259,4 +259,7 @@
         return m_argLocalVariables;

     }

 

+    public LinkedHashMap<Integer, LocalVariableNode> getLocals() {

+        return m_locals;

+    }

 }

diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ClassCheckerTestCase.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ClassCheckerTestCase.java
index 9e7398e..ca0f95b 100644
--- a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ClassCheckerTestCase.java
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ClassCheckerTestCase.java
@@ -39,12 +39,12 @@
 
     public void testIsAlreadyManipulatedWithNotManipulatedResource() throws Exception {
         ClassChecker checker = check(resource("test/SimplePojo.class"));
-        assertFalse(checker.isalreadyManipulated());
+        assertFalse(checker.isAlreadyManipulated());
     }
 
     public void testIsAlreadyManipulatedWithManipulatedResource() throws Exception {
         ClassChecker checker = check(manipulate(resource("test/SimplePojo.class")));
-        assertTrue(checker.isalreadyManipulated());
+        assertTrue(checker.isAlreadyManipulated());
     }
 
     public void testMetadataForAlreadyManipulatedClassAreCleaned() throws Exception {
diff --git a/ipojo/manipulator/manipulator/src/test/java/test/PlentyOfAnnotations.java b/ipojo/manipulator/manipulator/src/test/java/test/PlentyOfAnnotations.java
index 9aa65ec..b6d11ed 100644
--- a/ipojo/manipulator/manipulator/src/test/java/test/PlentyOfAnnotations.java
+++ b/ipojo/manipulator/manipulator/src/test/java/test/PlentyOfAnnotations.java
@@ -56,4 +56,8 @@
         // ...
     }
 
+    public String doSomethingWithArguments(String message, int value) {
+        return message + " - " + value;
+    }
+
 }