Skip to content
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

Jump to bottom

Throw CommandSyntaxExceptions when ListArgument input is wrong #324

Merged
merged 3 commits into from
Aug 10, 2022
Merged

Throw CommandSyntaxExceptions when ListArgument input is wrong #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b925e57
Throw CommandSyntaxExceptions when ListArgument input is wrong
willkroboth Aug 7, 2022
bff38fa
Use ArgumentTests#assertInvalidSyntax() instead of assertNull()
willkroboth Aug 8, 2022
c8b12ee
Use custom exception method when item is not allowed in list
willkroboth Aug 10, 2022
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
23 changes: 20 additions & 3 deletions commandapi-core/src/main/java/dev/jorel/commandapi/arguments/ListArgument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
package dev.jorel.commandapi.arguments;

import com.mojang.brigadier.LiteralMessage;
import com.mojang.brigadier.StringReader;
import com.mojang.brigadier.arguments.StringArgumentType;
import com.mojang.brigadier.context.CommandContext;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
import dev.jorel.commandapi.IStringTooltip;
import dev.jorel.commandapi.StringTooltip;
import dev.jorel.commandapi.nms.NMS;
Expand Down Expand Up @@ -130,18 +132,33 @@ public <CommandListenerWrapper> List<T> parseArgument(NMS<CommandListenerWrapper

// If the argument's value is in the list of values, include it
List<T> list = new ArrayList<>();
String[] strArr = cmdCtx.getArgument(key, String.class).split(Pattern.quote(delimiter));
String argument = cmdCtx.getArgument(key, String.class);
String[] strArr = argument.split(Pattern.quote(delimiter));
StringReader context = new StringReader(argument);
int cursor = 0;
for (String str : strArr) {
boolean addedItem = false;
// Yes, this isn't an instant lookup HashMap, but this is the best we can do
for (IStringTooltip value : values.keySet()) {
if (value.getSuggestion().equals(str)) {
if (allowDuplicates) {
list.add(values.get(value));
} else if (!list.contains(values.get(value))) {
list.add(values.get(value));
} else {
if (!list.contains(values.get(value))) {
list.add(values.get(value));
} else {
context.setCursor(cursor);
throw new SimpleCommandExceptionType(new LiteralMessage("Duplicate arguments are not allowed")).createWithContext(context);
}
}
addedItem = true;
}
}
if(!addedItem) {
context.setCursor(cursor);
throw new SimpleCommandExceptionType(new LiteralMessage("Item is not allowed in list")).createWithContext(context);
}
cursor += str.length() + delimiter.length();
}
return list;
}
Expand Down
62 changes: 26 additions & 36 deletions commandapi-plugin-test/src/test/java/ArgumentTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
Expand All @@ -16,12 +9,14 @@
import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.advancement.Advancement;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.potion.PotionEffectType;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;

import com.mojang.brigadier.exceptions.CommandSyntaxException;

Expand Down Expand Up @@ -65,6 +60,10 @@ private String getDispatcherString() {
}
}

public void assertInvalidSyntax(CommandSender sender, String command) {
assertThrows(CommandSyntaxException.class, () -> assertTrue(server.dispatchThrowableCommand(sender,command)));
}

@BeforeEach
public void setUp() {
server = MockBukkit.mock(new CustomServerMock());
Expand Down Expand Up @@ -143,8 +142,8 @@ public void executionTestWithStringArgument() {
assertEquals("success Hello_world", player.nextMessage());

// Negative tests from the documentation
assertThrows(CommandSyntaxException.class, () -> assertTrue(server.dispatchThrowableCommand(player, "test hello@email.com")));
assertThrows(CommandSyntaxException.class, () -> assertTrue(server.dispatchThrowableCommand(player, "test yesn't")));
assertInvalidSyntax(player, "test hello@email.com");
assertInvalidSyntax(player, "test yesn't");
}

@Test
Expand All @@ -162,7 +161,7 @@ public void executionTestWithBooleanArgument() {
server.dispatchCommand(player, "test false");
assertEquals("success true", player.nextMessage());
assertEquals("success false", player.nextMessage());
assertThrows(CommandSyntaxException.class, () -> server.dispatchThrowableCommand(player, "test aaaaa"));
assertInvalidSyntax(player, "test aaaaa");
}

@Test
Expand Down Expand Up @@ -316,7 +315,7 @@ public void executionTestWithPotionEffectArgument() {
assertEquals(PotionEffectType.SPEED, type.get());
assertEquals(null, type.get());
}

@SuppressWarnings("unchecked")
@Test
public void executionTestWithListArgument() {
Expand All @@ -336,13 +335,11 @@ public void executionTestWithListArgument() {
})
.register();

server.dispatchCommand(sender, "list cat, wolf, axolotl");
server.dispatchCommand(sender, "list cat, wolf, axolotl, chicken");
server.dispatchCommand(sender, "list axolotl, wolf, chicken, axolotl");
server.dispatchCommand(sender, "list cat, wolf, axolotl"); // normal list
assertInvalidSyntax(sender, "list cat, wolf, axolotl, wolf"); // don't allow duplicates
assertInvalidSyntax(sender, "list axolotl, wolf, chicken, cat"); // don't allow unknown items

assertEquals(List.of("cat", "wolf", "axolotl"), type.get());
assertEquals(List.of("cat", "wolf", "axolotl"), type.get());
assertEquals(List.of("axolotl", "wolf"), type.get());

// List argument, with duplicates

Expand All @@ -357,13 +354,10 @@ public void executionTestWithListArgument() {
})
.register();

server.dispatchCommand(sender, "listdup cat, wolf, axolotl, cat, wolf");
server.dispatchCommand(sender, "listdup cat, wolf, axolotl, chicken, cat");
server.dispatchCommand(sender, "listdup axolotl, wolf, chicken, axolotl, axolotl, axolotl, axolotl, axolotl, wolf");
server.dispatchCommand(sender, "listdup cat, wolf, axolotl, cat, wolf"); // allow duplicates
assertInvalidSyntax(sender, "listdup cat, wolf, axolotl, chicken, cat"); // don't allow unknown items

assertEquals(List.of("cat", "wolf", "axolotl", "cat", "wolf"), type.get());
assertEquals(List.of("cat", "wolf", "axolotl", "cat"), type.get());
assertEquals(List.of("axolotl", "wolf", "axolotl", "axolotl", "axolotl", "axolotl", "axolotl", "wolf"), type.get());

// List argument, with a constant list (not using a supplier)

Expand All @@ -377,13 +371,11 @@ public void executionTestWithListArgument() {
})
.register();

server.dispatchCommand(sender, "listconst cat, wolf, axolotl");
server.dispatchCommand(sender, "listconst cat, wolf, axolotl, chicken");
server.dispatchCommand(sender, "listconst axolotl, wolf, chicken, axolotl");
server.dispatchCommand(sender, "listconst cat, wolf, axolotl"); // normal list
assertInvalidSyntax(sender, "listconst cat, wolf, axolotl, wolf"); // don't allow duplicates
assertInvalidSyntax(sender, "listconst axolotl, wolf, chicken, cat"); // don't allow unknown items

assertEquals(List.of("cat", "wolf", "axolotl"), type.get());
assertEquals(List.of("cat", "wolf", "axolotl"), type.get());
assertEquals(List.of("axolotl", "wolf"), type.get());

// List argument using a function

Expand All @@ -396,18 +388,16 @@ public void executionTestWithListArgument() {
type.set((List<String>) args[0]);
})
.register();

server.dispatchCommand(sender, "listfunc cat, wolf, axolotl");
server.dispatchCommand(sender, "listfunc cat, wolf, axolotl, chicken");
server.dispatchCommand(sender, "listfunc axolotl, wolf, chicken, axolotl");
server.dispatchCommand(sender, "listfunc axolotl, wolf, chicken, axolotl, " + sender.getName());

assertEquals(List.of("cat", "wolf", "axolotl"), type.get());

server.dispatchCommand(sender, "listfunc cat, wolf, axolotl"); // normal list
assertInvalidSyntax(sender, "listfunc cat, wolf, axolotl, wolf"); // don't allow duplicates
assertInvalidSyntax(sender, "listfunc axolotl, wolf, chicken, cat"); // don't allow unknown items
server.dispatchCommand(sender, "listfunc axolotl, wolf, " + sender.getName()); // sender name

assertEquals(List.of("cat", "wolf", "axolotl"), type.get());
assertEquals(List.of("axolotl", "wolf"), type.get());
assertEquals(List.of("axolotl", "wolf", sender.getName()), type.get());
}

@Test
public void executionTestWithPlayerArgument() {
Mut<Player> type = Mut.of();
Expand All @@ -421,7 +411,7 @@ public void executionTestWithPlayerArgument() {

PlayerMock player = server.addPlayer("APlayer");
server.dispatchCommand(player, "test APlayer");
assertThrows(CommandSyntaxException.class, () -> server.dispatchThrowableCommand(player, "test BPlayer"));
assertInvalidSyntax(player, "test BPlayer");
assertEquals(player, type.get());
assertEquals(null, type.get());
}
Expand Down