Refactor how method parameters are read/handled

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedMethod.java

@@ -32,17 +32,19 @@
 package org.jf.dexlib2.dexbacked;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterators;
-import org.jf.dexlib2.base.BaseMethodParameter;
 import org.jf.dexlib2.base.reference.BaseMethodReference;
 import org.jf.dexlib2.dexbacked.util.AnnotationsDirectory;
 import org.jf.dexlib2.dexbacked.util.FixedSizeList;
+import org.jf.dexlib2.dexbacked.util.ParameterIterator;
 import org.jf.dexlib2.iface.Annotation;
 import org.jf.dexlib2.iface.Method;
 import org.jf.dexlib2.iface.MethodParameter;
+import org.jf.util.AbstractForwardSequentialList;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
@@ -102,6 +104,7 @@ public class DexBackedMethod extends BaseMethodReference implements Method {
         this.parameterAnnotationSetListOffset = paramaterAnnotationIterator.seekTo(methodIndex);
     }
 
+    public int getMethodIndex() { return methodIndex; }
     @Nonnull @Override public String getDefiningClass() { return classDef.getType(); }
     @Override public int getAccessFlags() { return accessFlags; }
 
@@ -120,63 +123,43 @@ public class DexBackedMethod extends BaseMethodReference implements Method {
     @Nonnull
     @Override
     public List<? extends MethodParameter> getParameters() {
-        if (getParametersOffset() > 0) {
+        int parametersOffset = getParametersOffset();
-            DexBackedMethodImplementation methodImpl = getImplementation();
+        if (parametersOffset > 0) {
-            if (methodImpl != null) {
+            final List<String> parameterTypes = getParameterTypes();
-                return methodImpl.getParametersWithNames();
+
-            }
+            return new AbstractForwardSequentialList<MethodParameter>() {
-            return getParametersWithoutNames();
+                @Nonnull @Override public Iterator<MethodParameter> iterator() {
+                    return new ParameterIterator(parameterTypes,
+                            getParameterAnnotations(),
+                            getParameterNames());
+                }
+
+                @Override public int size() {
+                    return parameterTypes.size();
+                }
+            };
         }
         return ImmutableList.of();
     }
 
     @Nonnull
-    public List<? extends MethodParameter> getParametersWithoutNames() {
+    public List<? extends Set<? extends DexBackedAnnotation>> getParameterAnnotations() {
-        final int parametersOffset = getParametersOffset();
+        return AnnotationsDirectory.getParameterAnnotations(dexBuf, parameterAnnotationSetListOffset);
-        if (parametersOffset > 0) {
+    }
-            final int size = dexBuf.readSmallUint(parametersOffset);
-            final List<Set<? extends DexBackedAnnotation>> parameterAnnotations =
-                    AnnotationsDirectory.getParameterAnnotations(dexBuf, parameterAnnotationSetListOffset);
 
-            return new FixedSizeList<MethodParameter>() {
+    @Nonnull
-                @Nonnull
+    public Iterator<String> getParameterNames() {
-                @Override
+        DexBackedMethodImplementation methodImpl = getImplementation();
-                public MethodParameter readItem(final int index) {
+        if (methodImpl != null) {
-                    return new BaseMethodParameter() {
+            return methodImpl.getParameterNames(null);
-                        @Nonnull
-                        @Override
-                        public String getType() {
-                            int typeIndex = dexBuf.readUshort(parametersOffset + 4 + (index * 2));
-                            return dexBuf.getType(typeIndex);
-                        }
-
-                        @Nonnull
-                        @Override
-                        public Set<? extends Annotation> getAnnotations() {
-                            if (index < parameterAnnotations.size()) {
-                                return parameterAnnotations.get(index);
-                            }
-                            return ImmutableSet.of();
-                        }
-
-                        @Nullable @Override public String getName() { return null; }
-                        //TODO: iterate over the annotations to get the signature
-                        @Nullable @Override public String getSignature() { return null; }
-                    };
-                }
-
-                @Override public int size() { return size; }
-            };
         }
-
+        return Iterators.emptyIterator();
-        return ImmutableList.of();
     }
 
     @Nonnull
     @Override
     public List<String> getParameterTypes() {
-        int protoIdItemOffset = getProtoIdItemOffset();
+        final int parametersOffset = getParametersOffset();
-        final int parametersOffset = dexBuf.readSmallUint(protoIdItemOffset + DexBuffer.PROTO_PARAM_LIST_OFF_OFFSET);
         if (parametersOffset > 0) {
             final int parameterCount = dexBuf.readSmallUint(parametersOffset + DexBuffer.TYPE_LIST_SIZE_OFFSET);
             final int paramListStart = parametersOffset + DexBuffer.TYPE_LIST_LIST_OFFSET;

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/DexBackedMethodImplementation.java

@@ -37,13 +37,13 @@ import org.jf.dexlib2.dexbacked.util.DebugInfo;
 import org.jf.dexlib2.dexbacked.util.FixedSizeList;
 import org.jf.dexlib2.dexbacked.util.VariableSizeLookaheadIterator;
 import org.jf.dexlib2.iface.MethodImplementation;
-import org.jf.dexlib2.iface.MethodParameter;
 import org.jf.dexlib2.iface.TryBlock;
 import org.jf.dexlib2.iface.debug.DebugItem;
 import org.jf.dexlib2.iface.instruction.Instruction;
 import org.jf.util.AlignmentUtils;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import java.util.Iterator;
 import java.util.List;
 
@@ -132,7 +132,7 @@ public class DexBackedMethodImplementation implements MethodImplementation {
     }
 
     @Nonnull
-    public List<? extends MethodParameter> getParametersWithNames() {
+    public Iterator<String> getParameterNames(@Nullable DexReader dexReader) {
-        return getDebugInfo().getParametersWithNames();
+        return getDebugInfo().getParameterNames(dexReader);
     }
 }

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/util/DebugInfo.java

@@ -34,12 +34,10 @@ package org.jf.dexlib2.dexbacked.util;
 import com.google.common.collect.Iterators;
 import org.jf.dexlib2.AccessFlags;
 import org.jf.dexlib2.DebugItemType;
-import org.jf.dexlib2.base.BaseMethodParameter;
 import org.jf.dexlib2.dexbacked.DexBackedMethod;
 import org.jf.dexlib2.dexbacked.DexBackedMethodImplementation;
 import org.jf.dexlib2.dexbacked.DexBuffer;
 import org.jf.dexlib2.dexbacked.DexReader;
-import org.jf.dexlib2.iface.Annotation;
 import org.jf.dexlib2.iface.MethodParameter;
 import org.jf.dexlib2.iface.debug.DebugItem;
 import org.jf.dexlib2.iface.debug.EndLocal;
@@ -48,25 +46,33 @@ import org.jf.dexlib2.immutable.debug.*;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Iterator;
 
 public abstract class DebugInfo implements Iterable<DebugItem> {
-    @Nonnull public abstract List<? extends MethodParameter> getParametersWithNames();
+    /**
+     * Gets an iterator that yields the parameter names from the debug_info_item
+     *
+     * @param reader Optional. If provided, the reader must be positioned at the debug_info_item.parameters_size
+     *               field, and will
+     * @return An iterator that yields the parameter names as strings
+     */
+    @Nonnull public abstract Iterator<String> getParameterNames(@Nullable DexReader reader);
 
     public static DebugInfo newOrEmpty(@Nonnull DexBuffer dexBuf, int debugInfoOffset,
                                        @Nonnull DexBackedMethodImplementation methodImpl) {
         if (debugInfoOffset == 0) {
-            return new EmptyDebugInfo(methodImpl.method);
+            return EmptyDebugInfo.INSTANCE;
         }
         return new DebugInfoImpl(dexBuf, debugInfoOffset, methodImpl);
     }
 
     private static class EmptyDebugInfo extends DebugInfo {
-        @Nonnull private final DexBackedMethod method;
+        public static final EmptyDebugInfo INSTANCE = new EmptyDebugInfo();
-        public EmptyDebugInfo(@Nonnull DexBackedMethod method) { this.method = method; }
+        private EmptyDebugInfo() {}
         @Nonnull @Override public Iterator<DebugItem> iterator() { return Iterators.emptyIterator(); }
-        @Nonnull @Override public List<? extends MethodParameter> getParametersWithNames() {
+        @Nonnull @Override public Iterator<String> getParameterNames(@Nullable DexReader reader) {
-            return method.getParametersWithoutNames();
+            return Iterators.emptyIterator();
         }
     }
 
@@ -92,17 +98,24 @@ public abstract class DebugInfo implements Iterable<DebugItem> {
         @Nonnull
         @Override
         public Iterator<DebugItem> iterator() {
-            DexReader initialReader = dexBuf.readerAt(debugInfoOffset);
+            DexReader reader = dexBuf.readerAt(debugInfoOffset);
             // TODO: this unsigned value could legitimally be > MAX_INT
-            final int lineNumberStart = initialReader.readSmallUleb128();
+            final int lineNumberStart = reader.readSmallUleb128();
             int registerCount = methodImpl.getRegisterCount();
 
             //TODO: does dalvik allow references to invalid registers?
             final LocalInfo[] locals = new LocalInfo[registerCount];
             Arrays.fill(locals, EMPTY_LOCAL_INFO);
 
-            final VariableSizeListIterator<? extends MethodParameter> parameterIterator =
+            DexBackedMethod method = methodImpl.method;
-                    getParametersWithNames().listIterator();
+
+            // Create a MethodParameter iterator that uses our DexReader instance to read the parameter names.
+            // After we have finished iterating over the parameters, reader will "point to" the beginning of the
+            // debug instructions
+            final Iterator<? extends MethodParameter> parameterIterator =
+                    new ParameterIterator(method.getParameterTypes(),
+                            method.getParameterAnnotations(),
+                            getParameterNames(reader));
 
             // first, we grab all the parameters and temporarily store them at the beginning of locals,
             // disregarding any wide types
@@ -111,7 +124,7 @@ public abstract class DebugInfo implements Iterable<DebugItem> {
                 // add the local info for the "this" parameter
                 locals[parameterIndex++] = new LocalInfo() {
                     @Override public String getName() { return "this"; }
-                    @Override public String getType() { return methodImpl.method.classDef.getType(); }
+                    @Override public String getType() { return methodImpl.method.getDefiningClass(); }
                     @Override public String getSignature() { return null; }
                 };
             }
@@ -138,7 +151,7 @@ public abstract class DebugInfo implements Iterable<DebugItem> {
                 }
             }
 
-            return new VariableSizeLookaheadIterator<DebugItem>(dexBuf, parameterIterator.getReaderOffset()) {
+            return new VariableSizeLookaheadIterator<DebugItem>(dexBuf, reader.getOffset()) {
                 private int codeAddress = 0;
                 private int lineNumber = lineNumberStart;
 
@@ -231,34 +244,16 @@ public abstract class DebugInfo implements Iterable<DebugItem> {
 
         @Nonnull
         @Override
-        public VariableSizeList<MethodParameter> getParametersWithNames() {
+        public VariableSizeIterator<String> getParameterNames(@Nullable DexReader reader) {
-            DexReader reader = dexBuf.readerAt(debugInfoOffset);
+            if (reader == null) {
-            reader.skipUleb128();
+                reader = dexBuf.readerAt(debugInfoOffset);
-            final int parameterNameCount = reader.readSmallUleb128();
+                reader.skipUleb128();
-            final List<? extends MethodParameter> methodParametersWithoutNames =
+            }
-                    methodImpl.method.getParametersWithoutNames();
             //TODO: make sure dalvik doesn't allow more parameter names than we have parameters
-
+            final int parameterNameCount = reader.readSmallUleb128();
-            return new VariableSizeList<MethodParameter>(dexBuf, reader.getOffset(),
+            return new VariableSizeIterator<String>(reader, parameterNameCount) {
-                                                               methodParametersWithoutNames.size()) {
+                @Override protected String readNextItem(@Nonnull DexReader reader, int index) {
-                @Nonnull
+                    return dexBuf.getOptionalString(reader.readSmallUleb128() - 1);
-                @Override
-                protected MethodParameter readNextItem(@Nonnull DexReader reader, int index) {
-                    final MethodParameter methodParameter = methodParametersWithoutNames.get(index);
-                    String _name = null;
-                    if (index < parameterNameCount) {
-                        _name = dexBuf.getOptionalString(reader.readSmallUleb128() - 1);
-                    }
-                    final String name = _name;
-
-                    return new BaseMethodParameter() {
-                        @Nonnull @Override public String getType() { return methodParameter.getType(); }
-                        @Nullable @Override public String getName() { return name; }
-                        @Nullable @Override public String getSignature() { return methodParameter.getSignature();}
-                        @Nonnull @Override public Set<? extends Annotation> getAnnotations() {
-                            return methodParameter.getAnnotations();
-                        }
-                    };
                 }
             };
         }

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/util/ParameterIterator.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright 2012, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.jf.dexlib2.dexbacked.util;
+
+import com.google.common.collect.ImmutableSet;
+import org.jf.dexlib2.base.BaseMethodParameter;
+import org.jf.dexlib2.iface.Annotation;
+import org.jf.dexlib2.iface.MethodParameter;
+import org.jf.util.ExceptionWithContext;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+public class ParameterIterator implements Iterator<MethodParameter> {
+    private final Iterator<? extends CharSequence> parameterTypes;
+    private final Iterator<? extends Set<? extends Annotation>> parameterAnnotations;
+    private final Iterator<String> parameterNames;
+
+    public ParameterIterator(@Nonnull List<? extends CharSequence> parameterTypes,
+                             @Nonnull List<? extends Set<? extends Annotation>> parameterAnnotations,
+                             @Nonnull Iterator<String> parameterNames) {
+        // TODO: verify if dalvik allows this
+        if (parameterAnnotations.size() > parameterTypes.size()) {
+            throw new ExceptionWithContext("Too many parameter annotations");
+        }
+        this.parameterTypes = parameterTypes.iterator();
+        this.parameterAnnotations = parameterAnnotations.iterator();
+        this.parameterNames = parameterNames;
+    }
+
+    @Override public boolean hasNext() {
+        return parameterTypes.hasNext();
+    }
+
+    @Override public MethodParameter next() {
+        @Nonnull final String type = parameterTypes.next().toString();
+        @Nonnull final Set<? extends Annotation> annotations;
+        @Nullable final String name;
+
+        if (parameterAnnotations.hasNext()) {
+            annotations = parameterAnnotations.next();
+        } else {
+            annotations = ImmutableSet.of();
+        }
+
+        if (parameterNames.hasNext()) {
+            name = parameterNames.next();
+        } else {
+            name = null;
+        }
+
+        return new BaseMethodParameter() {
+            @Nonnull @Override public Set<? extends Annotation> getAnnotations() { return annotations; }
+            @Nullable @Override public String getName() { return name; }
+            @Nonnull @Override public String getType() { return type; }
+        };
+    }
+
+    @Override public void remove() {
+        throw new UnsupportedOperationException();
+    }
+}

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/util/VariableSizeCollection.java

@@ -36,7 +36,6 @@ import org.jf.dexlib2.dexbacked.DexReader;
 
 import javax.annotation.Nonnull;
 import java.util.AbstractCollection;
-import java.util.Iterator;
 
 public abstract class VariableSizeCollection<T> extends AbstractCollection<T> {
     @Nonnull private final DexBuffer dexBuf;
@@ -49,12 +48,11 @@ public abstract class VariableSizeCollection<T> extends AbstractCollection<T> {
         this.size = size;
     }
 
-    @Nonnull protected abstract T readNextItem(@Nonnull DexReader reader, int index);
+    protected abstract T readNextItem(@Nonnull DexReader reader, int index);
 
     @Override
     public VariableSizeIterator<T> iterator() {
         return new VariableSizeIterator<T>(dexBuf, offset, size) {
-            @Nonnull
             @Override
             protected T readNextItem(@Nonnull DexReader reader, int index) {
                 return VariableSizeCollection.this.readNextItem(reader, index);

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/util/VariableSizeIterator.java

@@ -35,7 +35,6 @@ import org.jf.dexlib2.dexbacked.DexBuffer;
 import org.jf.dexlib2.dexbacked.DexReader;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 
@@ -50,6 +49,11 @@ public abstract class VariableSizeIterator<T> implements Iterator<T> {
         this.size = size;
     }
 
+    protected VariableSizeIterator(@Nonnull DexReader reader, int size) {
+        this.reader = reader;
+        this.size = size;
+    }
+
     /**
      * Reads the next item from reader.
      *
@@ -57,7 +61,7 @@ public abstract class VariableSizeIterator<T> implements Iterator<T> {
      * @param index The index of the item being read. This is guaranteed to be less than {@code size}
      * @return The item that was read
      */
-    @Nonnull protected abstract T readNextItem(@Nonnull DexReader reader, int index);
+    protected abstract T readNextItem(@Nonnull DexReader reader, int index);
 
     public int getReaderOffset() {
         return reader.getOffset();
@@ -69,7 +73,6 @@ public abstract class VariableSizeIterator<T> implements Iterator<T> {
     }
 
     @Override
-    @Nonnull
     public T next() {
         if (index >= size) {
             throw new NoSuchElementException();

dexlib2/src/main/java/org/jf/dexlib2/dexbacked/util/VariableSizeSet.java

@@ -48,12 +48,11 @@ public abstract class VariableSizeSet<T> extends AbstractSet<T> {
         this.size = size;
     }
 
-    @Nonnull protected abstract T readNextItem(@Nonnull DexReader reader, int index);
+    protected abstract T readNextItem(@Nonnull DexReader reader, int index);
 
     @Override
     public VariableSizeIterator<T> iterator() {
         return new VariableSizeIterator<T>(dexBuf, offset, size) {
-            @Nonnull
             @Override
             protected T readNextItem(@Nonnull DexReader reader, int index) {
                 return VariableSizeSet.this.readNextItem(reader, index);