Skip to content

Commit

Permalink
Improve logging implicit match failures for ModifyVariable, fixes Spo…
Browse files Browse the repository at this point in the history 
…ngePowered#382

Also add some scaffolding for adding the same capability to other
injectors
  • Loading branch information
@Mumfrey
Mumfrey committed Jan 23, 2020
1 parent e32a38e commit f5536fb
Show file tree
Hide file tree
Showing 10 changed files with 401 additions and 190 deletions.
3 changes: 2 additions & 1 deletion src/main/java/org/spongepowered/asm/mixin/injection/IInjectionPointContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@
import org.objectweb.asm.tree.AnnotationNode;
import org.objectweb.asm.tree.MethodNode;
import org.spongepowered.asm.mixin.refmap.IMixinContext;
import org.spongepowered.asm.util.IMessageSink;

/**
* Context for an injection point, used to access the mixin as well as the owner
* method and annotation for the injection point
*/
public interface IInjectionPointContext {
public interface IInjectionPointContext extends IMessageSink {

/**
* Get the mixin context for this injection
Expand Down
140 changes: 93 additions & 47 deletions src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,21 @@
import org.spongepowered.asm.mixin.injection.struct.InjectionPointData;
import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
import org.spongepowered.asm.mixin.refmap.IMixinContext;
import org.spongepowered.asm.mixin.struct.AnnotatedMethodInfo;
import org.spongepowered.asm.mixin.transformer.MixinTargetContext;
import org.spongepowered.asm.util.Annotations;
import org.spongepowered.asm.util.Bytecode;
import org.spongepowered.asm.util.IMessageSink;

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;

/**
* <p>Base class for injection point discovery classes. Each subclass describes
* a strategy for locating code injection points within a method, with the
* {@link #find} method populating a collection with insn nodes from the method
* which satisfy its strategy.</p>
* a strategy for locating code injection points within an instruction list,
* with the {@link #find} method populating a collection with insn nodes from
* the supplied list which satisfy its strategy.</p>
*
* <p>This base class also contains composite strategy factory methods such as
* {@link #and} and {@link #or} which allow strategies to be combined using
Expand Down Expand Up @@ -214,21 +216,28 @@ enum ShiftByViolationBehaviour {
private final String slice;
private final Selector selector;
private final String id;
private final IMessageSink messageSink;


protected InjectionPoint() {
this("", Selector.DEFAULT, null);
}

protected InjectionPoint(InjectionPointData data) {
this(data.getSlice(), data.getSelector(), data.getId());
this(data.getSlice(), data.getSelector(), data.getId(), data.getMessageSink());
}

public InjectionPoint(String slice, Selector selector, String id) {
this(slice, selector, id, null);
}

public InjectionPoint(String slice, Selector selector, String id, IMessageSink messageSink) {
this.slice = slice;
this.selector = selector;
this.id = id;
this.messageSink = messageSink;
}

public String getSlice() {
return this.slice;
}
Expand All @@ -241,6 +250,20 @@ public String getId() {
return this.id;
}

/**
* Notify method for subclasses to log when notable but non-fatal failures
* occur, for example allows subclasses to add notes when they return no
* results.
*
* @param format Message format
* @param args Format args
*/
protected void addMessage(String format, Object... args) {
if (this.messageSink != null) {
this.messageSink.addMessage(format, args);
}
}

/**
* Runs a priority check in the context of this injection point. A priority
* check should return <tt>true</tt> if the injection point is allowed to
Expand Down Expand Up @@ -505,32 +528,32 @@ public static InjectionPoint shift(InjectionPoint point, int count) {
* Parse a collection of InjectionPoints from the supplied {@link At}
* annotations
*
* @param owner Data for the mixin containing the annotation, used to obtain
* the refmap, amongst other things
* @param context Data for the mixin containing the annotation, used to
* obtain the refmap, amongst other things
* @param method The annotated handler method
* @param parent The parent annotation which owns this {@link At} annotation
* @param ats {@link At} annotations to parse information from
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static List<InjectionPoint> parse(IInjectionPointContext owner, List<AnnotationNode> ats) {
return InjectionPoint.parse(owner.getContext(), owner.getMethod(), owner.getAnnotation(), ats);
public static List<InjectionPoint> parse(IMixinContext context, MethodNode method, AnnotationNode parent, List<AnnotationNode> ats) {
return InjectionPoint.parse(new AnnotatedMethodInfo(context, method, parent), ats);
}

/**
* Parse a collection of InjectionPoints from the supplied {@link At}
* annotations
*
* @param context Data for the mixin containing the annotation, used to
* obtain the refmap, amongst other things
* @param method The annotated handler method
* @param parent The parent annotation which owns this {@link At} annotation
* @param context Data for the mixin containing the annotation, used to obtain
* the refmap, amongst other things
* @param ats {@link At} annotations to parse information from
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static List<InjectionPoint> parse(IMixinContext context, MethodNode method, AnnotationNode parent, List<AnnotationNode> ats) {
public static List<InjectionPoint> parse(IInjectionPointContext context, List<AnnotationNode> ats) {
Builder<InjectionPoint> injectionPoints = ImmutableList.<InjectionPoint>builder();
for (AnnotationNode at : ats) {
InjectionPoint injectionPoint = InjectionPoint.parse(context, method, parent, at);
InjectionPoint injectionPoint = InjectionPoint.parse(context, at);
if (injectionPoint != null) {
injectionPoints.add(injectionPoint);
}
Expand All @@ -541,14 +564,14 @@ public static List<InjectionPoint> parse(IMixinContext context, MethodNode metho
/**
* Parse an InjectionPoint from the supplied {@link At} annotation
*
* @param owner Data for the mixin containing the annotation, used to obtain
* @param context Data for the mixin containing the annotation, used to obtain
* the refmap, amongst other things
* @param at {@link At} annotation to parse information from
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static InjectionPoint parse(IInjectionPointContext owner, At at) {
return InjectionPoint.parse(owner.getContext(), owner.getMethod(), owner.getAnnotation(), at.value(), at.shift(), at.by(),
public static InjectionPoint parse(IInjectionPointContext context, At at) {
return InjectionPoint.parse(context, at.value(), at.shift(), at.by(),
Arrays.asList(at.args()), at.target(), at.slice(), at.ordinal(), at.opcode(), at.id());
}

Expand All @@ -564,52 +587,52 @@ public static InjectionPoint parse(IInjectionPointContext owner, At at) {
* failed
*/
public static InjectionPoint parse(IMixinContext context, MethodNode method, AnnotationNode parent, At at) {
return InjectionPoint.parse(context, method, parent, at.value(), at.shift(), at.by(), Arrays.asList(at.args()), at.target(), at.slice(),
at.ordinal(), at.opcode(), at.id());
return InjectionPoint.parse(new AnnotatedMethodInfo(context, method, parent), at.value(), at.shift(), at.by(), Arrays.asList(at.args()),
at.target(), at.slice(), at.ordinal(), at.opcode(), at.id());
}

/**
* Parse an InjectionPoint from the supplied {@link At} annotation supplied
* as an AnnotationNode instance
*
* @param owner Data for the mixin containing the annotation, used to obtain
* the refmap, amongst other things
* @param node {@link At} annotation to parse information from
* @param context Data for the mixin containing the annotation, used to
* obtain the refmap, amongst other things
* @param method The annotated handler method
* @param parent The parent annotation which owns this {@link At} annotation
* @param at {@link At} annotation to parse information from
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static InjectionPoint parse(IInjectionPointContext owner, AnnotationNode node) {
return InjectionPoint.parse(owner.getContext(), owner.getMethod(), owner.getAnnotation(), node);
public static InjectionPoint parse(IMixinContext context, MethodNode method, AnnotationNode parent, AnnotationNode at) {
return InjectionPoint.parse(new AnnotatedMethodInfo(context, method, parent), at);
}

/**
* Parse an InjectionPoint from the supplied {@link At} annotation supplied
* as an AnnotationNode instance
*
* @param context Data for the mixin containing the annotation, used to
* obtain the refmap, amongst other things
* @param method The annotated handler method
* @param parent The parent annotation which owns this {@link At} annotation
* @param node {@link At} annotation to parse information from
* @param context Data for the mixin containing the annotation, used to obtain
* the refmap, amongst other things
* @param at {@link At} annotation to parse information from
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static InjectionPoint parse(IMixinContext context, MethodNode method, AnnotationNode parent, AnnotationNode node) {
String at = Annotations.<String>getValue(node, "value");
List<String> args = Annotations.<List<String>>getValue(node, "args");
String target = Annotations.<String>getValue(node, "target", "");
String slice = Annotations.<String>getValue(node, "slice", "");
At.Shift shift = Annotations.<At.Shift>getValue(node, "shift", At.Shift.class, At.Shift.NONE);
int by = Annotations.<Integer>getValue(node, "by", Integer.valueOf(0));
int ordinal = Annotations.<Integer>getValue(node, "ordinal", Integer.valueOf(-1));
int opcode = Annotations.<Integer>getValue(node, "opcode", Integer.valueOf(0));
String id = Annotations.<String>getValue(node, "id");
public static InjectionPoint parse(IInjectionPointContext context, AnnotationNode at) {
String value = Annotations.<String>getValue(at, "value");
List<String> args = Annotations.<List<String>>getValue(at, "args");
String target = Annotations.<String>getValue(at, "target", "");
String slice = Annotations.<String>getValue(at, "slice", "");
At.Shift shift = Annotations.<At.Shift>getValue(at, "shift", At.Shift.class, At.Shift.NONE);
int by = Annotations.<Integer>getValue(at, "by", Integer.valueOf(0));
int ordinal = Annotations.<Integer>getValue(at, "ordinal", Integer.valueOf(-1));
int opcode = Annotations.<Integer>getValue(at, "opcode", Integer.valueOf(0));
String id = Annotations.<String>getValue(at, "id");

if (args == null) {
args = ImmutableList.<String>of();
}

return InjectionPoint.parse(context, method, parent, at, shift, by, args, target, slice, ordinal, opcode, id);
return InjectionPoint.parse(context, value, shift, by, args, target, slice, ordinal, opcode, id);
}

/**
Expand All @@ -634,10 +657,33 @@ public static InjectionPoint parse(IMixinContext context, MethodNode method, Ann
*/
public static InjectionPoint parse(IMixinContext context, MethodNode method, AnnotationNode parent, String at, At.Shift shift, int by,
List<String> args, String target, String slice, int ordinal, int opcode, String id) {
InjectionPointData data = new InjectionPointData(context, method, parent, at, args, target, slice, ordinal, opcode, id);
Class<? extends InjectionPoint> ipClass = findClass(context, data);
InjectionPoint point = InjectionPoint.create(context, data, ipClass);
return InjectionPoint.shift(context, method, parent, point, shift, by);
return InjectionPoint.parse(new AnnotatedMethodInfo(context, method, parent), at, shift, by, args, target, slice, ordinal, opcode, id);
}

/**
* Parse and instantiate an InjectionPoint from the supplied information.
* Returns null if an InjectionPoint could not be created.
*
* @param context The injection point context which owns this {@link At}
* annotation
* @param at Injection point specifier
* @param shift Shift type to apply
* @param by Amount of shift to apply for the BY shift type
* @param args Named parameters
* @param target Target for supported injection points
* @param slice Slice id for injectors which support multiple slices
* @param ordinal Ordinal offset for supported injection points
* @param opcode Bytecode opcode for supported injection points
* @param id Injection point id from annotation
* @return InjectionPoint parsed from the supplied data or null if parsing
* failed
*/
public static InjectionPoint parse(IInjectionPointContext context, String at, At.Shift shift, int by,
List<String> args, String target, String slice, int ordinal, int opcode, String id) {
InjectionPointData data = new InjectionPointData(context, at, args, target, slice, ordinal, opcode, id);
Class<? extends InjectionPoint> ipClass = findClass(context.getContext(), data);
InjectionPoint point = InjectionPoint.create(context.getContext(), data, ipClass);
return InjectionPoint.shift(context, point, shift, by);
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -678,7 +724,7 @@ private static InjectionPoint create(IMixinContext context, InjectionPointData d
return point;
}

private static InjectionPoint shift(IMixinContext context, MethodNode method, AnnotationNode parent, InjectionPoint point,
private static InjectionPoint shift(IInjectionPointContext context, InjectionPoint point,
At.Shift shift, int by) {

if (point != null) {
Expand All @@ -687,7 +733,7 @@ private static InjectionPoint shift(IMixinContext context, MethodNode method, An
} else if (shift == At.Shift.AFTER) {
return InjectionPoint.after(point);
} else if (shift == At.Shift.BY) {
InjectionPoint.validateByValue(context, method, parent, point, by);
InjectionPoint.validateByValue(context.getContext(), context.getMethod(), context.getAnnotation(), point, by);
return InjectionPoint.shift(point, by);
}
}
Expand Down
Loading

0 comments on commit f5536fb

Please sign in to comment.