More testing and adjustments

Author: APickledWalrus

src/main/java/ch/njol/skript/expressions/ExprCommandInfo.java

@@ -12,6 +12,7 @@
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandMap;
 import org.bukkit.command.PluginCommand;
+import org.bukkit.command.PluginIdentifiableCommand;
 import org.bukkit.command.defaults.BukkitCommand;
 import org.bukkit.event.Event;
 import org.bukkit.event.player.PlayerCommandPreprocessEvent;
@@ -62,8 +63,8 @@ private enum InfoType {
 		PERMISSION(Command::getPermission),
 		PERMISSION_MESSAGE(Command::getPermissionMessage),
 		PLUGIN(command -> {
-			if (command instanceof PluginCommand) {
-				return ((PluginCommand) command).getPlugin().getName();
+			if (command instanceof PluginIdentifiableCommand pluginCommand) {
+				return pluginCommand.getPlugin().getName();
 			} else if (command instanceof BukkitCommand) {
 				return "Bukkit";
 			} else if (command.getClass().getPackage().getName().startsWith("org.spigot")) {

src/main/java/org/skriptlang/skript/bukkit/command/elements/structures/util/SubCommandEntryData.java

@@ -84,8 +84,7 @@ protected List<String> getValue(String value) {
 			}
 		})
 		.addEntry("description", null, true)
-		// TODO this was previously a VariableString. need to handle that
-		.addEntry("usage", null, true)
+		.addEntryData(new VariableStringEntryData("usage", null, true))
 		.addEntry("prefix", null, true)
 		.addEntry("permission", null, true)
 		.addEntryData(new KeyValueEntryData<ExecutableBy>("executable by", null, true) {
@@ -188,10 +187,16 @@ public SubCommandEntryData(String key, boolean optional, boolean multiple) {
 		}
 
 		// command usage
-		String usage = entryContainer.getOptional("usage", String.class, false);
-		if (!isRoot && usage != null) {
-			Skript.error("Only the root of a command may have a usage.");
-			return null;
+		String usage = null;
+		VariableString variableUsage = entryContainer.getOptional("usage", VariableString.class, false);
+		if (variableUsage != null) {
+			if (!isRoot) {
+				Skript.error("Only the root of a command may have a usage.");
+				return null;
+			}
+			if (variableUsage.isSimple()) {
+				usage = variableUsage.toString(null);
+			}
 		}
 
 		// command prefix (custom namespace)
@@ -319,7 +324,6 @@ public SubCommandEntryData(String key, boolean optional, boolean multiple) {
 		}
 
 		// attach subcommand pieces
-		// TODO verify error behavior...
 		List<ArgumentBuilder<CommandSourceStack, ?>> subcommands =
 			entryContainer.getAll("subcommand", Result.class, false).stream()
 				.flatMap(result -> result.arguments().stream())
@@ -398,7 +402,6 @@ public boolean canCreateWith(Node node) {
 
 		// this is intentionally placed AFTER iterating over the children
 		// for conflicting command arguments, the argument at this level should be preferred over a subcommand's argument
-		// TODO there is actually more complexity here to handle
 		// it is not guaranteed to conflict if two arguments are at the same level
 		// e.g. Player-then-Number conflicts but Number-then-Player doesn't
 		if (commandElement.isLeaf()) {

src/test/skript/tests/bukkit/command/StructCommand.sk

@@ -3,11 +3,17 @@ using local variable type hints
 options:
 	command: skriptcommand
 
-# expect error for invalid executable by for subcommand
+# expect error for missing trigger given no subcommands
 parse:
 	results: {-StructCommand::entries::1::*}
 	code:
 		command /entries_test_1:
+
+# expect error for invalid executable by for subcommand
+parse:
+	results: {-StructCommand::entries::2::*}
+	code:
+		command /entries_test_2:
 			executable by: players
 			trigger:
 				stop
@@ -16,6 +22,13 @@ parse:
 				trigger:
 					stop
 
+# expect error for invalid subcommand and subsequently missing trigger (given no valid subcommands)
+parse:
+	results: {-StructCommand::entries::3::*}
+	code:
+		command /entries_test_3:
+			subcommand test:
+
 # expect error for duplicate argument names
 parse:
 	results: {-StructCommand::args::1::*}
@@ -64,6 +77,14 @@ parse:
 			trigger:
 				set {_a::*} to {_x::*} in lowercase
 
+command /structcommand:
+	aliases: /structcommand2
+	description: This is the description
+	usage: "This is the usage"
+	permission: command.structcommand
+	trigger:
+		stop
+
 command /test-subcommand <number>:
 	subcommand <number>:
 		trigger:
@@ -96,7 +117,10 @@ command /commandtest <string="player">:
 
 test "StructCommand":
 	# parsing
-	assert {-StructCommand::entries::1::*} is "It is not possible to restrict execution to the console as the parent command is only executable by players."
+	set {_error_missing_trigger} to "You must have a 'trigger' entry if there are no subcommands!"
+	assert {-StructCommand::entries::1::*} is {_error_missing_trigger}
+	assert {-StructCommand::entries::2::*} is "It is not possible to restrict execution to the console as the parent command is only executable by players."
+	assert {-StructCommand::entries::3::*} is {_error_missing_trigger} and {_error_missing_trigger}
 	assert {-StructCommand::args::1::*} is "The argument name 'x' was already used."
 	assert {-StructCommand::args::2::*} is "text arguments do not support minimum or maximum values."
 	assert {-StructCommand::args::3::*} is not set
@@ -105,6 +129,15 @@ test "StructCommand":
 	assert {-StructCommand::hints::1::*} is "Expected variable '{_x}' to be a text, but it is a number" with "Hint failed (%{StructCommand::hints::1::*}%)"
 	assert {-StructCommand::hints::2::*} is "Expected variable '{_x::*}' to be a text, but it is a number" with "Hint failed (%{StructCommand::hints::2::*}%)"
 
+	# entry validation
+	assert the main command name of the command "structcommand" is "structcommand"
+	assert the description of the command "structcommand" is "This is the description"
+	assert the label of the command "structcommand" is "structcommand"
+	#assert the usage of the command "structcommand" is "This is the usage"
+	assert the aliases of the command "structcommand" are "structcommand2" and "skript:structcommand2"
+	#assert the permission of the command "structcommand" is "skript.structcommand"
+	assert the plugin of the command "structcommand" is "Skript"
+
 	# execution
 	execute command "test-subcommand 10"
 	assert {-test-subcommand} is 10 with "root command execution failed"