diff --git a/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java b/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java index 271895c9993..b2fe0d95273 100644 --- a/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java +++ b/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java @@ -64,6 +64,10 @@ public static FunctionRegistry getRegistry() { /** * Registers a signature. + *

+ * Attempting to register a local signature in the global namespace, or a global signature in + * a local namespace, will throw an {@link IllegalArgumentException}. + *

* * @param namespace The namespace to register the signature in. * If namespace is null, will register this signature globally. @@ -71,14 +75,23 @@ public static FunctionRegistry getRegistry() { * @param signature The signature to register. * @throws SkriptAPIException if a signature with the same name and parameters is already registered * in this namespace. + * @throws IllegalArgumentException if the signature is global and namespace is not null, or + * if the signature is local and namespace is null. */ public void register(@Nullable String namespace, @NotNull Signature signature) { Preconditions.checkNotNull(signature, "signature cannot be null"); + if (signature.isLocal() && namespace == null) { + throw new IllegalArgumentException("Cannot register a local signature in the global namespace"); + } + if (!signature.isLocal() && namespace != null) { + throw new IllegalArgumentException("Cannot register a global signature in a local namespace"); + } + Skript.debug("Registering signature '%s'", signature.getName()); // namespace NamespaceIdentifier namespaceId; - if (namespace != null && signature.isLocal()) { + if (namespace != null) { namespaceId = new NamespaceIdentifier(namespace); } else { namespaceId = GLOBAL_NAMESPACE; @@ -118,17 +131,29 @@ public void register(@NotNull Function function) { /** * Registers a function. + *

+ * Attempting to register a local function in the global namespace, or a global function in + * a local namespace, will throw an {@link IllegalArgumentException}. + *

* * @param namespace The namespace to register the function in. - * If namespace is null, will register this function globally. + * If namespace is null, will register this function globally, only if the function is global. * Usually represents the path of the script this function is registered in. * @param function The function to register. - * @throws SkriptAPIException if the function name is invalid or if - * a function with the same name and parameters is already registered - * in this namespace. + * @throws SkriptAPIException if the function name is invalid or if + * a function with the same name and parameters is already registered + * in this namespace. + * @throws IllegalArgumentException if the function is global and namespace is not null, or + * if the function is local and namespace is null. */ public void register(@Nullable String namespace, @NotNull Function function) { Preconditions.checkNotNull(function, "function cannot be null"); + if (function.getSignature().isLocal() && namespace == null) { + throw new IllegalArgumentException("Cannot register a local function in the global namespace"); + } + if (!function.getSignature().isLocal() && namespace != null) { + throw new IllegalArgumentException("Cannot register a global function in a local namespace"); + } Skript.debug("Registering function '%s'", function.getName()); String name = function.getName(); @@ -138,7 +163,7 @@ public void register(@Nullable String namespace, @NotNull Function function) // namespace NamespaceIdentifier namespaceId; - if (namespace != null && function.getSignature().isLocal()) { + if (namespace != null) { namespaceId = new NamespaceIdentifier(namespace); } else { namespaceId = GLOBAL_NAMESPACE; @@ -487,8 +512,13 @@ public void remove(@NotNull Signature signature) { String name = signature.getName(); FunctionIdentifier identifier = FunctionIdentifier.of(signature); - Namespace namespace = namespaces.getOrDefault(new NamespaceIdentifier(signature.script), - namespaces.get(GLOBAL_NAMESPACE)); + Namespace namespace; + if (signature.isLocal()) { + namespace = namespaces.get(new NamespaceIdentifier(signature.script)); + } else { + namespace = namespaces.get(GLOBAL_NAMESPACE); + } + if (namespace == null) { return; } @@ -532,6 +562,7 @@ private record NamespaceIdentifier(@Nullable String name) { /** * Returns whether this identifier is for local namespaces. + * * @return Whether this identifier is for local namespaces. */ public boolean local() { diff --git a/src/main/java/ch/njol/skript/lang/function/Functions.java b/src/main/java/ch/njol/skript/lang/function/Functions.java index b9a40367189..b03a2195fd7 100644 --- a/src/main/java/ch/njol/skript/lang/function/Functions.java +++ b/src/main/java/ch/njol/skript/lang/function/Functions.java @@ -104,7 +104,11 @@ public static JavaFunction registerFunction(JavaFunction function) { namespace.addFunction(function); } - FunctionRegistry.getRegistry().register(script.getConfig().getFileName(), function); + if (function.getSignature().isLocal()) { + FunctionRegistry.getRegistry().register(script.getConfig().getFileName(), function); + } else { + FunctionRegistry.getRegistry().register(null, function); + } return function; } diff --git a/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java b/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java index 99b7208c3a0..80662ab3702 100644 --- a/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java +++ b/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java @@ -388,4 +388,35 @@ public void testIdentifierSignatureOf() { assertEquals(FunctionIdentifier.of(function2.getSignature()), identifier); } + // see https://github.com/SkriptLang/Skript/pull/8015 + @Test + public void testRemoveGlobalScriptFunctions8015() { + // create empty TEST_SCRIPT namespace such that it is not null + registry.register(TEST_SCRIPT, LOCAL_TEST_FUNCTION); + registry.remove(LOCAL_TEST_FUNCTION.getSignature()); + + assertEquals(RetrievalResult.NOT_REGISTERED, registry.getSignature(TEST_SCRIPT, FUNCTION_NAME).result()); + + // construct a global function with a non-null script, which happens in script functions + Signature signature = new Signature<>(TEST_SCRIPT, FUNCTION_NAME, new Parameter[0], + false, DefaultClasses.BOOLEAN, true, ""); + SimpleJavaFunction fn = new SimpleJavaFunction<>(signature) { + @Override + public Boolean @Nullable [] executeSimple(Object[][] params) { + return new Boolean[] { true }; + } + }; + + // ensure new behaviour + assertThrows(IllegalArgumentException.class, () -> registry.register(TEST_SCRIPT, fn)); + + registry.register(null, fn); + + assertEquals(RetrievalResult.EXACT, registry.getSignature(null, FUNCTION_NAME).result()); + + registry.remove(signature); + + assertEquals(RetrievalResult.NOT_REGISTERED, registry.getSignature(null, FUNCTION_NAME).result()); + } + }