ronantakizawa/codereview-bench huggingface dataset:
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/EntityEffect.java (Java)

Code before:
package org.bukkit;

import com.google.common.base.Preconditions;
import io.papermc.paper.datacomponent.DataComponentTypes;
import org.bukkit.entity.*;
import org.bukkit.entity.minecart.ExplosiveMinecart;
import org.bukkit.entity.minecart.SpawnerMinecart;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import java.util.Set;

/**
 * A list of all effects that can happen to entities.
 */
public enum EntityEffect {

    /**
     * Colored particles from an arrow.
     */
    ARROW_PARTICLES(0, Arrow.class),
    /**
     * Rabbit jumping.
     */
    RABBIT_JUMP(1, Rabbit.class),
    /**
     * Resets a spawner minecart's delay to 200. Does not affect actual spawning
     * delay, only the speed at which the entity in the spawner spins.
     */
    RESET_SPAWNER_MINECART_DELAY(1, SpawnerMinecart.class),
    /**

Review comment (suggestion):
Please do not use wildcard imports.
They are generally considered not too stable and iirc even banned in like the google code style.
IJ sometimes does that, but you can iirc just incr the value of imports to like 99999.

Code after:
package org.bukkit;

import com.google.common.base.Preconditions;
import io.papermc.paper.datacomponent.DataComponentTypes;
import org.bukkit.entity.Allay;
import org.bukkit.entity.Animals;
import org.bukkit.entity.Armadillo;
import org.bukkit.entity.ArmorStand;
import org.bukkit.entity.Arrow;
import org.bukkit.entity.Cat;
import org.bukkit.entity.Creaking;
import org.bukkit.entity.Dolphin;
import org.bukkit.entity.Egg;
import org.bukkit.entity.Entity;
import org.bukkit.entity.EvokerFangs;
import org.bukkit.entity.Firework;
import org.bukkit.entity.Fox;
import org.bukkit.entity.Goat;
import org.bukkit.entity.Guardian;
import org.bukkit.entity.Hoglin;
import org.bukkit.entity.IronGolem;
import org.bukkit.entity.LivingEntity;
import org.bukkit.entity.Mob;
import org.bukkit.entity.Ocelot;
import org.bukkit.entity.Player;
import org.bukkit.entity.Rabbit;
import org.bukkit.entity.Ravager;
import org.bukkit.entity.Sheep;
import org.bukkit.entity.Sniffer;
import org.bukkit.entity.Snowball;
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/inventory/InventoryView.java (Java)

Code before:
    @NotNull
    @Deprecated(since = "1.21.1") // Paper
    public String getOriginalTitle();

    /**
     * Sets the title of this inventory window to the specified title if the
     * inventory window supports it.
     * <p>
     * Note if the inventory does not support titles that can be changed (ie, it
     * is not creatable or viewed by a player), then this method will throw an
     * exception.
     *
     * @param title The new title.
     * @deprecated changing the title is not supported. This method has
     * poorly defined and broken behaviors. It should not be used.
     */
    @Deprecated(since = "1.21.1") // Paper
    public void setTitle(@NotNull String title);

    /**
     * Gets the menu type of the inventory view.
     *
     * @return the menu type of the inventory view
     */
    @ApiStatus.Experimental
    @NotNull MenuType getMenuType();
}


Review comment (suggestion):
Given the changes required to make this API work effectively, this should be marked as Nullable.

Code after:
    @NotNull
    @Deprecated(since = "1.21.1") // Paper
    public String getOriginalTitle();

    /**
     * Sets the title of this inventory window to the specified title if the
     * inventory window supports it.
     * <p>
     * Note if the inventory does not support titles that can be changed (ie, it
     * is not creatable or viewed by a player), then this method will throw an
     * exception.
     *
     * @param title The new title.
     * @deprecated changing the title is not supported. This method has
     * poorly defined and broken behaviors. It should not be used.
     */
    @Deprecated(since = "1.21.1") // Paper
    public void setTitle(@NotNull String title);

    /**
     * Gets the menu type of the inventory view if applicable.
     * <p>
     * Some inventory types do not support a menu type. In such cases, this method
     * returns null. This typically applies to inventories belonging to entities
     * like players or animals (e.g., a horse).
     *
     * @return the menu type of the inventory view or null if not applicable
     */
    @ApiStatus.Experimental
    @Nullable MenuType getMenuType();
}
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/inventory/InventoryView.java (Java)

Code before:
    @Deprecated(since = "1.21.1") // Paper
    public String getOriginalTitle();

    /**
     * Sets the title of this inventory window to the specified title if the
     * inventory window supports it.
     * <p>
     * Note if the inventory does not support titles that can be changed (ie, it
     * is not creatable or viewed by a player), then this method will throw an
     * exception.
     *
     * @param title The new title.
     * @deprecated changing the title is not supported. This method has
     * poorly defined and broken behaviors. It should not be used.
     */
    @Deprecated(since = "1.21.1") // Paper
    public void setTitle(@NotNull String title);

    /**
     * Gets the menu type of the inventory view.
     *
     * @return the menu type of the inventory view or null if not applicable
     */
    @ApiStatus.Experimental
    @Nullable MenuType getMenuType();
}


Review comment (suggestion):
rather than just stating null if not applicable might be smart to list the cases where null is returned here as far as I'm aware this is the case only player and animal menus, but it might be worth poking around a bit more to make sure that I'm 100% right about this.  Might just be nice to have a little paragraph not in the return statement being more involved on why this method might return null

Code after:
    @Deprecated(since = "1.21.1") // Paper
    public String getOriginalTitle();

    /**
     * Sets the title of this inventory window to the specified title if the
     * inventory window supports it.
     * <p>
     * Note if the inventory does not support titles that can be changed (ie, it
     * is not creatable or viewed by a player), then this method will throw an
     * exception.
     *
     * @param title The new title.
     * @deprecated changing the title is not supported. This method has
     * poorly defined and broken behaviors. It should not be used.
     */
    @Deprecated(since = "1.21.1") // Paper
    public void setTitle(@NotNull String title);

    /**
     * Gets the menu type of the inventory view if applicable.
     * <p>
     * Some inventory types do not support a menu type. In such cases, this method
     * returns null. This typically applies to inventories belonging to entities
     * like players or animals (e.g., a horse).
     *
     * @return the menu type of the inventory view or null if not applicable
     */
    @ApiStatus.Experimental
    @Nullable MenuType getMenuType();
}
Repository: PaperMC/Paper
File: paper-api/src/main/java/io/papermc/paper/event/inventory/ItemCraftedEvent.java (Java)

Code before:
package io.papermc.paper.event.inventory;

import org.bukkit.entity.Player;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;
import org.bukkit.inventory.ItemStack;
import org.jspecify.annotations.NonNull;


/**
 * Called when a player picks up a crafted item from the result slot of a crafting grid.
 */
public class ItemCraftedEvent extends Event{
    private static final HandlerList HANDLERS = new HandlerList();
    private final Player player;
    private final ItemStack craftedItemStack;

    public ItemCraftedEvent(@NonNull Player player, @NonNull ItemStack craftedItemStack) {
        this.player = player;
        this.craftedItemStack = craftedItemStack;
    }

    /**
     * Gets the Player who triggered the event by picking up the crafted item.
     *
     * @return Player
     */
    @NonNull
    public Player getPlayer() {
        return player;
    }

    /**
     * Gets the ItemStack that was crafted and picked up by the Player.
     *
     * @return ItemStack
     */
    @NonNull

Review comment (suggestion):
You should annotate the class as `@NullMarked` and remove the individual `@NonNull` annotations

Code after:
package io.papermc.paper.event.inventory;

import org.bukkit.entity.Player;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;
import org.bukkit.inventory.ItemStack;
import org.jetbrains.annotations.ApiStatus;
import org.jspecify.annotations.NullMarked;

/**
 * Called when a player picks up a crafted item from the result slot of a crafting grid.
 */
@NullMarked
public class ItemCraftedEvent extends Event {

    private static final HandlerList HANDLER_LIST = new HandlerList();

    private final Player player;
    private final ItemStack craftedItem;

    @ApiStatus.Internal
    public ItemCraftedEvent(final Player player, final ItemStack craftedItem) {
        this.player = player;
        this.craftedItem = craftedItem;
    }

    /**
     * Gets the player who triggered the event by picking up the crafted item.
     *
     * @return the player
     */
    public Player getPlayer() {
        return this.player;
    }

    /**
     * Gets the item that was crafted and picked up by the player.
     *
Repository: PaperMC/Paper
File: paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftSkull.java (Java)

Code before:
import org.bukkit.profile.PlayerProfile;
import org.jetbrains.annotations.Nullable;

public class CraftSkull extends CraftBlockEntityState<SkullBlockEntity> implements Skull {

    private static final int MAX_OWNER_LENGTH = 16;
    private ResolvableProfile profile;
    private Component customName;

    public CraftSkull(World world, SkullBlockEntity tileEntity) {
        super(world, tileEntity);
    }

    protected CraftSkull(CraftSkull state, Location location) {
        super(state, location);
    }

    @Override
    public void load(SkullBlockEntity skull) {
        super.load(skull);

        ResolvableProfile owner = skull.getOwnerProfile();
        if (owner != null) {
            this.profile = owner;
        }
        this.customName = PaperAdventure.asAdventure(skull.customName);
    }

    @Override
    public boolean hasOwner() {
        return this.profile != null;
    }

    @Override
    public String getOwner() {
        return this.hasOwner() ? this.profile.name().orElse(null) : null;
    }

    @Override
    public boolean setOwner(String name) {
        if (name == null || name.length() > CraftSkull.MAX_OWNER_LENGTH) {
            return false;
        }

        GameProfile profile = MinecraftServer.getServer().getProfileCache().get(name).orElse(null);
        if (profile == null) {
            return false;
        }

        this.profile = new ResolvableProfile(profile);
        return true;

Review comment (suggestion):
This will return an empty component when the field is null, i don't think that's what you want if the methods are nullable

Code after:
import org.bukkit.profile.PlayerProfile;
import org.jetbrains.annotations.Nullable;

public class CraftSkull extends CraftBlockEntityState<SkullBlockEntity> implements Skull {

    private static final int MAX_OWNER_LENGTH = 16;
    private ResolvableProfile profile;

    public CraftSkull(World world, SkullBlockEntity tileEntity) {
        super(world, tileEntity);
    }

    protected CraftSkull(CraftSkull state, Location location) {
        super(state, location);
    }

    @Override
    public void load(SkullBlockEntity skull) {
        super.load(skull);

        ResolvableProfile owner = skull.getOwnerProfile();
        if (owner != null) {
            this.profile = owner;
        }
    }

    @Override
    public boolean hasOwner() {
        return this.profile != null;
    }

    @Override
    public String getOwner() {
        return this.hasOwner() ? this.profile.name().orElse(null) : null;
    }

    @Override
    public boolean setOwner(String name) {
        if (name == null || name.length() > CraftSkull.MAX_OWNER_LENGTH) {
            return false;
        }

        GameProfile profile = MinecraftServer.getServer().getProfileCache().get(name).orElse(null);
        if (profile == null) {
            return false;
        }

        this.profile = new ResolvableProfile(profile);
        return true;
    }
Repository: PaperMC/Paper
File: paper-server/src/main/java/org/bukkit/craftbukkit/generator/CraftChunkData.java (Java)

Code before:
            return;
        }

        ChunkAccess access = this.getHandle();
        BlockPos blockPosition = new BlockPos(access.getPos().getMinBlockX() + x, y, access.getPos().getMinBlockZ() + z);
        BlockState oldBlockData = access.setBlockState(blockPosition, type, false);

        if (type.hasBlockEntity()) {
            BlockEntity tileEntity = ((EntityBlock) type.getBlock()).newBlockEntity(blockPosition, type);

            // createTile can return null, currently only the case with material MOVING_PISTON
            if (tileEntity == null) {
                access.removeBlockEntity(blockPosition);
            } else {
                access.setBlockEntity(tileEntity);
            }
        } else if (oldBlockData != null && oldBlockData.hasBlockEntity()) {
            access.removeBlockEntity(blockPosition);
        }
    }

    @Override
    public int getHeight(HeightMap heightMap, final int x, final int z) {
        Preconditions.checkArgument(heightMap != null, "HeightMap cannot be null");
        if (x < 0 || x > 15 || z < 0 || z > 15) {
            throw new IllegalArgumentException("Cannot get height outside of a chunks bounds");
        }
        return getHandle().getHeight(CraftHeightMap.toNMS(heightMap), x, z);
    }
}


Review comment (bug):
maybe print the coords, just so it's easier to see what has happened when debugging, etc; People mistaking block/chunk bound coords is always kinda fun

Code after:
            return;
        }

        ChunkAccess access = this.getHandle();
        BlockPos blockPosition = new BlockPos(access.getPos().getMinBlockX() + x, y, access.getPos().getMinBlockZ() + z);
        BlockState oldBlockData = access.setBlockState(blockPosition, type, false);

        if (type.hasBlockEntity()) {
            BlockEntity tileEntity = ((EntityBlock) type.getBlock()).newBlockEntity(blockPosition, type);

            // createTile can return null, currently only the case with material MOVING_PISTON
            if (tileEntity == null) {
                access.removeBlockEntity(blockPosition);
            } else {
                access.setBlockEntity(tileEntity);
            }
        } else if (oldBlockData != null && oldBlockData.hasBlockEntity()) {
            access.removeBlockEntity(blockPosition);
        }
    }

    @Override
    public int getHeight(final HeightMap heightMap, final int x, final int z) {
        Preconditions.checkArgument(heightMap != null, "HeightMap cannot be null");
        Preconditions.checkArgument(x >= 0 && x <= 15 && z >= 0 && z <= 15, "Cannot get height outside of a chunks bounds, must be between 0 and 15, got x: %s, z: %s", x, z);

        return getHandle().getHeight(CraftHeightMap.toNMS(heightMap), x, z);
    }
}
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/block/Block.java (Java)

Code before:
     *
     * @param itemStack {@link ItemStack} used to mine this Block
     * @return the speed that this Block will be mined by the given {@link ItemStack}
     */
    default float getDestroySpeed(final @NotNull ItemStack itemStack) {
        return this.getBlockData().getDestroySpeed(itemStack);
    }

    /**
     * Gets the speed at which this block will be destroyed by a given {@link ItemStack}
     * <p>
     * Default value is 1.0
     *
     * @param itemStack {@link ItemStack} used to mine this Block
     * @param considerEnchants true to look at enchants on the itemstack
     * @return the speed that this Block will be mined by the given {@link ItemStack}
     */
    default float getDestroySpeed(@NotNull ItemStack itemStack, boolean considerEnchants) {
        return this.getBlockData().getDestroySpeed(itemStack, considerEnchants);
    }
    // Paper end - destroy speed API

    /**
     * Checks if the block can suffocate.
     *
     * @return <code>true</code> if the block can suffocate
     */
    boolean isSuffocating();
}


Review comment (suggestion):
You can use the `@code` tag instead, same for the other method

Code after:
     *
     * @param itemStack {@link ItemStack} used to mine this Block
     * @return the speed that this Block will be mined by the given {@link ItemStack}
     */
    default float getDestroySpeed(final @NotNull ItemStack itemStack) {
        return this.getBlockData().getDestroySpeed(itemStack);
    }

    /**
     * Gets the speed at which this block will be destroyed by a given {@link ItemStack}
     * <p>
     * Default value is 1.0
     *
     * @param itemStack {@link ItemStack} used to mine this Block
     * @param considerEnchants true to look at enchants on the itemstack
     * @return the speed that this Block will be mined by the given {@link ItemStack}
     */
    default float getDestroySpeed(@NotNull ItemStack itemStack, boolean considerEnchants) {
        return this.getBlockData().getDestroySpeed(itemStack, considerEnchants);
    }
    // Paper end - destroy speed API

    /**
     * Checks if the block can suffocate.
     *
     * @return {@code true} if the block can suffocate
     */
    boolean isSuffocating();
}
Repository: PaperMC/Paper
File: paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftBee.java (Java)

Code before:
    }

    @Override
    public void setCropsGrownSincePollination(int crops) {
        this.getHandle().numCropsGrownSincePollination = crops;
    }

    @Override
    public int getCropsGrownSincePollination() {
        return this.getHandle().numCropsGrownSincePollination;
    }

    @Override
    public void setTicksSincePollination(int ticks) {
        this.getHandle().ticksWithoutNectarSinceExitingHive = ticks;
    }

    @Override
    public int getTicksSincePollination() {
        return this.getHandle().ticksWithoutNectarSinceExitingHive;
    }

    @Override
    public void setTimeSinceSting(int time) {
        if (time < 0) {
            throw new IllegalArgumentException("Time since sting cannot be negative");
        }

        this.getHandle().timeSinceSting = time;
    }

    @Override
    public int getTimeSinceSting() {
        return this.getHandle().timeSinceSting;
    }
}


Review comment (suggestion):
You can use a precondition, can you also rebuild patches to sort the AT file if you don't mind.
Can also put the @ NonNegative annotation for the api.

Code after:
    }

    @Override
    public void setCropsGrownSincePollination(int crops) {
        this.getHandle().numCropsGrownSincePollination = crops;
    }

    @Override
    public int getCropsGrownSincePollination() {
        return this.getHandle().numCropsGrownSincePollination;
    }

    @Override
    public void setTicksSincePollination(int ticks) {
        this.getHandle().ticksWithoutNectarSinceExitingHive = ticks;
    }

    @Override
    public int getTicksSincePollination() {
        return this.getHandle().ticksWithoutNectarSinceExitingHive;
    }

    @Override
    public void setTimeSinceSting(int time) {
        Preconditions.checkArgument(time >= 0, "Time since sting cannot be negative");
        this.getHandle().timeSinceSting = time;
    }

    @Override
    public int getTimeSinceSting() {
        return this.getHandle().timeSinceSting;
    }
}
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/block/DecoratedPot.java (Java)

Code before:
    public DecoratedPotInventory getInventory();

    @Override // Paper - move docs to TileStateInventoryHolder
    @NotNull
    public DecoratedPotInventory getSnapshotInventory();

    /**
     * Run the specified animation on the decorated pot.
     *
     * @param style the animation style
     */
    public void startWobble(@org.jspecify.annotations.NonNull Wobble style);

    /**
     * A side on a decorated pot. Sides are relative to the facing state of a
     * {@link org.bukkit.block.data.type.DecoratedPot}.
     */
    public static enum Side {
        BACK,
        LEFT,
        RIGHT,
        FRONT
    }

    /**
     * An animation style of decorated pot.
     */
    public static enum Wobble {
        POSITIVE,
        NEGATIVE
    }
}


Review comment (suggestion):
sounds a bit rough
```suggestion
     * Style a DecoratedPot can wobble in.
```

^ maybe someone has a better idea for jdocs but this sounds a bit better.

Code after:
    public DecoratedPotInventory getInventory();

    @Override // Paper - move docs to TileStateInventoryHolder
    @NotNull
    public DecoratedPotInventory getSnapshotInventory();

    /**
     * Run the specified animation on the decorated pot.
     *
     * @param style the animation style
     * @throws IllegalStateException if this block state is not placed
     */
    public void startWobble(@NotNull WobbleStyle style);

    /**
     * A side on a decorated pot. Sides are relative to the facing state of a
     * {@link org.bukkit.block.data.type.DecoratedPot}.
     */
    public static enum Side {
        BACK,
        LEFT,
        RIGHT,
        FRONT
    }

    /**
     * Style a DecoratedPot can wobble in.
     */
    public static enum WobbleStyle {
        POSITIVE,
        NEGATIVE
    }
}
Repository: PaperMC/Paper
File: paper-api/src/main/java/org/bukkit/block/DecoratedPot.java (Java)

Code before:
    @Override // Paper - move docs to TileStateInventoryHolder
    @NotNull
    public DecoratedPotInventory getSnapshotInventory();

    /**
     * Run the specified animation on the decorated pot.
     *
     * @param style the animation style
     */
    public void startWobble(@org.jspecify.annotations.NonNull Wobble style);

    /**
     * A side on a decorated pot. Sides are relative to the facing state of a
     * {@link org.bukkit.block.data.type.DecoratedPot}.
     */
    public static enum Side {
        BACK,
        LEFT,
        RIGHT,
        FRONT
    }

    /**
     * An animation style of decorated pot.
     */
    public static enum Wobble {
        POSITIVE,
        NEGATIVE
    }
}


Review comment (suggestion):
```suggestion
    public static enum WobbleStyle {
```

Code after:
    @Override // Paper - move docs to TileStateInventoryHolder
    @NotNull
    public DecoratedPotInventory getSnapshotInventory();

    /**
     * Run the specified animation on the decorated pot.
     *
     * @param style the animation style
     * @throws IllegalStateException if this block state is not placed
     */
    public void startWobble(@NotNull WobbleStyle style);

    /**
     * A side on a decorated pot. Sides are relative to the facing state of a
     * {@link org.bukkit.block.data.type.DecoratedPot}.
     */
    public static enum Side {
        BACK,
        LEFT,
        RIGHT,
        FRONT
    }

    /**
     * Style a DecoratedPot can wobble in.
     */
    public static enum WobbleStyle {
        POSITIVE,
        NEGATIVE
    }
}