From f40c2e4ff4bad35cf00627dbc030ad6de2416d42 Mon Sep 17 00:00:00 2001 From: Jikoo Date: Fri, 20 Mar 2026 11:50:33 -0400 Subject: [PATCH] Clean up code smells --- .../anvil/CombineEnchants.java | 12 +++++----- .../table/EnchantingTable.java | 5 +++-- .../planarenchanting/util/EnchantData.java | 15 +------------ .../util/EnchantDataService.java | 22 +++++++++++++++++++ .../anvil/CombineEnchantsTest.java | 16 ++++++++------ .../anvil/PlanarForgeTest.java | 2 +- .../table/EnchantingTableTest.java | 12 +++++----- .../table/TableEnchantListenerTest.java | 16 +++++++------- .../anvil/ComponentAnvilFunctions.java | 2 +- .../planarenchanting/generator/Generator.java | 6 ++--- .../generator/ItemConsumingGenerator.java | 2 +- .../generator/util/FieldAccessor.java | 4 ++-- 12 files changed, 65 insertions(+), 49 deletions(-) create mode 100644 enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantDataService.java diff --git a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/anvil/CombineEnchants.java b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/anvil/CombineEnchants.java index 7920db2..3eec976 100644 --- a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/anvil/CombineEnchants.java +++ b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/anvil/CombineEnchants.java @@ -6,6 +6,8 @@ import java.util.Objects; import com.github.jikoo.planarenchanting.util.EnchantData; import com.github.jikoo.planarenchanting.util.EnchantmentAccess; +import com.github.jikoo.planarenchanting.util.EnchantDataService; + import org.bukkit.enchantments.Enchantment; import org.jspecify.annotations.NullMarked; @@ -22,7 +24,7 @@ public class CombineEnchants implements AnvilFunction { protected CombineEnchants(Platform platform, EnchantmentAccess access) { this.access = access; - this.platform = platform.platform; + this.platform = platform.internalPlatform; } @Override @@ -110,10 +112,10 @@ public enum Platform { JAVA(new Java()), BEDROCK(new Bedrock()),; - private final EnchantingPlatform platform; + private final EnchantingPlatform internalPlatform; Platform(EnchantingPlatform platform) { - this.platform = platform; + this.internalPlatform = platform; } } @@ -128,7 +130,7 @@ private sealed interface EnchantingPlatform { private static final class Java implements EnchantingPlatform { @Override public int getAnvilCost(Enchantment enchantment, boolean isFromBook) { - int value = EnchantData.Service.PROVIDER.of(enchantment).getAnvilCost(); + int value = EnchantDataService.PROVIDER.of(enchantment).getAnvilCost(); return isFromBook ? Math.max(1, value / 2) : value; } @@ -146,7 +148,7 @@ public int getInapplicableCost() { private static final class Bedrock implements EnchantingPlatform { @Override public int getAnvilCost(Enchantment enchantment, boolean isFromBook) { - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); int cost = data.getAnvilCost(); diff --git a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/table/EnchantingTable.java b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/table/EnchantingTable.java index 1c42ce2..1d13e99 100644 --- a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/table/EnchantingTable.java +++ b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/table/EnchantingTable.java @@ -1,6 +1,7 @@ package com.github.jikoo.planarenchanting.table; import com.github.jikoo.planarenchanting.util.EnchantData; +import com.github.jikoo.planarenchanting.util.EnchantDataService; import com.github.jikoo.planarwrappers.util.WeightedRandom; import java.util.Collection; import java.util.Collections; @@ -104,7 +105,7 @@ public void setMaxLevel(@NotNull ToIntFunction<@NotNull Enchantment> maxLevel) { Map available = new HashMap<>(); for (Enchantment enchantment : this.enchantments) { - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); // Find a level appropriate for the finalized enchanting level. for (int lvl = maxLevel.applyAsInt(enchantment); lvl >= enchantment.getStartLevel(); --lvl) { if (enchantQuality >= data.getMinModifiedCost(lvl) @@ -138,7 +139,7 @@ private void addEnchant( Enchantment choice = WeightedRandom.choose( random, available.keySet(), - enchant -> EnchantData.Service.PROVIDER.of(enchant).getWeight() + enchant -> EnchantDataService.PROVIDER.of(enchant).getWeight() ); // Add selected enchantment and remove it from the available listings. diff --git a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantData.java b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantData.java index 0e86343..ad2fb14 100644 --- a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantData.java +++ b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantData.java @@ -1,8 +1,6 @@ package com.github.jikoo.planarenchanting.util; -import java.util.ServiceLoader; import org.bukkit.enchantments.Enchantment; -import org.jetbrains.annotations.ApiStatus; import org.jspecify.annotations.NullMarked; /** @@ -53,7 +51,7 @@ public interface EnchantData { /** * An enchantment data provider. * - * @see Service#PROVIDER + * @see EnchantDataService#PROVIDER */ @NullMarked interface Provider { @@ -62,15 +60,4 @@ interface Provider { } - @ApiStatus.NonExtendable - interface Service { - - /** - * A {@link Provider} loaded from a service. - */ - Provider PROVIDER = ServiceLoader.load(Provider.class, Provider.class.getClassLoader()) - .findFirst().orElseThrow(); - - } - } diff --git a/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantDataService.java b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantDataService.java new file mode 100644 index 0000000..d8a1fd8 --- /dev/null +++ b/enchanting-common/src/main/java/com/github/jikoo/planarenchanting/util/EnchantDataService.java @@ -0,0 +1,22 @@ +package com.github.jikoo.planarenchanting.util; + +import java.util.ServiceLoader; + +import org.jetbrains.annotations.ApiStatus; + +import com.github.jikoo.planarenchanting.util.EnchantData.Provider; + +@ApiStatus.NonExtendable +public class EnchantDataService { + + /** + * A {@link Provider} loaded from a service. + */ + public static final Provider PROVIDER = ServiceLoader.load(Provider.class, Provider.class.getClassLoader()) + .findFirst().orElseThrow(); + + private EnchantDataService() { + throw new IllegalStateException("Cannot instantiate static helper container."); + } + +} \ No newline at end of file diff --git a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/CombineEnchantsTest.java b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/CombineEnchantsTest.java index c20cfb9..5d59ca8 100644 --- a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/CombineEnchantsTest.java +++ b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/CombineEnchantsTest.java @@ -21,6 +21,8 @@ import com.github.jikoo.planarenchanting.anvil.CombineEnchants.Platform; import com.github.jikoo.planarenchanting.util.EnchantData; import com.github.jikoo.planarenchanting.util.EnchantmentAccess; +import com.github.jikoo.planarenchanting.util.EnchantDataService; + import java.util.Map; import org.bukkit.Bukkit; import org.bukkit.NamespacedKey; @@ -185,7 +187,7 @@ void getLevelCostMerge(Platform platform, int cost, boolean conflict) { Map base = Map.of(enchantment, 1); Map added = Map.of(enchantment, 1); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); doReturn(5).when(data).getAnvilCost(); CombineEnchants function = new CombineEnchants<>(platform, access); @@ -220,7 +222,7 @@ void getLevelCostUsesHigher( Map base = Map.of(enchantment, baseLevel); Map added = Map.of(enchantment, addedLevel); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); int anvilCost = 5; doReturn(anvilCost).when(data).getAnvilCost(); @@ -252,7 +254,7 @@ void getLevelCostCapsToMax(int baseLevel, int addedLevel, int maxLevel) { Map base = Map.of(enchantment, baseLevel); Map added = Map.of(enchantment, addedLevel); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); int anvilCost = 5; doReturn(anvilCost).when(data).getAnvilCost(); @@ -284,7 +286,7 @@ void getLevelCostBook(Platform platform) { Map base = Map.of(); Map added = Map.of(enchantment, 1); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); int anvilCost = 5; doReturn(anvilCost).when(data).getAnvilCost(); @@ -314,7 +316,7 @@ void getLevelCostBedrockTrident() { Map base = Map.of(); Map added = Map.of(enchantment, 1); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); int anvilCost = 5; doReturn(anvilCost).when(data).getAnvilCost(); doReturn(true).when(data).isTridentEnchant(); @@ -352,7 +354,7 @@ void getResult() { doReturn(1).when(behavior).getEnchantMaxLevel(any()); Enchantment enchantment = mock(); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); doReturn(5).when(data).getAnvilCost(); doReturn(Map.of()).doReturn(Map.of(enchantment, 1)).when(access).getEnchantments(any()); @@ -381,7 +383,7 @@ void getResultNegative() { doReturn(view).when(state).getAnvilView(); Enchantment enchantment = mock(); - EnchantData data = EnchantData.Service.PROVIDER.of(enchantment); + EnchantData data = EnchantDataService.PROVIDER.of(enchantment); doReturn(5).when(data).getAnvilCost(); doReturn(Map.of(enchantment, -2)).when(access).getEnchantments(any()); diff --git a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/PlanarForgeTest.java b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/PlanarForgeTest.java index d15611d..67bd78e 100644 --- a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/PlanarForgeTest.java +++ b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/anvil/PlanarForgeTest.java @@ -51,7 +51,7 @@ void beforeEach() { functions = mock(Mockito.RETURNS_MOCKS); forgeResult = mock(); anvil = new PlanarForge<>( - view -> { + localView -> { WorkPiece piece = mock(); doReturn(forgeResult).when(piece).temper(); doAnswer(invocation -> { diff --git a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/EnchantingTableTest.java b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/EnchantingTableTest.java index d5c0c50..d24813f 100644 --- a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/EnchantingTableTest.java +++ b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/EnchantingTableTest.java @@ -19,6 +19,8 @@ import static org.mockito.Mockito.mockStatic; import com.github.jikoo.planarenchanting.util.EnchantData; +import com.github.jikoo.planarenchanting.util.EnchantDataService; + import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -96,7 +98,7 @@ void setUpEach() { } static void setUpToolEnchants() { - EnchantData data = EnchantData.Service.PROVIDER.of(Enchantment.EFFICIENCY); + EnchantData data = EnchantDataService.PROVIDER.of(Enchantment.EFFICIENCY); doReturn(10).when(data).getWeight(); doAnswer(invocation -> { int level = invocation.getArgument(0); @@ -108,7 +110,7 @@ static void setUpToolEnchants() { }).when(data).getMaxModifiedCost(anyInt()); doReturn(5).when(Enchantment.EFFICIENCY).getMaxLevel(); - data = EnchantData.Service.PROVIDER.of(Enchantment.UNBREAKING); + data = EnchantDataService.PROVIDER.of(Enchantment.UNBREAKING); doReturn(5).when(data).getWeight(); doAnswer(invocation -> { int level = invocation.getArgument(0); @@ -120,7 +122,7 @@ static void setUpToolEnchants() { }).when(data).getMaxModifiedCost(anyInt()); doReturn(3).when(Enchantment.UNBREAKING).getMaxLevel(); - data = EnchantData.Service.PROVIDER.of(Enchantment.FORTUNE); + data = EnchantDataService.PROVIDER.of(Enchantment.FORTUNE); doReturn(2).when(data).getWeight(); doAnswer(invocation -> { int level = invocation.getArgument(0); @@ -132,7 +134,7 @@ static void setUpToolEnchants() { }).when(data).getMaxModifiedCost(anyInt()); doReturn(3).when(Enchantment.FORTUNE).getMaxLevel(); - data = EnchantData.Service.PROVIDER.of(Enchantment.SILK_TOUCH); + data = EnchantDataService.PROVIDER.of(Enchantment.SILK_TOUCH); doReturn(1).when(data).getWeight(); doReturn(15).when(data).getMinModifiedCost(anyInt()); doReturn(65).when(data).getMaxModifiedCost(anyInt()); @@ -231,7 +233,7 @@ private boolean conflicts(Enchantment enchantment1, Enchantment enchantment2) { @ParameterizedTest @CsvSource({"1,0", "10,0", "15,0", "1,12348", "10,98124", "15,23479"}) void testGetButtonLevels(int shelves, int seed) { - Random random = new Random(seed); + random.setSeed(seed); int[] buttonLevels1 = EnchantingTable.getButtonLevels(random, shelves); random.setSeed(seed); int[] buttonLevels2 = EnchantingTable.getButtonLevels(random, shelves); diff --git a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/TableEnchantListenerTest.java b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/TableEnchantListenerTest.java index 244a957..fdc7097 100644 --- a/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/TableEnchantListenerTest.java +++ b/enchanting-common/src/test/java/com/github/jikoo/planarenchanting/table/TableEnchantListenerTest.java @@ -60,8 +60,8 @@ class TableEnchantListenerTest { private MockedStatic bukkit; - private final Material ENCHANTABLE_MATERIAL = Material.COAL_ORE; - private final Material UNENCHANTABLE_MATERIAL = Material.DIRT; + private final Material enchantableMaterial = Material.COAL_ORE; + private final Material unenchantableMaterial = Material.DIRT; private Enchantment validEnchant; private Collection toolEnchants; @@ -77,9 +77,9 @@ void setUpAll() { bukkit.when(() -> Bukkit.getRegistry(any())).thenAnswer(inv -> { Registry registry = mock(); doAnswer(invocation -> { - NamespacedKey key = invocation.getArgument(0); + NamespacedKey invocationKey = invocation.getArgument(0); Enchantment enchant = mock(); - doReturn(key).when(enchant).getKey(); + doReturn(invocationKey).when(enchant).getKey(); return enchant; }).when(registry).getOrThrow(any()); return registry; @@ -114,7 +114,7 @@ void setUp() { @Override protected boolean isIneligible(Player player, ItemStack enchanted) { - return itemStack.getType() != ENCHANTABLE_MATERIAL; + return itemStack.getType() != enchantableMaterial; } @Override @@ -148,7 +148,7 @@ protected EnchantingTable getTable(Player player, ItemStack enchanted) { }).when(player).setEnchantmentSeed(anyInt()); itemStack = mock(); - doReturn(ENCHANTABLE_MATERIAL).when(itemStack).getType(); + doReturn(enchantableMaterial).when(itemStack).getType(); doReturn(1).when(itemStack).getAmount(); key = new NamespacedKey(plugin, "enchanting_table_seed"); } @@ -167,7 +167,7 @@ void testCanNotEnchantStack() { @Test void testCanNotEnchantWrongMaterial() { - doReturn(UNENCHANTABLE_MATERIAL).when(itemStack).getType(); + doReturn(unenchantableMaterial).when(itemStack).getType(); assertThat( "Material with no enchants cannot be enchanted", listener.canNotEnchant(player, itemStack)); @@ -183,7 +183,7 @@ void testCanEnchant() { @Test void testPrepareItemEnchantInvalid() { - doReturn(UNENCHANTABLE_MATERIAL).when(itemStack).getType(); + doReturn(unenchantableMaterial).when(itemStack).getType(); var event = prepareEvent(15); assertDoesNotThrow(() -> listener.onPrepareItemEnchant(event)); assertThat( diff --git a/enchanting-components/src/main/java/com/github/jikoo/planarenchanting/anvil/ComponentAnvilFunctions.java b/enchanting-components/src/main/java/com/github/jikoo/planarenchanting/anvil/ComponentAnvilFunctions.java index ff7ac1b..1d477d3 100644 --- a/enchanting-components/src/main/java/com/github/jikoo/planarenchanting/anvil/ComponentAnvilFunctions.java +++ b/enchanting-components/src/main/java/com/github/jikoo/planarenchanting/anvil/ComponentAnvilFunctions.java @@ -228,7 +228,7 @@ public void modifyResult(ItemStack modified) { public static final AnvilFunction COMBINE_ENCHANTMENTS_JAVA; public static final AnvilFunction COMBINE_ENCHANTMENTS_BEDROCK; - public static ComponentAnvilFunctions INSTANCE = new ComponentAnvilFunctions(); + public static final ComponentAnvilFunctions INSTANCE = new ComponentAnvilFunctions(); static { ComponentEnchantmentAccess access = new ComponentEnchantmentAccess(); diff --git a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/Generator.java b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/Generator.java index ba21e46..51e2547 100644 --- a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/Generator.java +++ b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/Generator.java @@ -23,7 +23,7 @@ public abstract class Generator { protected final ClassName generatedClass; protected TypeSpec.@UnknownNullability Builder builder; - public Generator(String pkg, String name) { + protected Generator(String pkg, String name) { this.generatedClass = ClassName.get(pkg, name); } @@ -105,7 +105,7 @@ protected String tweakFormatting(String content) { /** * Lenient annotation declaration. This is a bit hairy, because annotations may have values set. */ - private static final @RegExp String ANNOTATION = "@[\\w.]+(?:\\(.*?\\))?"; + private static final @RegExp String ANNOTATION = "@[\\w.]+(?:\\([^\\)]*+\\))?"; /** Lenient generic type declaration. */ private static final @RegExp String GENERIC = "<(?:(?:" + ANNOTATION + " )*?[\\w<>, ]+)+>"; /** @@ -157,7 +157,7 @@ protected String tweakFormatting(String content) { // Javadoc comments. Multi-line only, because JavaPoet only generates multi-line Javadocs. + "(?: {2}/\\*\\*\n(?:.*\n)*?\\s*\\*/\n)?" // Regular comments in case of comment hack fields. - + "(?: {2}//.*?\n)?" + + "(?: {2}//[^\\n]*+\n)?" // Any annotations. + "(?: {2}" + ANNOTATION + "\n)*?" // Matching indent. diff --git a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/ItemConsumingGenerator.java b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/ItemConsumingGenerator.java index b431759..e40d6d3 100644 --- a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/ItemConsumingGenerator.java +++ b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/ItemConsumingGenerator.java @@ -7,7 +7,7 @@ @NullMarked public abstract class ItemConsumingGenerator extends Generator { - public ItemConsumingGenerator(String pkg, String name) { + protected ItemConsumingGenerator(String pkg, String name) { super(pkg, "Baked" + name + "Data"); } diff --git a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/util/FieldAccessor.java b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/util/FieldAccessor.java index 5978c8d..817340d 100644 --- a/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/util/FieldAccessor.java +++ b/enchanting-generator/src/main/java/com/github/jikoo/planarenchanting/generator/util/FieldAccessor.java @@ -18,7 +18,7 @@ public final class FieldAccessor { constructor.setAccessible(true); COMMENT = constructor.newInstance("//"); } catch (ReflectiveOperationException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } @@ -31,7 +31,7 @@ public static void consumeFieldsOfType(Class holder, Class type, BiCon try { consumer.accept(field.getName(), type.cast(field.get(null))); } catch (IllegalAccessException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } }