Skip to content

Fix bad FunctionRegistry#remove logic #8015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 13, 2025
Merged

Fix bad FunctionRegistry#remove logic #8015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6a4b53
fix bad remove logic
Efnilite Jul 8, 2025
9d9d6ab
update to pr
Efnilite Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,34 @@ public static FunctionRegistry getRegistry() {

/**
* Registers a signature.
* <p>
* Attempting to register a local signature in the global namespace, or a global signature in
* a local namespace, will throw an {@link IllegalArgumentException}.
* </p>
*
* @param namespace The namespace to register the signature in.
* If namespace is null, will register this signature globally.
* Usually represents the path of the script this signature is registered in.
* @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;
Expand Down Expand Up @@ -118,17 +131,29 @@ public void register(@NotNull Function<?> function) {

/**
* Registers a function.
* <p>
* Attempting to register a local function in the global namespace, or a global function in
* a local namespace, will throw an {@link IllegalArgumentException}.
* </p>
*
* @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();
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/ch/njol/skript/lang/function/Functions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
31 changes: 31 additions & 0 deletions src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Boolean> signature = new Signature<>(TEST_SCRIPT, FUNCTION_NAME, new Parameter<?>[0],
false, DefaultClasses.BOOLEAN, true, "");
SimpleJavaFunction<Boolean> 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());
}

}