Skip to content

Commit b4a514a

Browse files
EfniliteEfnilite
authored
Fix function overloading for convertable arguments (#8046)
1 parent b6974f1 commit b4a514a

File tree

5 files changed

+136
-28
lines changed

5 files changed

+136
-28
lines changed

‎src/main/java/ch/njol/skript/lang/function/FunctionReference.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
import ch.njol.skript.SkriptAPIException;
55
import ch.njol.skript.classes.ClassInfo;
66
import ch.njol.skript.config.Node;
7-
import ch.njol.skript.lang.Expression;
8-
import ch.njol.skript.lang.KeyProviderExpression;
9-
import ch.njol.skript.lang.KeyedValue;
10-
import ch.njol.skript.lang.SkriptParser;
7+
import ch.njol.skript.lang.*;
118
import ch.njol.skript.lang.function.FunctionRegistry.Retrieval;
129
import ch.njol.skript.lang.function.FunctionRegistry.RetrievalResult;
1310
import ch.njol.skript.log.RetainingLogHandler;
@@ -326,7 +323,6 @@ private Function<?> getRegisteredFunction() {
326323
}
327324

328325
Retrieval<Function<?>> attempt = FunctionRegistry.getRegistry().getFunction(script, functionName, parameterTypes);
329-
330326
if (attempt.result() == RetrievalResult.EXACT) {
331327
return attempt.retrieved();
332328
}

‎src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java‎

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,13 @@ record Retrieval<T>(
280280
@NotNull String name,
281281
@NotNull Class<?>... args
282282
) {
283-
if (namespace == null) {
284-
return getFunction(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args));
283+
Retrieval<Function<?>> attempt = null;
284+
if (namespace != null) {
285+
attempt = getFunction(new NamespaceIdentifier(namespace),
286+
FunctionIdentifier.of(name, true, args));
285287
}
286-
287-
Retrieval<Function<?>> attempt = getFunction(new NamespaceIdentifier(namespace),
288-
FunctionIdentifier.of(name, true, args));
289-
if (attempt.result() == RetrievalResult.NOT_REGISTERED) {
290-
return getFunction(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args));
288+
if (attempt == null || attempt.result() == RetrievalResult.NOT_REGISTERED) {
289+
attempt = getFunction(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args));
291290
}
292291
return attempt;
293292
}
@@ -312,7 +311,7 @@ record Retrieval<T>(
312311
return new Retrieval<>(RetrievalResult.NOT_REGISTERED, null, null);
313312
}
314313

315-
Set<FunctionIdentifier> candidates = candidates(provided, existing);
314+
Set<FunctionIdentifier> candidates = candidates(provided, existing, false);
316315
if (candidates.isEmpty()) {
317316
Skript.debug("Failed to find a function for '%s'", provided.name);
318317
return new Retrieval<>(RetrievalResult.NOT_REGISTERED, null, null);
@@ -353,14 +352,45 @@ public Retrieval<Signature<?>> getSignature(
353352
@NotNull String name,
354353
@NotNull Class<?>... args
355354
) {
356-
if (namespace == null) {
357-
return getSignature(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args));
355+
Retrieval<Signature<?>> attempt = null;
356+
if (namespace != null) {
357+
attempt = getSignature(new NamespaceIdentifier(namespace),
358+
FunctionIdentifier.of(name, true, args), false);
359+
}
360+
if (attempt == null || attempt.result() == RetrievalResult.NOT_REGISTERED) {
361+
attempt = getSignature(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args), false);
362+
}
363+
return attempt;
364+
}
365+
366+
/**
367+
* Gets the signature for a function with the given name and arguments. If no local function is found,
368+
* checks for global functions. If {@code namespace} is null, only global signatures will be checked.
369+
* <p>
370+
* This function checks performs no argument conversions, and is only used for determining whether a
371+
* signature already exists with the exact specified arguments. In almost all cases, {@link #getSignature(String, String, Class[])}
372+
* should be used.
373+
* </p>
374+
*
375+
* @param namespace The namespace to get the function from.
376+
* Usually represents the path of the script this function is registered in.
377+
* @param name The name of the function.
378+
* @param args The types of the arguments of the function.
379+
* @return The signature for the function with the given name and argument types, or null if no such function exists.
380+
*/
381+
Retrieval<Signature<?>> getExactSignature(
382+
@Nullable String namespace,
383+
@NotNull String name,
384+
@NotNull Class<?>... args
385+
) {
386+
Retrieval<Signature<?>> attempt = null;
387+
if (namespace != null) {
388+
attempt = getSignature(new NamespaceIdentifier(namespace),
389+
FunctionIdentifier.of(name, true, args), true);
358390
}
359391

360-
Retrieval<Signature<?>> attempt = getSignature(new NamespaceIdentifier(namespace),
361-
FunctionIdentifier.of(name, true, args));
362-
if (attempt.result() == RetrievalResult.NOT_REGISTERED) {
363-
return getSignature(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args));
392+
if (attempt == null || attempt.result() == RetrievalResult.NOT_REGISTERED) {
393+
attempt = getSignature(GLOBAL_NAMESPACE, FunctionIdentifier.of(name, false, args), true);
364394
}
365395
return attempt;
366396
}
@@ -370,10 +400,12 @@ public Retrieval<Signature<?>> getSignature(
370400
*
371401
* @param namespace The namespace to get the function from.
372402
* @param provided The provided identifier of the function.
403+
* @param exact When false, will convert arguments to different types to attempt to find a match.
404+
* When true, will not convert arguments.
373405
* @return The signature for the function with the given name and argument types, or null if no such signature exists
374406
* in the specified namespace.
375407
*/
376-
private Retrieval<Signature<?>> getSignature(@NotNull NamespaceIdentifier namespace, @NotNull FunctionIdentifier provided) {
408+
private Retrieval<Signature<?>> getSignature(@NotNull NamespaceIdentifier namespace, @NotNull FunctionIdentifier provided, boolean exact) {
377409
Preconditions.checkNotNull(namespace, "namespace cannot be null");
378410
Preconditions.checkNotNull(provided, "provided cannot be null");
379411

@@ -383,7 +415,7 @@ private Retrieval<Signature<?>> getSignature(@NotNull NamespaceIdentifier namesp
383415
return new Retrieval<>(RetrievalResult.NOT_REGISTERED, null, null);
384416
}
385417

386-
Set<FunctionIdentifier> candidates = candidates(provided, ns.identifiers.get(provided.name));
418+
Set<FunctionIdentifier> candidates = candidates(provided, ns.identifiers.get(provided.name), exact);
387419
if (candidates.isEmpty()) {
388420
Skript.debug("Failed to find a signature for '%s'", provided.name);
389421
return new Retrieval<>(RetrievalResult.NOT_REGISTERED, null, null);
@@ -415,11 +447,14 @@ private Retrieval<Signature<?>> getSignature(@NotNull NamespaceIdentifier namesp
415447
*
416448
* @param provided The provided function.
417449
* @param existing The existing functions with the same name.
450+
* @param exact When false, will convert arguments to different types to attempt to find a match.
451+
* When true, will not convert arguments.
418452
* @return An unmodifiable list of candidates for the provided function.
419453
*/
420454
private static @Unmodifiable @NotNull Set<FunctionIdentifier> candidates(
421455
@NotNull FunctionIdentifier provided,
422-
Set<FunctionIdentifier> existing
456+
Set<FunctionIdentifier> existing,
457+
boolean exact
423458
) {
424459
Set<FunctionIdentifier> candidates = new HashSet<>();
425460

@@ -459,8 +494,15 @@ private Retrieval<Signature<?>> getSignature(@NotNull NamespaceIdentifier namesp
459494
candidateType = candidate.args[i];
460495
}
461496

462-
if (!Converters.converterExists(provided.args[i], candidateType)) {
463-
continue candidates;
497+
Class<?> providedArg = provided.args[i];
498+
if (exact) {
499+
if (providedArg != candidateType) {
500+
continue candidates;
501+
}
502+
} else {
503+
if (!Converters.converterExists(providedArg, candidateType)) {
504+
continue candidates;
505+
}
464506
}
465507
}
466508

‎src/main/java/ch/njol/skript/lang/function/Functions.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,18 @@ public static JavaFunction<?> registerFunction(JavaFunction<?> function) {
161161
Parameter<?>[] parameters = signature.parameters;
162162

163163
if (parameters.length == 1 && !parameters[0].isSingleValue()) {
164-
existing = FunctionRegistry.getRegistry().getSignature(signature.script, signature.getName(), parameters[0].type.getC().arrayType());
164+
existing = FunctionRegistry.getRegistry().getExactSignature(signature.script, signature.getName(), parameters[0].type.getC().arrayType());
165165
} else {
166166
Class<?>[] types = new Class<?>[parameters.length];
167167
for (int i = 0; i < parameters.length; i++) {
168-
types[i] = parameters[i].type.getC();
168+
if (parameters[i].isSingleValue()) {
169+
types[i] = parameters[i].type.getC();
170+
} else {
171+
types[i] = parameters[i].type.getC().arrayType();
172+
}
169173
}
170174

171-
existing = FunctionRegistry.getRegistry().getSignature(signature.script, signature.getName(), types);
175+
existing = FunctionRegistry.getRegistry().getExactSignature(signature.script, signature.getName(), types);
172176
}
173177

174178
// if this function has already been registered, only allow it if one function is local and one is global.

‎src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package ch.njol.skript.lang.function;
22

33
import ch.njol.skript.SkriptAPIException;
4-
import ch.njol.skript.lang.function.FunctionRegistry.RetrievalResult;
54
import ch.njol.skript.lang.function.FunctionRegistry.FunctionIdentifier;
5+
import ch.njol.skript.lang.function.FunctionRegistry.RetrievalResult;
66
import ch.njol.skript.lang.util.SimpleLiteral;
77
import ch.njol.skript.registrations.DefaultClasses;
8+
import org.bukkit.OfflinePlayer;
9+
import org.bukkit.entity.Player;
810
import org.jetbrains.annotations.Nullable;
911
import org.junit.Test;
1012

@@ -419,4 +421,46 @@ public void testRemoveGlobalScriptFunctions8015() {
419421
assertEquals(RetrievalResult.NOT_REGISTERED, registry.getSignature(null, FUNCTION_NAME).result());
420422
}
421423

424+
private static final Function<Boolean> TEST_FUNCTION_P = new SimpleJavaFunction<>(FUNCTION_NAME,
425+
new Parameter[]{
426+
new Parameter<>("a", DefaultClasses.PLAYER, true, null)
427+
}, DefaultClasses.BOOLEAN, true) {
428+
@Override
429+
public Boolean @Nullable [] executeSimple(Object[][] params) {
430+
return new Boolean[]{true};
431+
}
432+
};
433+
434+
private static final Function<Boolean> TEST_FUNCTION_OP = new SimpleJavaFunction<>(FUNCTION_NAME,
435+
new Parameter[]{
436+
new Parameter<>("a", DefaultClasses.OFFLINE_PLAYER, true, null)
437+
}, DefaultClasses.BOOLEAN, true) {
438+
@Override
439+
public Boolean @Nullable [] executeSimple(Object[][] params) {
440+
return new Boolean[]{true};
441+
}
442+
};
443+
444+
@Test
445+
public void testGetExactSignature() {
446+
assertSame(RetrievalResult.NOT_REGISTERED, registry.getSignature(null, FUNCTION_NAME, Player.class).result());
447+
assertNull(registry.getSignature(null, FUNCTION_NAME, Player.class).retrieved());
448+
assertNull(registry.getFunction(null, FUNCTION_NAME, Player.class).retrieved());
449+
assertSame(RetrievalResult.NOT_REGISTERED, registry.getSignature(null, FUNCTION_NAME, OfflinePlayer.class).result());
450+
assertNull(registry.getSignature(null, FUNCTION_NAME, OfflinePlayer.class).retrieved());
451+
assertNull(registry.getFunction(null, FUNCTION_NAME, OfflinePlayer.class).retrieved());
452+
453+
registry.register(null, TEST_FUNCTION_P);
454+
455+
assertSame(RetrievalResult.EXACT, registry.getExactSignature(null, FUNCTION_NAME, Player.class).result());
456+
assertEquals(TEST_FUNCTION_P.getSignature(), registry.getExactSignature(null, FUNCTION_NAME, Player.class).retrieved());
457+
assertNull(registry.getExactSignature(null, FUNCTION_NAME, OfflinePlayer.class).retrieved());
458+
459+
assertEquals(TEST_FUNCTION_P.getSignature(), registry.getSignature(null, FUNCTION_NAME, Player.class).retrieved());
460+
assertEquals(TEST_FUNCTION_P.getSignature(), registry.getSignature(null, FUNCTION_NAME, OfflinePlayer.class).retrieved());
461+
462+
registry.remove(TEST_FUNCTION_P.getSignature());
463+
registry.remove(TEST_FUNCTION_OP.getSignature());
464+
}
465+
422466
}

‎src/test/skript/tests/misc/function overloading.sk‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,25 @@ parse:
6363
test "function overloading with different return types":
6464
assert size of {FunctionOverloading3::parse::*} = 1
6565
assert {FunctionOverloading3::parse::1} contains "Function 'overloaded3' with the same argument types already exists in script"
66+
67+
function overloading_supertype1(p: offlineplayer):
68+
stop
69+
70+
parse:
71+
results: {FunctionOverloadingSupertype1::parse::*}
72+
code:
73+
function overloading_supertype1(n: player):
74+
stop
75+
76+
function overloading_supertype2(p: player):
77+
stop
78+
79+
parse:
80+
results: {FunctionOverloadingSupertype2::parse::*}
81+
code:
82+
function overloading_supertype2(n: offlineplayer):
83+
stop
84+
85+
test "function overloading with supertype":
86+
assert {FunctionOverloadingSupertype1::parse::*} is not set
87+
assert {FunctionOverloadingSupertype2::parse::*} is not set

0 commit comments

Comments
 (0)