New: Allow injection point specifiers anywhere. (#129)

Takes the relevant changes from SpongePowered@e02f4ca.

src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java

@@ -162,15 +162,19 @@ public abstract class InjectionPoint {
     }
 
     /**
-     * Selector type for slice delmiters, ignored for normal injection points.
-     * <tt>Selectors</tt> can be supplied in {@link At} annotations by including
-     * a colon (<tt>:</tt>) character followed by the selector type
-     * (case-sensitive), eg:
+     * Additional specifier for injection points. <tt>Specifiers</tt> can be
+     * supplied in {@link At} annotations by including a colon (<tt>:</tt>)
+     * character followed by the specifier type (case-sensitive), eg:
      * 
      * <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
      */
-    public enum Selector {
-
+    public enum Specifier {
+
+        /**
+         * Use all instructions from the query result.
+         */
+        ALL,
+
         /**
          * Use the <em>first</em> instruction from the query result.
          */
@@ -186,13 +190,13 @@ public enum Selector {
          * more than one instruction this should be considered a fail-fast error
          * state and a runtime exception will be thrown.
          */
-        ONE;
+        ONE,
 
         /**
-         * Default selector type used if no selector is explicitly specified.
-         * <em>For internal use only. Currently {@link #FIRST}</em>
+         * Use the default setting as defined by the consumer. For slices this
+         * is {@link #FIRST}, for all other consumers this is {@link #ALL} 
          */
-        public static final Selector DEFAULT = Selector.FIRST;
+        DEFAULT;
 
     }
 
@@ -276,26 +280,26 @@ enum ShiftByViolationBehaviour {
     }
 
     private final String slice;
-    private final Selector selector;
+    private final Specifier specifier;
     private final String id;
     private final IMessageSink messageSink;
 
 
     protected InjectionPoint() {
-        this("", Selector.DEFAULT, null);
+        this("", Specifier.DEFAULT, null);
     }
 
     protected InjectionPoint(InjectionPointData data) {
-        this(data.getSlice(), data.getSelector(), data.getId(), data.getMessageSink());
+        this(data.getSlice(), data.getSpecifier(), data.getId(), data.getMessageSink());
     }
 
-    public InjectionPoint(String slice, Selector selector, String id) {
-        this(slice, selector, id, null);
+    public InjectionPoint(String slice, Specifier specifier, String id) {
+        this(slice, specifier, id, null);
     }
 
-    public InjectionPoint(String slice, Selector selector, String id, IMessageSink messageSink) {
+    public InjectionPoint(String slice, Specifier specifier, String id, IMessageSink messageSink) {
         this.slice = slice;
-        this.selector = selector;
+        this.specifier = specifier;
         this.id = id;
         this.messageSink = messageSink;
     }
@@ -304,8 +308,8 @@ public String getSlice() {
         return this.slice;
     }
 
-    public Selector getSelector() {
-        return this.selector;
+    public Specifier getSpecifier(Specifier defaultSpecifier) {
+        return this.specifier == Specifier.DEFAULT ? defaultSpecifier : this.specifier;
     }
 
     public String getId() {

src/main/java/org/spongepowered/asm/mixin/injection/Slice.java

@@ -28,7 +28,7 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
-import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector;
+import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier;
 
 /**
  * A <tt>Slice</tt> identifies a section of a method to search for injection
@@ -162,10 +162,10 @@
 
     /**
      * Injection point which specifies the <em>start</em> of the slice region.
-     * {@link At}s supplied here should generally specify a {@link Selector}
+     * {@link At}s supplied here should generally use a {@link Specifier}
      * in order to identify which instruction should be used for queries which
-     * return multiple results. The selector is specified by appending the
-     * selector type to the injection point type as follows:
+     * return multiple results. The specifier is supplied by appending the
+     * specifier type to the injection point type as follows:
      * 
      * <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
      * 
@@ -182,9 +182,9 @@
     /**
      * Injection point which specifies the <em>end</em> of the slice region.
      * Like {@link #from}, {@link At}s supplied here should generally specify a
-     * {@link Selector} in order to identify which instruction should be used
-     * for queries which return multiple results. The selector is specified by
-     * appending the selector type to the injection point type as follows:
+     * {@link Specifier} in order to identify which instruction should be used
+     * for queries which return multiple results. The specifier is supplied by
+     * appending the specifier type to the injection point type as follows:
      * 
      * <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
      * 

src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java

@@ -42,6 +42,7 @@
 import org.spongepowered.asm.mixin.injection.Coerce;
 import org.spongepowered.asm.mixin.injection.InjectionPoint;
 import org.spongepowered.asm.mixin.injection.InjectionPoint.RestrictTargetLevel;
+import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier;
 import org.spongepowered.asm.mixin.injection.invoke.RedirectInjector;
 import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
 import org.spongepowered.asm.mixin.injection.struct.InjectionNodes.InjectionNode;
@@ -294,7 +295,7 @@ private Collection<TargetNode> findTargetNodes(InjectorTarget injectorTarget, Li
         IMixinContext mixin = this.info.getMixin();
         MethodNode method = injectorTarget.getMethod();
         Map<Integer, TargetNode> targetNodes = new TreeMap<Integer, TargetNode>();
-        Collection<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32);
+        List<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32);
 
         for (InjectionPoint injectionPoint : injectionPoints) {
             nodes.clear();
@@ -307,22 +308,37 @@ private Collection<TargetNode> findTargetNodes(InjectorTarget injectorTarget, Li
                         injectorTarget, injectorTarget.getMergedBy(), injectorTarget.getMergedPriority()));
             }
 
-            if (this.findTargetNodes(method, injectionPoint, injectorTarget, nodes)) {
+            if (!this.findTargetNodes(method, injectionPoint, injectorTarget, nodes)) {
+                continue;
+            }
+
+            Specifier specifier = injectionPoint.getSpecifier(Specifier.ALL);
+            if (specifier == Specifier.ONE && nodes.size() != 1) {
+                throw new InvalidInjectionException(this.info, String.format("%s on %s has specifier :ONE but matched %d instructions",
+                        injectionPoint, this, nodes.size()));
+            } else if (specifier != Specifier.ALL && nodes.size() > 1) {
+                AbstractInsnNode specified = nodes.get(specifier == Specifier.FIRST ? 0 : nodes.size() - 1);
+                this.addTargetNode(method, targetNodes, injectionPoint, specified);
+            } else {
                 for (AbstractInsnNode insn : nodes) {
-                    Integer key = method.instructions.indexOf(insn);
-                    TargetNode targetNode = targetNodes.get(key);
-                    if (targetNode == null) {
-                        targetNode = new TargetNode(insn);
-                        targetNodes.put(key, targetNode);
-                    }
-                    targetNode.nominators.add(injectionPoint);
+                    this.addTargetNode(method, targetNodes, injectionPoint, insn);
                 }
             }
         }
 
         return targetNodes.values();
     }
 
+    protected void addTargetNode(MethodNode method, Map<Integer, TargetNode> targetNodes, InjectionPoint injectionPoint, AbstractInsnNode insn) {
+        Integer key = method.instructions.indexOf(insn);
+        TargetNode targetNode = targetNodes.get(key);
+        if (targetNode == null) {
+            targetNode = new TargetNode(insn);
+            targetNodes.put(key, targetNode);
+        }
+        targetNode.nominators.add(injectionPoint);
+    }
+
     protected boolean findTargetNodes(MethodNode into, InjectionPoint injectionPoint, InjectorTarget injectorTarget,
             Collection<AbstractInsnNode> nodes) {
         return injectionPoint.find(into.desc, injectorTarget.getSlice(injectionPoint), nodes);

src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlice.java

@@ -37,7 +37,7 @@
 import org.spongepowered.asm.mixin.MixinEnvironment.Option;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.InjectionPoint;
-import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector;
+import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier;
 import org.spongepowered.asm.mixin.injection.Slice;
 import org.spongepowered.asm.mixin.injection.struct.InjectionPointAnnotationContext;
 import org.spongepowered.asm.mixin.injection.throwables.InjectionError;
@@ -325,6 +325,11 @@ public int realIndexOf(AbstractInsnNode insn) {
      * Descriptive name of the slice, used in exceptions
      */
     private final String name;
+
+    /**
+     * Success counts for from and to injection points 
+     */
+    private int successCountFrom, successCountTo;
 
     /**
      * ctor
@@ -361,8 +366,8 @@ public String getId() {
      */
     public InsnListReadOnly getSlice(MethodNode method) {
         int max = method.instructions.size() - 1;
-        int start = this.find(method, this.from, 0, 0, this.name + "(from)");
-        int end = this.find(method, this.to, max, start, this.name + "(to)");
+        int start = this.find(method, this.from, 0, 0, "from");
+        int end = this.find(method, this.to, max, start, "to");
 
         if (start > end) {
             throw new InvalidSliceException(this.owner, String.format("%s is negative size. Range(%d -> %d)", this.describe(), start, end));
@@ -389,32 +394,53 @@ public InsnListReadOnly getSlice(MethodNode method) {
      * @param defaultValue Value to return if injection point is null (open
      *      ended)
      * @param failValue Value to use if query fails
-     * @param description Description for error message
+     * @param argument The name of the argument ("from" or "to") for debug msgs
      * @return matching insn index
      */
-    private int find(MethodNode method, InjectionPoint injectionPoint, int defaultValue, int failValue, String description) {
+    private int find(MethodNode method, InjectionPoint injectionPoint, int defaultValue, int failValue, String argument) {
         if (injectionPoint == null) {
             return defaultValue;
         }
 
+        String description = String.format("%s(%s)", this.name, argument);
         Deque<AbstractInsnNode> nodes = new LinkedList<AbstractInsnNode>();
         InsnListReadOnly insns = new InsnListReadOnly(method.instructions);
         boolean result = injectionPoint.find(method.desc, insns, nodes);
-        Selector select = injectionPoint.getSelector();
-        if (nodes.size() != 1 && select == Selector.ONE) {
+        Specifier specifier = injectionPoint.getSpecifier(Specifier.FIRST);
+        if (specifier == Specifier.ALL) {
+            throw new InvalidSliceException(this.owner, String.format("ALL is not a valid specifier for slice %s", this.describe(description)));
+        }
+        if (nodes.size() != 1 && specifier == Specifier.ONE) {
             throw new InvalidSliceException(this.owner, String.format("%s requires 1 result but found %d", this.describe(description), nodes.size()));
         }
 
         if (!result) {
-            if (this.owner.getMixin().getOption(Option.DEBUG_VERBOSE)) {
-                MethodSlice.logger.warn("{} did not match any instructions", this.describe(description));
-            }
             return failValue;
         }
 
-        return method.instructions.indexOf(select == Selector.FIRST ? nodes.getFirst() : nodes.getLast());
+        if ("from".equals(argument)) {
+            this.successCountFrom++;
+        } else {
+            this.successCountTo++;
+        }
+
+        return method.instructions.indexOf(specifier == Specifier.FIRST ? nodes.getFirst() : nodes.getLast());
     }
-
+
+    /**
+     * Perform post-injection debugging and validation tasks
+     */
+    public void postInject() {
+        if (this.owner.getMixin().getOption(Option.DEBUG_VERBOSE)) {
+            if (this.from != null && this.successCountFrom == 0) {
+                MethodSlice.logger.warn("{} did not match any instructions", this.describe(this.name + "(from)"));
+            }
+            if (this.to != null && this.successCountTo == 0) {
+                MethodSlice.logger.warn("{} did not match any instructions", this.describe(this.name + "(to)"));
+            }
+        }
+    }
+
     /* (non-Javadoc)
      * @see java.lang.Object#toString()
      */

src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlices.java

@@ -82,6 +82,16 @@ public MethodSlice get(String id) {
         return this.slices.get(id);
     }
 
+    /**
+     * Called to do post-injection validation/debug logging for slices
+     */
+    public void postInject() {
+        for (MethodSlice slice : this.slices.values()) {
+            slice.postInject();
+        }
+    }
+
+
     /* (non-Javadoc)
      * @see java.lang.Object#toString()
      */

src/main/java/org/spongepowered/asm/mixin/injection/points/BeforeConstant.java

@@ -147,7 +147,7 @@ public class BeforeConstant extends InjectionPoint {
     private final boolean log;
 
     public BeforeConstant(IMixinContext context, AnnotationNode node, String returnType) {
-        super(Annotations.<String>getValue(node, "slice", ""), Selector.DEFAULT, null);
+        super(Annotations.<String>getValue(node, "slice", ""), Specifier.DEFAULT, null);
 
         Boolean empty = Annotations.<Boolean>getValue(node, "nullValue", (Boolean)null);
         this.ordinal = Annotations.<Integer>getValue(node, "ordinal", Integer.valueOf(-1));

src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionInfo.java

@@ -473,6 +473,8 @@ public void postInject() {
                     String.format("Critical injection failure: %s %s%s in %s failed injection check, %d succeeded of %d allowed.%s",
                     description, this.methodName, this.method.desc, this.mixin, this.injectedCallbackCount, this.maxCallbackCount, extraInfo));
         }
+
+        this.slices.postInject();
     }
 
     /**

src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionPointData.java

@@ -36,7 +36,7 @@
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.IInjectionPointContext;
 import org.spongepowered.asm.mixin.injection.Inject;
-import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector;
+import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier;
 import org.spongepowered.asm.mixin.injection.modify.LocalVariableDiscriminator;
 import org.spongepowered.asm.mixin.injection.selectors.ITargetSelector;
 import org.spongepowered.asm.mixin.injection.selectors.InvalidSelectorException;
@@ -86,7 +86,7 @@ public class InjectionPointData {
     /**
      * Selector parsed from the at argument, only used by slices  
      */
-    private final Selector selector;
+    private final Specifier specifier;
 
     /**
      * Target 
@@ -131,7 +131,7 @@ public InjectionPointData(IInjectionPointContext context, String at, List<String
 
         Matcher matcher = InjectionPointData.AT_PATTERN.matcher(at);
         this.type = InjectionPointData.parseType(matcher, at);
-        this.selector = InjectionPointData.parseSelector(matcher);
+        this.specifier = InjectionPointData.parseSpecifier(matcher);
     }
 
     private void parseArgs(List<String> args) {
@@ -172,10 +172,10 @@ public String getType() {
     }
 
     /**
-     * Get the selector value parsed from the injector
+     * Get the specifier value parsed from the injector
      */
-    public Selector getSelector() {
-        return this.selector;
+    public Specifier getSpecifier() {
+        return this.specifier;
     }
 
     /**
@@ -362,7 +362,7 @@ public String toString() {
     }
 
     private static Pattern createPattern() {
-        return Pattern.compile(String.format("^(.+?)(:(%s))?$", Joiner.on('|').join(Selector.values())));
+        return Pattern.compile(String.format("^(.+?)(:(%s))?$", Joiner.on('|').join(Specifier.values())));
     }
 
     /**
@@ -380,8 +380,8 @@ private static String parseType(Matcher matcher, String at) {
         return matcher.matches() ? matcher.group(1) : at;
     }
 
-    private static Selector parseSelector(Matcher matcher) {
-        return matcher.matches() && matcher.group(3) != null ? Selector.valueOf(matcher.group(3)) : Selector.DEFAULT;
+    private static Specifier parseSpecifier(Matcher matcher) {
+        return matcher.matches() && matcher.group(3) != null ? Specifier.valueOf(matcher.group(3)) : Specifier.DEFAULT;
     }
 
     private static int parseInt(String string, int defaultValue) {