Fix lore command index handling and default-command typo

CHANGELOG.md

@@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Fixed
+- `/pl edit` and `/pl remove` now treat `lineIndex` as 1-based, matching `COMMANDS.md`/`USER_GUIDE.md`, instead of silently indexing into the lore list as 0-based
+- `/pl edit` and `/pl remove` no longer throw an uncaught exception when given a non-numeric index or no index at all; they now send a player-facing error message
+- The default (no-argument) command no longer tells players to type the non-existent `/lp help`; it now correctly says `/pl help`
+
 ## [1.1]
 
 ### Added

pom.xml

@@ -14,6 +14,7 @@
     <properties>
         <java.version>1.8</java.version>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <junit.version>5.10.2</junit.version>
     </properties>
 
     <build>
@@ -27,6 +28,11 @@
                     <target>${java.version}</target>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.2.5</version>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-shade-plugin</artifactId>
@@ -80,5 +86,17 @@
             <version>1.0</version>
             <scope>compile</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>${junit.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>5.11.0</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

src/main/java/dansplugins/playerlore/commands/DefaultCommand.java

@@ -27,7 +27,7 @@ public boolean execute(CommandSender commandSender) {
         commandSender.sendMessage(ChatColor.AQUA + "Requested by: Rochelle");
         commandSender.sendMessage(ChatColor.AQUA + "Wiki: https://github.com/dmccoystephenson/PlayerLore/wiki");
         commandSender.sendMessage("");
-        commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /lp help");
+        commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /pl help");
         return true;
     }
 

src/main/java/dansplugins/playerlore/commands/EditCommand.java

@@ -38,7 +38,18 @@ public boolean execute(CommandSender commandSender, String[] args) {
         Player player = (Player) commandSender;
 
         // get line to edit
-        int lineIndex = Integer.parseInt(args[0]);
+        if (args.length == 0) {
+            player.sendMessage(ChatColor.RED + "Usage: /pl edit (lineIndex) \"new line of lore\"");
+            return false;
+        }
+
+        int lineIndex;
+        try {
+            lineIndex = Integer.parseInt(args[0]);
+        } catch (NumberFormatException e) {
+            player.sendMessage(ChatColor.RED + "Line index must be a number.");
+            return false;
+        }
 
         // get line of lore
         ArgumentParser argumentParser = new ArgumentParser();
@@ -69,12 +80,12 @@ public boolean execute(CommandSender commandSender, String[] args) {
             lore = new ArrayList<>();
         }
 
-        if (lineIndex >= lore.size()) {
+        if (lineIndex < 1 || lineIndex > lore.size()) {
             player.sendMessage(ChatColor.RED + "There aren't that many lines of lore.");
             return false;
         }
 
-        lore.set(lineIndex, ChatColor.WHITE + lineOfLore);
+        lore.set(lineIndex - 1, ChatColor.WHITE + lineOfLore);
         itemMeta.setLore(lore);
         item.setItemMeta(itemMeta);
 

src/main/java/dansplugins/playerlore/commands/RemoveCommand.java

@@ -37,7 +37,18 @@ public boolean execute(CommandSender commandSender, String[] args) {
         Player player = (Player) commandSender;
 
         // get line to edit
-        int lineIndex = Integer.parseInt(args[0]);
+        if (args.length == 0) {
+            player.sendMessage(ChatColor.RED + "Usage: /pl remove (lineIndex)");
+            return false;
+        }
+
+        int lineIndex;
+        try {
+            lineIndex = Integer.parseInt(args[0]);
+        } catch (NumberFormatException e) {
+            player.sendMessage(ChatColor.RED + "Line index must be a number.");
+            return false;
+        }
 
         // get item
         ItemStack item = player.getInventory().getItemInMainHand();
@@ -59,12 +70,12 @@ public boolean execute(CommandSender commandSender, String[] args) {
             lore = new ArrayList<>();
         }
 
-        if (lineIndex >= lore.size()) {
+        if (lineIndex < 1 || lineIndex > lore.size()) {
             player.sendMessage(ChatColor.RED + "There aren't that many lines of lore.");
             return false;
         }
 
-        lore.remove(lineIndex);
+        lore.remove(lineIndex - 1);
         itemMeta.setLore(lore);
         item.setItemMeta(itemMeta);
 

src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java

@@ -0,0 +1,23 @@
+package dansplugins.playerlore.commands;
+
+import static org.mockito.ArgumentMatchers.contains;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import dansplugins.playerlore.PlayerLore;
+import org.bukkit.command.CommandSender;
+import org.junit.jupiter.api.Test;
+
+public class DefaultCommandTest {
+
+    @Test
+    public void execute_pointsPlayersToTheRegisteredHelpCommand() {
+        PlayerLore playerLore = mock(PlayerLore.class);
+        CommandSender commandSender = mock(CommandSender.class);
+        DefaultCommand defaultCommand = new DefaultCommand(playerLore);
+
+        defaultCommand.execute(commandSender);
+
+        verify(commandSender).sendMessage(contains("/pl help"));
+    }
+}

src/test/java/dansplugins/playerlore/commands/EditCommandTest.java

@@ -0,0 +1,90 @@
+package dansplugins.playerlore.commands;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.bukkit.ChatColor;
+import org.bukkit.Material;
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
+import org.bukkit.inventory.meta.ItemMeta;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class EditCommandTest {
+
+    private EditCommand editCommand;
+    private Player player;
+    private PlayerInventory inventory;
+    private ItemStack item;
+    private ItemMeta itemMeta;
+
+    @BeforeEach
+    public void setUp() {
+        editCommand = new EditCommand();
+        player = mock(Player.class);
+        inventory = mock(PlayerInventory.class);
+        item = mock(ItemStack.class);
+        itemMeta = mock(ItemMeta.class);
+
+        when(player.getInventory()).thenReturn(inventory);
+        when(inventory.getItemInMainHand()).thenReturn(item);
+        when(item.getType()).thenReturn(Material.DIAMOND_SWORD);
+        when(item.getItemMeta()).thenReturn(itemMeta);
+    }
+
+    @Test
+    public void execute_editsFirstLoreLineUsing1BasedIndex() {
+        List<String> lore = new ArrayList<>(Arrays.asList("original line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = editCommand.execute(player, new String[]{"1", "\"new line\""});
+
+        assertTrue(result);
+        verify(itemMeta).setLore(Arrays.asList(ChatColor.WHITE + "new line"));
+    }
+
+    @Test
+    public void execute_rejectsIndexBelow1() {
+        List<String> lore = new ArrayList<>(Arrays.asList("original line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = editCommand.execute(player, new String[]{"0", "\"new line\""});
+
+        assertFalse(result);
+        verify(player).sendMessage(anyString());
+    }
+
+    @Test
+    public void execute_rejectsIndexBeyondLoreSize() {
+        List<String> lore = new ArrayList<>(Arrays.asList("original line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = editCommand.execute(player, new String[]{"2", "\"new line\""});
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void execute_rejectsNonNumericIndex() {
+        boolean result = editCommand.execute(player, new String[]{"abc", "\"new line\""});
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void execute_rejectsMissingArgs() {
+        boolean result = editCommand.execute(player, new String[]{});
+
+        assertFalse(result);
+    }
+}

src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java

@@ -0,0 +1,87 @@
+package dansplugins.playerlore.commands;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.bukkit.Material;
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
+import org.bukkit.inventory.meta.ItemMeta;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class RemoveCommandTest {
+
+    private RemoveCommand removeCommand;
+    private Player player;
+    private PlayerInventory inventory;
+    private ItemStack item;
+    private ItemMeta itemMeta;
+
+    @BeforeEach
+    public void setUp() {
+        removeCommand = new RemoveCommand();
+        player = mock(Player.class);
+        inventory = mock(PlayerInventory.class);
+        item = mock(ItemStack.class);
+        itemMeta = mock(ItemMeta.class);
+
+        when(player.getInventory()).thenReturn(inventory);
+        when(inventory.getItemInMainHand()).thenReturn(item);
+        when(item.getType()).thenReturn(Material.DIAMOND_SWORD);
+        when(item.getItemMeta()).thenReturn(itemMeta);
+    }
+
+    @Test
+    public void execute_removesFirstLoreLineUsing1BasedIndex() {
+        List<String> lore = new ArrayList<>(Arrays.asList("first line", "second line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = removeCommand.execute(player, new String[]{"1"});
+
+        assertTrue(result);
+        verify(itemMeta).setLore(Arrays.asList("second line"));
+    }
+
+    @Test
+    public void execute_rejectsIndexBelow1() {
+        List<String> lore = new ArrayList<>(Arrays.asList("first line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = removeCommand.execute(player, new String[]{"0"});
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void execute_rejectsIndexBeyondLoreSize() {
+        List<String> lore = new ArrayList<>(Arrays.asList("first line"));
+        when(itemMeta.getLore()).thenReturn(lore);
+
+        boolean result = removeCommand.execute(player, new String[]{"2"});
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void execute_rejectsNonNumericIndex() {
+        boolean result = removeCommand.execute(player, new String[]{"abc"});
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void execute_rejectsMissingArgs() {
+        boolean result = removeCommand.execute(player, new String[]{});
+
+        assertFalse(result);
+    }
+}