Skip to content

Commit 2de39c7

Browse files
ctruedenctrueden
authored
Merge pull request #19 from scijava/single-op-output
Restrict Ops to single output
2 parents 75e5155 + c107500 commit 2de39c7

38 files changed

+205
-227
lines changed

‎src/main/java/org/scijava/ops/OpService.java‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,9 @@ private void resolveOpDependencies(Object obj, OpCandidate parentOp) throws OpMa
231231

232232
@SuppressWarnings("unchecked")
233233
public <T> T findOpInstance(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes,
234-
final Nil<?>[] outTypes, final Object... secondaryArgs) {
235-
final OpRef ref = OpRef.fromTypes(opName, toTypes(specialType), toTypes(outTypes), toTypes(inTypes));
234+
final Nil<?> outType, final Object... secondaryArgs) {
235+
final OpRef ref = OpRef.fromTypes(opName, toTypes(specialType), outType != null ? outType.getType() : null, toTypes(
236+
inTypes));
236237
return (T) findOpInstance(opName, ref, secondaryArgs);
237238
}
238239

@@ -278,14 +279,9 @@ else if (transformation != null)
278279
return op;
279280
}
280281

281-
public <T> T findOp(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes, final Nil<?>[] outTypes,
282-
final Object... secondaryArgs) {
283-
return findOpInstance(opName, specialType, inTypes, outTypes, secondaryArgs);
284-
}
285-
286282
public <T> T findOp(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes, final Nil<?> outType,
287283
final Object... secondaryArgs) {
288-
return findOpInstance(opName, specialType, inTypes, new Nil[] { outType }, secondaryArgs);
284+
return findOpInstance(opName, specialType, inTypes, outType, secondaryArgs);
289285
}
290286

291287
private Type[] toTypes(Nil<?>... nils) {
@@ -295,11 +291,10 @@ private Type[] toTypes(Nil<?>... nils) {
295291
public Object run(final String opName, final Object... args) {
296292

297293
Nil<?>[] inTypes = Arrays.stream(args).map(arg -> Nil.of(typeService.reify(arg))).toArray(Nil[]::new);
298-
Nil<?>[] outTypes = new Nil<?>[] { new Nil<Object>() {
299-
} };
294+
Nil<?> outType = new Nil<Object>() {};
300295

301296
OpRunner<Object> op = findOpInstance(opName, new Nil<OpRunner<Object>>() {
302-
}, inTypes, outTypes);
297+
}, inTypes, outType);
303298

304299
// TODO change
305300
return op.run(args);
@@ -329,7 +324,9 @@ public Object run(final String opName, final Object... args) {
329324
* @param name
330325
* @return null if the specified type has no functional method
331326
*/
332-
private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> typeVarAssigns) {
327+
private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> typeVarAssigns)
328+
throws OpMatchingException
329+
{
333330
List<FunctionalMethodType> fmts = ParameterStructs.getFunctionalMethodTypes(type);
334331
if (fmts == null)
335332
return null;
@@ -346,7 +343,16 @@ private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> type
346343
Type[] mappedInputs = Types.mapVarToTypes(inputs, typeVarAssigns);
347344
Type[] mappedOutputs = Types.mapVarToTypes(outputs, typeVarAssigns);
348345

349-
return new OpRef(name, new Type[] { type }, mappedOutputs, mappedInputs);
346+
final int numOutputs = mappedOutputs.length;
347+
if (numOutputs != 1) {
348+
String error = "Op '" + name + "' of type " + type + " specifies ";
349+
error += numOutputs == 0 //
350+
? "no outputs" //
351+
: "multiple outputs: " + Arrays.toString(outputs);
352+
error += ". This is not supported.";
353+
throw new OpMatchingException(error);
354+
}
355+
return new OpRef(name, new Type[] { type }, mappedOutputs[0], mappedInputs);
350356
}
351357

352358
/**

‎src/main/java/org/scijava/ops/OpUtils.java‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131

3232
import java.lang.reflect.Type;
3333
import java.util.Arrays;
34+
import java.util.Collections;
3435
import java.util.List;
36+
import java.util.Objects;
3537
import java.util.stream.Collectors;
3638

3739
import org.scijava.ops.matcher.DefaultOpTypeMatchingService;
@@ -41,6 +43,8 @@
4143
import org.scijava.ops.matcher.OpInfo;
4244
import org.scijava.ops.matcher.OpRef;
4345
import org.scijava.param.ParameterMember;
46+
import org.scijava.param.ValidityException;
47+
import org.scijava.param.ValidityProblem;
4448
import org.scijava.struct.Member;
4549
import org.scijava.struct.MemberInstance;
4650
import org.scijava.struct.Struct;
@@ -139,12 +143,12 @@ public static Type[] inputTypes(OpCandidate candidate) {
139143
return getTypes(inputs(candidate.struct()));
140144
}
141145

142-
public static List<Member<?>> outputs(OpCandidate candidate) {
143-
return outputs(candidate.struct());
146+
public static Member<?> output(OpCandidate candidate) {
147+
return candidate.opInfo().output();
144148
}
145149

146-
public static Type[] outputTypes(OpCandidate candidate) {
147-
return getTypes(outputs(candidate.struct()));
150+
public static Type outputType(OpCandidate candidate) {
151+
return output(candidate).getType();
148152
}
149153

150154
public static List<Member<?>> outputs(final Struct struct) {
@@ -158,7 +162,17 @@ public static List<MemberInstance<?>> outputs(StructInstance<?> op) {
158162
.filter(memberInstance -> memberInstance.member().isOutput()) //
159163
.collect(Collectors.toList());
160164
}
161-
165+
166+
public static void checkHasSingleOutput(Struct struct) throws ValidityException {
167+
final int numOutputs = OpUtils.outputs(struct).size();
168+
if (numOutputs != 1) {
169+
final String error = numOutputs == 0 //
170+
? "No output parameters specified. Must specify exactly one." //
171+
: "Multiple output parameters specified. Only a single output is allowed.";
172+
throw new ValidityException(Collections.singletonList(new ValidityProblem(error)));
173+
}
174+
}
175+
162176
public static Type[] types(OpCandidate candidate) {
163177
return getTypes(candidate.struct().members());
164178
}
@@ -238,19 +252,19 @@ public static List<Member<?>> injectableMembers(Struct struct) {
238252

239253
/**
240254
* Checks if incomplete type matching could have occurred. If we have
241-
* several matches that do not have equal output types, output types may not
255+
* several matches that do not have equal output types, the output type may not
242256
* completely match the request as only raw type assignability will be checked
243257
* at the moment.
244258
* @see DefaultOpTypeMatchingService#typesMatch(OpCandidate)
245259
* @param matches
246260
* @return
247261
*/
248262
private static boolean typeCheckingIncomplete(List<OpCandidate> matches) {
249-
Type[] outputTypes = null;
263+
Type outputType = null;
250264
for (OpCandidate match : matches) {
251-
Type[] ts = getTypes(outputs(match));
252-
if (outputTypes == null || Arrays.deepEquals(outputTypes, ts)) {
253-
outputTypes = ts;
265+
Type ts = output(match).getType();
266+
if (outputType == null || Objects.equals(outputType, ts)) {
267+
outputType = ts;
254268
continue;
255269
} else {
256270
return true;
@@ -404,13 +418,12 @@ public static String opString(final OpInfo info) {
404418
sb.append("\n");
405419
}
406420
sb.append("\t Outputs:\n");
407-
for (final Member<?> arg : info.outputs()) {
408-
sb.append("\t\t");
409-
sb.append(arg.getType().getTypeName());
410-
sb.append(" ");
411-
sb.append(arg.getKey());
412-
sb.append("\n");
413-
}
421+
final Member<?> arg = info.output();
422+
sb.append("\t\t");
423+
sb.append(arg.getType().getTypeName());
424+
sb.append(" ");
425+
sb.append(arg.getKey());
426+
sb.append("\n");
414427
sb.append(")\n");
415428
return sb.toString();
416429
}

‎src/main/java/org/scijava/ops/matcher/DefaultOpTypeMatchingService.java‎

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.HashMap;
3737
import java.util.List;
3838
import java.util.Map;
39+
import java.util.Objects;
3940
import java.util.function.Predicate;
4041

4142
import org.scijava.Context;
@@ -239,15 +240,9 @@ private boolean typesPerfectMatch(final OpCandidate candidate) {
239240
i++;
240241
}
241242

242-
Type[] outputTypes = candidate.getRef().getOutTypes();
243-
i = 0;
244-
for (final Type t : OpUtils.outputTypes(candidate)) {
245-
if (outputTypes[i] != null) {
246-
if (!t.equals(outputTypes[i]))
247-
return false;
248-
}
249-
i++;
250-
}
243+
final Type outputType = candidate.getRef().getOutType();
244+
if (Objects.equals(outputType, OpUtils.outputType(candidate)))
245+
return false;
251246

252247
candidate.setStatus(StatusCode.MATCH);
253248
return true;
@@ -309,37 +304,26 @@ private boolean inputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>
309304
}
310305

311306
/**
312-
* Checks whether the output types of the candidate match the output types
313-
* of the {@link OpRef}. Sets candidate status code if there are too many,
314-
* to few, or not matching types.
307+
* Checks whether the output type of the candidate matches the output type
308+
* of the {@link OpRef}. Sets candidate status code if they are not matching.
315309
*
316310
* @param candidate
317-
* the candidate to check outputs for
311+
* the candidate to check output for
318312
* @param typeBounds
319313
* possibly predetermined type bounds for type variables
320314
* @return whether the output types match
321315
*/
322316
private boolean outputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
323-
final Type[] refOutTypes = candidate.getRef().getOutTypes();
324-
if (refOutTypes == null)
317+
final Type refOutType = candidate.getRef().getOutType();
318+
if (refOutType == null)
325319
return true; // no constraints on output types
326320

327-
Type[] candidateOutTypes = OpUtils.outputTypes(candidate);
328-
if (candidateOutTypes.length < refOutTypes.length) {
329-
candidate.setStatus(StatusCode.TOO_FEW_OUTPUTS);
330-
return false;
331-
} else if (candidateOutTypes.length > refOutTypes.length) {
332-
candidate.setStatus(StatusCode.TOO_MANY_OUTPUTS);
333-
return false;
334-
}
335-
336-
int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(candidateOutTypes, refOutTypes,
337-
typeBounds);
321+
final Type candidateOutType = OpUtils.outputType(candidate);
322+
final int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(new Type[] { candidateOutType },
323+
new Type[] { refOutType }, typeBounds);
338324
if (conflictingIndex != -1) {
339-
final Type to = refOutTypes[conflictingIndex];
340-
final Type from = candidateOutTypes[conflictingIndex];
341325
candidate.setStatus(StatusCode.OUTPUT_TYPES_DO_NOT_MATCH, //
342-
"request=" + to.getTypeName() + ", actual=" + from.getTypeName());
326+
"request=" + refOutType.getTypeName() + ", actual=" + candidateOutType.getTypeName());
343327
return false;
344328
}
345329
return true;

‎src/main/java/org/scijava/ops/matcher/OpCandidate.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ public class OpCandidate {
5353
public static enum StatusCode {
5454
MATCH, //
5555
INVALID_STRUCT, //
56-
TOO_FEW_OUTPUTS, //
57-
TOO_MANY_OUTPUTS,
5856
OUTPUT_TYPES_DO_NOT_MATCH, //
5957
TOO_MANY_ARGS, //
6058
TOO_FEW_ARGS, //
@@ -168,9 +166,6 @@ public String getStatus() {
168166
sb.append(vp.getMessage());
169167
}
170168
break;
171-
case TOO_FEW_OUTPUTS:
172-
sb.append("Too few outputs");
173-
break;
174169
case OUTPUT_TYPES_DO_NOT_MATCH:
175170
sb.append("Output types do not match");
176171
break;

‎src/main/java/org/scijava/ops/matcher/OpClassInfo.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@
3232
import java.lang.reflect.Constructor;
3333
import java.lang.reflect.InvocationTargetException;
3434
import java.lang.reflect.Type;
35-
import java.util.List;
3635

3736
import org.scijava.core.Priority;
3837
import org.scijava.ops.OpUtils;
3938
import org.scijava.ops.core.Op;
4039
import org.scijava.param.ParameterStructs;
4140
import org.scijava.param.ValidityException;
4241
import org.scijava.plugin.Plugin;
43-
import org.scijava.struct.Member;
4442
import org.scijava.struct.Struct;
4543
import org.scijava.struct.StructInstance;
4644
import org.scijava.util.Types;
@@ -61,6 +59,7 @@ public OpClassInfo(final Class<? extends Op> opClass) {
6159
this.opClass = opClass;
6260
try {
6361
struct = ParameterStructs.structOf(opClass);
62+
OpUtils.checkHasSingleOutput(struct);
6463
} catch (ValidityException e) {
6564
validityException = e;
6665
}
@@ -80,16 +79,6 @@ public Struct struct() {
8079
return struct;
8180
}
8281

83-
@Override
84-
public List<Member<?>> inputs() {
85-
return OpUtils.inputs(struct());
86-
}
87-
88-
@Override
89-
public List<Member<?>> outputs() {
90-
return OpUtils.outputs(struct());
91-
}
92-
9382
@Override
9483
public double priority() {
9584
final Plugin opAnnotation = opClass.getAnnotation(Plugin.class);

‎src/main/java/org/scijava/ops/matcher/OpFieldInfo.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public OpFieldInfo(final Field field) {
6969
this.field = field;
7070
try {
7171
struct = ParameterStructs.structOf(field.getDeclaringClass(), field);
72+
OpUtils.checkHasSingleOutput(struct);
7273
if (!Modifier.isStatic(field.getModifiers())) {
7374
instance = field.getDeclaringClass().newInstance();
7475
}

‎src/main/java/org/scijava/ops/matcher/OpInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ default List<Member<?>> inputs() {
3030
}
3131

3232
/** Gets the op's output parameters. */
33-
default List<Member<?>> outputs() {
34-
return OpUtils.outputs(struct());
33+
default Member<?> output() {
34+
return OpUtils.outputs(struct()).get(0);
3535
}
3636

3737
/** The op's priority. */

0 commit comments

Comments
 (0)