Skip to content

Commit e870dca

Browse files
Treiblesschorlectrueden
Treiblesschorle
authored and
ctrueden
committed
Switch to generic assignability test for outputs
TODO: This test is not implemented yet. For now we will just use raw assignability.
1 parent bacafed commit e870dca

4 files changed

Lines changed: 139 additions & 27 deletions

File tree

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

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@
3030
package org.scijava.ops.matcher;
3131

3232
import java.lang.reflect.Type;
33+
import java.lang.reflect.TypeVariable;
3334
import java.util.ArrayList;
3435
import java.util.Collections;
36+
import java.util.HashMap;
3537
import java.util.List;
3638
import java.util.function.Predicate;
3739

@@ -47,6 +49,7 @@
4749
import org.scijava.struct.Member;
4850
import org.scijava.struct.StructInstance;
4951
import org.scijava.util.Types;
52+
import org.scijava.util.Types.TypeVarInfo;
5053

5154
/**
5255
* Default service for finding ops which match a request.
@@ -134,28 +137,11 @@ public StructInstance<?> match(final OpCandidate candidate) {
134137
*/
135138
@Override
136139
public boolean typesMatch(final OpCandidate candidate) {
137-
if (checkCandidates(Collections.singletonList(candidate)).isEmpty())
138-
return false;
139-
final Type[] refArgTypes = candidate.paddedArgs();
140-
final Type[] candidateArgTypes = OpUtils.inputTypes(candidate);
141-
142-
if (refArgTypes == null)
143-
return true; // no constraints on output types
144-
145-
if (candidateArgTypes.length < refArgTypes.length) {
146-
candidate.setStatus(StatusCode.TOO_FEW_ARGS);
147-
return false;
148-
} else if (candidateArgTypes.length > refArgTypes.length) {
149-
candidate.setStatus(StatusCode.TOO_MANY_ARGS);
140+
HashMap<TypeVariable<?>, TypeVarInfo> typeBounds = new HashMap<>();
141+
if(!inputsMatch(candidate, typeBounds)) {
150142
return false;
151143
}
152-
153-
int conflictingIndex = Types.isApplicable(refArgTypes, candidateArgTypes);
154-
if (conflictingIndex != -1) {
155-
final Type to = refArgTypes[conflictingIndex];
156-
final Type from = candidateArgTypes[conflictingIndex];
157-
candidate.setStatus(StatusCode.ARG_TYPES_DO_NOT_MATCH, //
158-
"request=" + to.getTypeName() + ", actual=" + from.getTypeName());
144+
if(!outputsMatch(candidate, typeBounds)) {
159145
return false;
160146
}
161147
candidate.setStatus(StatusCode.MATCH);
@@ -178,7 +164,7 @@ public boolean typesMatch(final OpCandidate candidate) {
178164
private List<OpCandidate> checkCandidates(final List<OpCandidate> candidates) {
179165
final ArrayList<OpCandidate> validCandidates = new ArrayList<>();
180166
for (final OpCandidate candidate : candidates) {
181-
if (!isValid(candidate) || !outputsMatch(candidate))
167+
if (!isValid(candidate))
182168
continue;
183169
final Type[] args = candidate.paddedArgs();
184170
if (args == null)
@@ -323,14 +309,42 @@ private boolean isValid(final OpCandidate candidate) {
323309
return false;
324310
}
325311

312+
private boolean inputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
313+
if (checkCandidates(Collections.singletonList(candidate)).isEmpty())
314+
return false;
315+
final Type[] refArgTypes = candidate.paddedArgs();
316+
final Type[] candidateArgTypes = OpUtils.inputTypes(candidate);
317+
318+
if (refArgTypes == null)
319+
return true; // no constraints on output types
320+
321+
if (candidateArgTypes.length < refArgTypes.length) {
322+
candidate.setStatus(StatusCode.TOO_FEW_ARGS);
323+
return false;
324+
} else if (candidateArgTypes.length > refArgTypes.length) {
325+
candidate.setStatus(StatusCode.TOO_MANY_ARGS);
326+
return false;
327+
}
328+
329+
int conflictingIndex = Types.isApplicable(refArgTypes, candidateArgTypes, typeBounds);
330+
if (conflictingIndex != -1) {
331+
final Type to = refArgTypes[conflictingIndex];
332+
final Type from = candidateArgTypes[conflictingIndex];
333+
candidate.setStatus(StatusCode.ARG_TYPES_DO_NOT_MATCH, //
334+
"request=" + to.getTypeName() + ", actual=" + from.getTypeName());
335+
return false;
336+
}
337+
return true;
338+
}
339+
326340
/**
327341
* Checks whether the output types of the candidate satisfy the output types of the {@link OpRef}.
328342
* Sets candidate status code if there are too many, to few, or not matching types.
329343
*
330344
* @param candidate the candidate to check outputs for
331345
* @return whether the output types are satisfied
332346
*/
333-
private boolean outputsMatch(final OpCandidate candidate) {
347+
private boolean outputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
334348
final Type[] refOutTypes = candidate.getRef().getOutTypes();
335349
if (refOutTypes == null)
336350
return true; // no constraints on output types
@@ -344,7 +358,7 @@ private boolean outputsMatch(final OpCandidate candidate) {
344358
return false;
345359
}
346360

347-
int conflictingIndex = Types.isApplicable(candidateOutTypes, refOutTypes);
361+
int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(candidateOutTypes, refOutTypes, typeBounds);
348362
if (conflictingIndex != -1) {
349363
final Type to = refOutTypes[conflictingIndex];
350364
final Type from = candidateOutTypes[conflictingIndex];

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444

4545
import org.scijava.ops.types.Nil;
4646
import org.scijava.util.Types;
47+
import org.scijava.util.Types.TypeVarInfo;
4748

4849
import com.google.common.base.Objects;
4950

@@ -53,6 +54,64 @@ private MatchingUtils() {
5354
// prevent instantiation of utility class
5455
}
5556

57+
/**
58+
* Checks for raw assignability. TODO This method is not yet fully
59+
* implemented. The correct behavior should be as follows. Suppose we have a
60+
* generic typed method like:
61+
*
62+
* <pre>
63+
*public static &lt;N&gt; List&lt;N&gt; foo(N in) {
64+
* ...
65+
*}
66+
* </pre>
67+
*
68+
* This method should discern if the following assignments would be legal,
69+
* possibly using predetermined {@link TypeVariable} assignments:
70+
*
71+
* <pre>
72+
*List&lt;Integer&gt; listOfInts = foo(new Integer(0)) //legal
73+
*List&lt;Number&gt; listOfNumbers = foo(new Integer(0)) //legal
74+
*List&lt;? extends Number&gt; listOfBoundedWildcards = foo(new Integer(0)) //legal
75+
* </pre>
76+
*
77+
* The corresponding calls to this method would be:
78+
*
79+
* <pre>
80+
* Nil&lt;List&lt;N&gt;&gt; nilN = new Nil&lt;List&lt;N&gt;&gt;(){}
81+
* Nil&lt;List&lt;Integer&gt;&gt; nilInteger = new Nil&lt;List&lt;Integer&gt;&gt;(){}
82+
* Nil&lt;List&lt;Number&gt;&gt; nilNumber = new Nil&lt;List&lt;Number&gt;&gt;(){}
83+
* Nil&lt;List&lt;? extends Number&gt;&gt; nilWildcardNumber = new Nil&lt;List&lt;? extends Number&gt;&gt;(){}
84+
*
85+
* checkGenericOutputsAssignability(nilN.getType(), nilInteger.getType, ...)
86+
* checkGenericOutputsAssignability(nilN.getType(), nilNumber.getType, ...)
87+
* checkGenericOutputsAssignability(nilN.getType(), nilWildcardNumber.getType, ...)
88+
* </pre>
89+
*
90+
* Using a map where N was already bound to Integer (N -> Integer.class).
91+
* This method is useful for the following scenario: During ops matching, we
92+
* first check if the arguments (inputs) of the requested op are applicable
93+
* to the arguments of an op candidate. During this process, possible type
94+
* variables may be inferred. The can then be used with this method to find
95+
* out if the outputs of the op candidate would be assignable to the output
96+
* of the requested op.
97+
*
98+
* @param froms
99+
* @param tos
100+
* @param typeBounds
101+
* @return
102+
*/
103+
public static int checkGenericOutputsAssignability(Type[] froms, Type[] tos,
104+
HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
105+
for (int i = 0; i < froms.length; i++) {
106+
Type from = froms[i];
107+
Type to = tos[i];
108+
109+
if (!Types.isAssignable(Types.raw(from), Types.raw(to)))
110+
return i;
111+
}
112+
return -1;
113+
}
114+
56115
/**
57116
* Checks whether it would be legal to assign the {@link ParameterizedType}
58117
* source, represented as a raw type, to the specified
@@ -309,7 +368,7 @@ public static Type[] getParams(Class<?> subType, Class<?> superErasure) {
309368
}
310369
return new Type[0];
311370
}
312-
371+
313372
/**
314373
* Gets the "useful" class information carries on the given object, which
315374
* depends on the actual type of the object.

‎src/main/java/org/scijava/util/Types.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,7 @@ private static Class<?> arrayOrNull(final Class<?> componentType) {
13341334
/**
13351335
* maintains info about an {@link TypeVariable}.
13361336
*/
1337-
private static class TypeVarInfo {
1337+
public static class TypeVarInfo {
13381338

13391339
private final TypeVariable<?> var;
13401340
private final Type[] upperBounds;

‎src/test/java/org/scijava/ops/util/MatchingUtilsTest.java‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import java.lang.reflect.ParameterizedType;
3535
import java.lang.reflect.Type;
36+
import java.util.HashMap;
3637
import java.util.List;
3738
import java.util.function.BiFunction;
3839
import java.util.function.Function;
@@ -65,13 +66,14 @@ private void assertAll(Class<?> from, boolean condition, Type... tos) {
6566
public void genericAssignabilitySingleVar() {
6667
abstract class Single<I> implements Supplier<I> {
6768
}
68-
6969
Nil<Supplier<Double>> y1 = new Nil<Supplier<Double>>() {
7070
};
71+
Nil<Supplier<Number>> y2 = new Nil<Supplier<Number>>() {
72+
};
7173
Nil<Double> n1 = new Nil<Double>() {
7274
};
7375

74-
assertAll(Single.class, true, y1);
76+
assertAll(Single.class, true, y1, y2);
7577
assertAll(Single.class, false, n1);
7678
}
7779

@@ -250,6 +252,12 @@ abstract class SingleVarBoundedNestedWildcardMultipleOccurenceUsedNested<I exten
250252

251253
assertAll(SingleVarBoundedNestedWildcardMultipleOccurenceUsedNested.class, true, y5);
252254
assertAll(SingleVarBoundedNestedWildcardMultipleOccurenceUsedNested.class, false, n5);
255+
256+
abstract class SingleVarMultipleOccurenceUsedNested<I> implements Function<I, List<I>> {}
257+
258+
Nil<Function<Integer, List<Number>>> n6 = new Nil<Function<Integer,List<Number>>>() {
259+
};
260+
assertAll(SingleVarMultipleOccurenceUsedNested.class, false, n6);
253261
}
254262

255263
@Test
@@ -416,4 +424,35 @@ public <T extends Number> void testIsAssignableT() {
416424
assertAll(List.class, true, listT, listNumber, listInteger);
417425
assertAll(List.class, false, listExtendsNumber, t);
418426
}
427+
428+
/**
429+
* {@link MatchingUtils#checkGenericOutputsAssignability(Type[], Type[], HashMap)} not yet fully
430+
* implemented. If this is done, all the tests below should not fail.
431+
*/
432+
@Test
433+
public <N> void testOutputAssignability() {
434+
// Nil<N> n = new Nil<N>() {};
435+
// Nil<List<N>> ln = new Nil<List<N>>() {};
436+
// Nil<List<? extends Number>> lWildNum = new Nil<List<? extends Number>>() {};
437+
// Nil<List<Number>> lNum = new Nil<List<Number>>() {};
438+
// Nil<List<?>> lwild = new Nil<List<?>>() {};
439+
//
440+
// HashMap<TypeVariable<?>, TypeVarInfo> typeBounds = new HashMap<>();
441+
// assertTrue(-1 == Types.isApplicable(new Type[]{Integer.class}, new Type[]{n.getType()}, typeBounds));
442+
// Type[] toOuts = new Type[]{lWildNum.getType()};
443+
// Type[] fromOuts = new Type[]{ln.getType()};
444+
// assertTrue(-1 == MatchingUtils.checkGenericOutputsAssignability(fromOuts, toOuts, typeBounds));
445+
//
446+
// toOuts = new Type[]{lNum.getType()};
447+
// assertTrue(-1 == MatchingUtils.checkGenericOutputsAssignability(fromOuts, toOuts, typeBounds));
448+
//
449+
// toOuts = new Type[]{lwild.getType()};
450+
// assertTrue(-1 == MatchingUtils.checkGenericOutputsAssignability(fromOuts, toOuts, typeBounds));
451+
//
452+
// typeBounds = new HashMap<>();
453+
// assertTrue(-1 == Types.isApplicable(new Type[]{String.class}, new Type[]{n.getType()}, typeBounds));
454+
// toOuts = new Type[]{lWildNum.getType()};
455+
// fromOuts = new Type[]{ln.getType()};
456+
// assertFalse(-1 == MatchingUtils.checkGenericOutputsAssignability(fromOuts, toOuts, typeBounds));
457+
}
419458
}

0 commit comments

Comments
 (0)