SPIGOT-7355: More field renames and fixes

- Rename MapCursor Types to match their Minecraft names
- SPIGOT-7355: Rename ItemFlag#HIDE_POTION_EFFECTS to better reflect its function
- Fix Attribute rename (CraftBukkit only)
- Add rename routing validation (CraftBukkit only)

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaEnchantedBook.java

@@ -39,7 +39,7 @@ class CraftMetaEnchantedBook extends CraftMetaItem implements EnchantmentStorage
         getOrEmpty(tag, STORED_ENCHANTMENTS).ifPresent((itemEnchantments) -> {
             enchantments = buildEnchantments(itemEnchantments);
             if (!itemEnchantments.showInTooltip) {
-                addItemFlags(ItemFlag.HIDE_POTION_EFFECTS);
+                addItemFlags(ItemFlag.HIDE_ADDITIONAL_TOOLTIP);
             }
         });
     }
@@ -54,7 +54,7 @@ class CraftMetaEnchantedBook extends CraftMetaItem implements EnchantmentStorage
     void applyToItem(CraftMetaItem.Applicator itemTag) {
         super.applyToItem(itemTag);
 
-        applyEnchantments(enchantments, itemTag, STORED_ENCHANTMENTS, ItemFlag.HIDE_POTION_EFFECTS);
+        applyEnchantments(enchantments, itemTag, STORED_ENCHANTMENTS, ItemFlag.HIDE_ADDITIONAL_TOOLTIP);
     }
 
     @Override

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java

@@ -332,7 +332,7 @@ class CraftMetaItem implements ItemMeta, Damageable, Repairable, BlockDataMeta {
         });
 
         getOrEmpty(tag, HIDE_ADDITIONAL_TOOLTIP).ifPresent((h) -> {
-            addItemFlags(ItemFlag.HIDE_POTION_EFFECTS);
+            addItemFlags(ItemFlag.HIDE_ADDITIONAL_TOOLTIP);
         });
         getOrEmpty(tag, HIDE_TOOLTIP).ifPresent((u) -> {
             hideTooltip = true;
@@ -724,7 +724,7 @@ class CraftMetaItem implements ItemMeta, Damageable, Repairable, BlockDataMeta {
         }
 
         if (hideFlag != 0) {
-            if (hasItemFlag(ItemFlag.HIDE_POTION_EFFECTS)) {
+            if (hasItemFlag(ItemFlag.HIDE_ADDITIONAL_TOOLTIP)) {
                 itemTag.put(HIDE_ADDITIONAL_TOOLTIP, Unit.INSTANCE);
             }
         }

src/main/java/org/bukkit/craftbukkit/legacy/FieldRename.java

@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit.legacy;
 
 import org.bukkit.Particle;
+import org.bukkit.attribute.Attribute;
 import org.bukkit.block.Biome;
 import org.bukkit.block.banner.PatternType;
 import org.bukkit.craftbukkit.legacy.fieldrename.FieldRenameData;
@@ -11,7 +12,9 @@ import org.bukkit.craftbukkit.legacy.reroute.RerouteStatic;
 import org.bukkit.craftbukkit.util.ApiVersion;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.EntityType;
+import org.bukkit.inventory.ItemFlag;
 import org.bukkit.loot.LootTables;
+import org.bukkit.map.MapCursor;
 import org.bukkit.potion.PotionEffectType;
 import org.bukkit.potion.PotionType;
 
@@ -34,6 +37,8 @@ public class FieldRename {
             case "org/bukkit/Particle" -> convertParticleName(apiVersion, from);
             case "org/bukkit/loot/LootTables" -> convertLootTablesName(apiVersion, from);
             case "org/bukkit/attribute/Attribute" -> convertAttributeName(apiVersion, from);
+            case "org/bukkit/map/MapCursor$Type" -> convertMapCursorTypeName(apiVersion, from);
+            case "org/bukkit/inventory/ItemFlag" -> convertItemFlagName(apiVersion, from);
             default -> from;
         };
     }
@@ -320,7 +325,7 @@ public class FieldRename {
         return LootTables.valueOf(convertLootTablesName(ApiVersion.CURRENT, name));
     }
 
-    // LootTables
+    // Attribute
     private static final FieldRenameData ATTRIBUTE_DATA = FieldRenameData.Builder.newBuilder()
             .forAllVersions()
             .change("HORSE_JUMP_STRENGTH", "GENERIC_JUMP_STRENGTH")
@@ -333,8 +338,57 @@ public class FieldRename {
 
     @RerouteMethodName("valueOf")
     @RerouteStatic("org/bukkit/attribute/Attribute")
-    public static LootTables valueOf_Attribute(String name) {
+    public static Attribute valueOf_Attribute(String name) {
         // We don't have version-specific changes, so just use current, and don't inject a version
-        return LootTables.valueOf(convertAttributeName(ApiVersion.CURRENT, name));
+        return Attribute.valueOf(convertAttributeName(ApiVersion.CURRENT, name));
+    }
+
+    // MapCursor Type
+    private static final FieldRenameData MAP_CURSOR_TYPE_DATA = FieldRenameData.Builder.newBuilder()
+            .forVersionsBefore(ApiVersion.FIELD_NAME_PARITY)
+            .change("RED_MARKER", "TARGET_POINT")
+            .forAllVersions()
+            .change("WHITE_POINTER", "PLAYER")
+            .change("GREEN_POINTER", "FRAME")
+            .change("RED_POINTER", "RED_MARKER")
+            .change("BLUE_POINTER", "BLUE_MARKER")
+            .change("WHITE_CROSS", "TARGET_X")
+            .change("WHITE_CIRCLE", "PLAYER_OFF_MAP")
+            .change("SMALL_WHITE_CIRCLE", "PLAYER_OFF_LIMITS")
+            .change("TEMPLE", "MONUMENT")
+            .change("DESERT_VILLAGE", "VILLAGE_DESERT")
+            .change("PLAINS_VILLAGE", "VILLAGE_PLAINS")
+            .change("SAVANNA_VILLAGE", "VILLAGE_SAVANNA")
+            .change("SNOWY_VILLAGE", "VILLAGE_SNOWY")
+            .change("TAIGA_VILLAGE", "VILLAGE_TAIGA")
+            .build();
+
+    @DoNotReroute
+    public static String convertMapCursorTypeName(ApiVersion version, String from) {
+        return MAP_CURSOR_TYPE_DATA.getReplacement(version, from);
+    }
+
+    @RerouteMethodName("valueOf")
+    @RerouteStatic("org/bukkit/map/MapCursor$Type")
+    public static MapCursor.Type valueOf_MapCursorType(String name, @InjectPluginVersion ApiVersion version) {
+        return MapCursor.Type.valueOf(convertMapCursorTypeName(version, name));
+    }
+
+    // ItemFlag
+    private static final FieldRenameData ITEM_FLAG_DATA = FieldRenameData.Builder.newBuilder()
+            .forAllVersions()
+            .change("HIDE_POTION_EFFECTS", "HIDE_ADDITIONAL_TOOLTIP")
+            .build();
+
+    @DoNotReroute
+    public static String convertItemFlagName(ApiVersion version, String from) {
+        return ITEM_FLAG_DATA.getReplacement(version, from);
+    }
+
+    @RerouteMethodName("valueOf")
+    @RerouteStatic("org/bukkit/inventory/ItemFlag")
+    public static ItemFlag valueOf_ItemFlag(String name) {
+        // We don't have version-specific changes, so just use current, and don't inject a version
+        return ItemFlag.valueOf(convertItemFlagName(ApiVersion.CURRENT, name));
     }
 }

src/main/java/org/bukkit/craftbukkit/legacy/reroute/NotInBukkit.java

@@ -0,0 +1,11 @@
+package org.bukkit.craftbukkit.legacy.reroute;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+public @interface NotInBukkit {
+}

src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java

@@ -110,6 +110,8 @@ public class RerouteBuilder {
 
         Type targetType = Type.getType(method);
 
-        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn);
+        boolean inBukkit = !method.isAnnotationPresent(NotInBukkit.class);
+
+        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit);
     }
 }

src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java

@@ -5,5 +5,5 @@ import org.objectweb.asm.Type;
 
 public record RerouteMethodData(String source, Type sourceDesc, Type sourceOwner, String sourceName,
                                 boolean staticReroute, Type targetType, String targetOwner, String targetName,
-                                List<RerouteArgument> arguments, RerouteReturn rerouteReturn) {
+                                List<RerouteArgument> arguments, RerouteReturn rerouteReturn, boolean isInBukkit) {
 }

src/main/java/org/bukkit/craftbukkit/util/Commodore.java

@@ -25,6 +25,7 @@ import org.bukkit.craftbukkit.legacy.reroute.RerouteArgument;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteBuilder;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData;
 import org.bukkit.plugin.AuthorNagException;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.objectweb.asm.AnnotationVisitor;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
@@ -62,7 +63,15 @@ public class Commodore {
             "org/spigotmc/event/entity/EntityDismountEvent", "org/bukkit/event/entity/EntityDismountEvent"
     );
 
-    private static final Map<String, RerouteMethodData> FIELD_RENAME_METHOD_REROUTE = RerouteBuilder.buildFromClass(FieldRename.class);
+    private static Map<String, RerouteMethodData> createReroutes(Class<?> clazz) {
+        Map<String, RerouteMethodData> reroutes = RerouteBuilder.buildFromClass(clazz);
+        REROUTES.add(reroutes);
+        return reroutes;
+    }
+
+    @VisibleForTesting
+    public static final List<Map<String, RerouteMethodData>> REROUTES = new ArrayList<>(); // Only used for testing
+    private static final Map<String, RerouteMethodData> FIELD_RENAME_METHOD_REROUTE = createReroutes(FieldRename.class);
 
     public static void main(String[] args) {
         OptionParser parser = new OptionParser();

src/test/java/org/bukkit/craftbukkit/legacy/RerouteValidationTest.java

@@ -0,0 +1,108 @@
+package org.bukkit.craftbukkit.legacy;
+
+import static org.junit.jupiter.api.Assertions.*;
+import com.google.common.base.Joiner;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Stream;
+import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData;
+import org.bukkit.craftbukkit.util.Commodore;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.objectweb.asm.Type;
+
+public class RerouteValidationTest {
+
+    public static Stream<Arguments> data() {
+        return Commodore.REROUTES.stream().map(Arguments::of);
+    }
+
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testReroutes(Map<String, RerouteMethodData> reroutes) {
+        Map<String, String> wrongReroutes = new HashMap<>();
+        String owner = null;
+
+        for (Map.Entry<String, RerouteMethodData> entry : reroutes.entrySet()) {
+            owner = entry.getValue().targetOwner();
+            if (!entry.getValue().isInBukkit()) {
+                continue;
+            }
+
+            String error = isValid(entry.getKey(), entry.getValue());
+
+            if (error != null) {
+                wrongReroutes.put(entry.getKey(), error);
+            }
+        }
+
+        assertTrue(wrongReroutes.isEmpty(), String.format("""
+                There are validation errors for method reroutes in Class: %s
+                This means there is no methods in bukkit matching the reroute.
+                If the method is no longer present in bukkit because it got removed, than mark the reroute method with @NotInBukkit
+
+                Following Errors where found:
+                %s
+                """, owner, Joiner.on('\n').withKeyValueSeparator(": ").join(wrongReroutes)));
+    }
+
+    private String isValid(String key, RerouteMethodData rerouteMethodData) {
+        try {
+            Class<?> clazz = toClass(rerouteMethodData.sourceOwner());
+            Class<?> returnClazz = toClass(rerouteMethodData.sourceDesc().getReturnType());
+            Class<?>[] paras = new Class[rerouteMethodData.sourceDesc().getArgumentCount()];
+            Type[] paraTypes = rerouteMethodData.sourceDesc().getArgumentTypes();
+
+            for (int i = 0; i < paraTypes.length; i++) {
+                paras[i] = toClass(paraTypes[i]);
+            }
+
+            Method method = clazz.getDeclaredMethod(rerouteMethodData.sourceName(), paras);
+
+            if (method.getReturnType() != returnClazz) {
+                return "Return type mismatch expected " + method.getReturnType().getName() + " got: " + returnClazz;
+            }
+
+            if (!Modifier.isPublic(method.getModifiers())) {
+                return "Method is not public";
+            }
+
+            if (Modifier.isStatic(method.getModifiers()) && !rerouteMethodData.staticReroute()) {
+                return "Method is static, but the reroute data is not marked as static reroute";
+            }
+
+            if (!Modifier.isStatic(method.getModifiers()) && rerouteMethodData.staticReroute()) {
+                return "Method is not static, but the reroute data is marked as static reroute";
+            }
+        } catch (ClassNotFoundException e) {
+            return "No such Class: " + e.getLocalizedMessage();
+        } catch (NoSuchMethodException e) {
+            return "No such method: " + e.getLocalizedMessage();
+        }
+
+        return null;
+    }
+
+    private Class<?> toClass(Type type) throws ClassNotFoundException {
+        if (type.getSort() == Type.OBJECT) {
+            return Class.forName(type.getClassName(), false, getClass().getClassLoader());
+        } else if (type.getSort() == Type.ARRAY) {
+            return Class.forName(type.getDescriptor().replace('/', '.'), false, getClass().getClassLoader());
+        } else {
+            return switch (type.getSort()) {
+                case Type.BOOLEAN -> boolean.class;
+                case Type.CHAR -> char.class;
+                case Type.BYTE -> byte.class;
+                case Type.SHORT -> short.class;
+                case Type.INT -> int.class;
+                case Type.FLOAT -> float.class;
+                case Type.LONG -> long.class;
+                case Type.DOUBLE -> double.class;
+                default -> throw new UnsupportedOperationException("Type not supported: " + type);
+            };
+        }
+    }
+}