fix: handle missing phase in processPhase without NPE (Sonar S2259)

pom.xml

@@ -67,7 +67,7 @@
         <!-- Do not change unless you want different name for local builds. -->
         <build.number>-LOCAL</build.number>
         <!-- This allows to change between versions. -->
-        <build.version>1.27.1</build.version>
+        <build.version>1.27.2</build.version>
         <!-- SonarCloud -->
         <sonar.projectKey>BentoBoxWorld_AOneBlock</sonar.projectKey>
         <sonar.organization>bentobox-world</sonar.organization>

src/main/java/world/bentobox/aoneblock/listeners/BlockListener.java

@@ -423,7 +423,7 @@ private void process(@NonNull Cancellable e, @NonNull Island island, @Nullable P
 
         // Process phase changes and requirements
         ProcessPhaseResult phaseResult = processPhase(e, island, is, player, world, block);
-        if (e.isCancelled()) {
+        if (phaseResult == null || e.isCancelled()) {
             return;
         }
 
@@ -455,16 +455,29 @@ private record ProcessPhaseResult(OneBlockPhase phase, boolean isCurrPhaseNew, i
      * @param player - player involved
      * @param world - world where processing occurs
      * @param block - block being processed
-     * @return ProcessPhaseResult containing phase details
+     * @return ProcessPhaseResult containing phase details, or null if no phase covers the
+     *         island's block number. In that case the event is cancelled so the magic block
+     *         is not lost, and an error is logged for the admin.
      */
+    @Nullable
     private ProcessPhaseResult processPhase(Cancellable e, Island i, OneBlockIslands is, Player player, World world, Block block) {
         OneBlockPhase phase = oneBlocksManager.getPhase(is.getBlockNumber());
         String prevPhaseName = is.getPhaseName();
 
-        if (Objects.requireNonNull(phase).getGotoBlock() != null) {
+        if (phase != null && phase.getGotoBlock() != null) {
             phase = handleGoto(is, phase.getGotoBlock());
         }
 
+        if (phase == null) {
+            // No phase covers this block number. This happens when no phase files loaded,
+            // when the first phase does not start at block 0, or when a goto points below
+            // the first phase. Keep the block in place rather than breaking the island.
+            addon.logError("No phase found for block number " + is.getBlockNumber() + " on island "
+                    + i.getUniqueId() + ". Check the phase files in the phases folder.");
+            e.setCancelled(true);
+            return null;
+        }
+
         String currPhaseName = phase.getPhaseName() == null ? "" : phase.getPhaseName();
         handlePhaseChange(is, currPhaseName);
 
@@ -516,7 +529,7 @@ private void handleNewPhase(Player player, Island i, OneBlockIslands is, OneBloc
      */
     private void handlePhaseChange(OneBlockIslands is, String currPhaseName) {
         OneBlockPhase nextPhase = oneBlocksManager.getPhase(is.getBlockNumber() + 1);
-        if (Objects.requireNonNull(nextPhase).getGotoBlock() != null) {
+        if (nextPhase != null && nextPhase.getGotoBlock() != null) {
             nextPhase = oneBlocksManager.getPhase(nextPhase.getGotoBlock());
         }
         String nextPhaseName = nextPhase == null || nextPhase.getPhaseName() == null ? "" : nextPhase.getPhaseName();

src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest.java

@@ -3,6 +3,8 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -17,7 +19,10 @@
 import org.bukkit.World;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
+import org.bukkit.entity.ArmorStand;
+import org.bukkit.entity.EntityType;
 import org.bukkit.event.block.BlockFromToEvent;
+import org.bukkit.event.entity.EntityInteractEvent;
 import org.eclipse.jdt.annotation.NonNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -192,6 +197,57 @@ void testOnBlockFromToCenterBlock() {
         assertTrue(e.isCancelled());
     }
 
+    /**
+     * When no phase covers the island's block number (e.g. phase files failed to load or
+     * a goto points below the first phase), the break must be cancelled and an error logged
+     * instead of throwing a NullPointerException.
+     */
+    @Test
+    void testProcessNoPhaseForBlockNumberCancelsAndLogs() {
+        when(addon.inWorld(any(World.class))).thenReturn(true);
+        island.setCenter(location);
+        when(im.getIslandAt(location)).thenReturn(Optional.of(island));
+        when(obm.getPhase(anyInt())).thenReturn(null);
+
+        Block block = mock(Block.class);
+        when(block.getLocation()).thenReturn(location);
+        when(block.getWorld()).thenReturn(world);
+        ArmorStand minion = mock(ArmorStand.class);
+        when(minion.getType()).thenReturn(EntityType.ARMOR_STAND);
+
+        EntityInteractEvent e = new EntityInteractEvent(minion, block);
+        bl.onBlockBreakByMinion(e);
+
+        assertTrue(e.isCancelled());
+        verify(addon).logError(anyString());
+    }
+
+    /**
+     * A phase whose goto target lies below the first phase must also be handled gracefully.
+     */
+    @Test
+    void testProcessGotoBelowFirstPhaseCancelsAndLogs() {
+        when(addon.inWorld(any(World.class))).thenReturn(true);
+        island.setCenter(location);
+        when(im.getIslandAt(location)).thenReturn(Optional.of(island));
+        OneBlockPhase gotoPhase = new OneBlockPhase("0");
+        gotoPhase.setGotoBlock(-5);
+        when(obm.getPhase(0)).thenReturn(gotoPhase);
+        when(obm.getPhase(-5)).thenReturn(null);
+
+        Block block = mock(Block.class);
+        when(block.getLocation()).thenReturn(location);
+        when(block.getWorld()).thenReturn(world);
+        ArmorStand minion = mock(ArmorStand.class);
+        when(minion.getType()).thenReturn(EntityType.ARMOR_STAND);
+
+        EntityInteractEvent e = new EntityInteractEvent(minion, block);
+        bl.onBlockBreakByMinion(e);
+
+        assertTrue(e.isCancelled());
+        verify(addon).logError(anyString());
+    }
+
     /**
      * Test method for {@link world.bentobox.aoneblock.listeners.BlockListener#saveCache()}.
      */