Skip to content

Commit 3fe607a

Browse files
wiedenmwiedenm
committed
Restrict Ops to single output
This commit removes the support for secondary Op outputs. Both OpRef and OpInfo only specify single outputs from now on. Op matching and transformation are adapted accordingly. Also, validation is added to enforce this new constraint during Op discovery.
1 parent 13dd8af commit 3fe607a

38 files changed

+206
-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: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929

3030
package org.scijava.ops.matcher;
3131

32+
import com.google.common.base.Objects;
33+
3234
import java.lang.reflect.Type;
3335
import java.lang.reflect.TypeVariable;
3436
import java.util.ArrayList;
@@ -239,15 +241,9 @@ private boolean typesPerfectMatch(final OpCandidate candidate) {
239241
i++;
240242
}
241243

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-
}
244+
final Type outputType = candidate.getRef().getOutType();
245+
if (Objects.equal(outputType, OpUtils.outputType(candidate)))
246+
return false;
251247

252248
candidate.setStatus(StatusCode.MATCH);
253249
return true;
@@ -309,37 +305,26 @@ private boolean inputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>
309305
}
310306

311307
/**
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.
308+
* Checks whether the output type of the candidate matches the output type
309+
* of the {@link OpRef}. Sets candidate status code if they are not matching.
315310
*
316311
* @param candidate
317-
* the candidate to check outputs for
312+
* the candidate to check output for
318313
* @param typeBounds
319314
* possibly predetermined type bounds for type variables
320315
* @return whether the output types match
321316
*/
322317
private boolean outputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
323-
final Type[] refOutTypes = candidate.getRef().getOutTypes();
324-
if (refOutTypes == null)
318+
final Type refOutType = candidate.getRef().getOutType();
319+
if (refOutType == null)
325320
return true; // no constraints on output types
326321

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);
322+
final Type candidateOutType = OpUtils.outputType(candidate);
323+
final int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(new Type[] { candidateOutType },
324+
new Type[] { refOutType }, typeBounds);
338325
if (conflictingIndex != -1) {
339-
final Type to = refOutTypes[conflictingIndex];
340-
final Type from = candidateOutTypes[conflictingIndex];
341326
candidate.setStatus(StatusCode.OUTPUT_TYPES_DO_NOT_MATCH, //
342-
"request=" + to.getTypeName() + ", actual=" + from.getTypeName());
327+
"request=" + refOutType.getTypeName() + ", actual=" + candidateOutType.getTypeName());
343328
return false;
344329
}
345330
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)