From d224ce116ddf8251b4dc22e09a5ed98f0ba90463 Mon Sep 17 00:00:00 2001 From: Algent Date: Sat, 15 Aug 2026 20:04:55 +0200 Subject: [PATCH] refactor: remove the package objects instead of porting them A Scala package object compiles to a class literally named 'package', and that is a Java keyword, so no Java source can declare or even name it. The characterization had to reach both through Class.forName, which is the argument in one line. There is no Java form to port to. Each held exactly one member, a logger alias for its proxy, so the indirection is inlined at its eight call sites: ConfigContent (7) and MultipartGenerator (1). The functional characterization asserted beforehand that each alias returned the very instance its proxy holds, so the substitution is an identity; both classes are descriptor-identical afterwards. Exactly four classes leave the jar and nothing else changes. No consumer referenced them, and no Java consumer could have. 120 plain-JVM tests and 22 Forge server tests pass. PackageObjectCharacterization went with its subject; ProxyLoggerFunctionalTest keeps the assertion worth having, that both proxies publish a logger during preInit. --- JAVA_MIGRATION.md | 4 ++ JAVA_MIGRATION_DIVERGENCES.md | 40 ++++++++++++++ JAVA_MIGRATION_HANDOFF.md | 15 +++-- .../test/ProxyLoggerFunctionalTest.java | 21 +------ .../microblock/ConfigContent.scala | 18 +++--- .../codechicken/microblock/package.scala | 7 --- .../multipart/MultipartGenerator.scala | 4 +- .../scala/codechicken/multipart/package.scala | 7 --- .../PackageObjectCharacterizationTest.java | 55 ------------------- 9 files changed, 71 insertions(+), 100 deletions(-) delete mode 100644 src/main/scala/codechicken/microblock/package.scala delete mode 100644 src/main/scala/codechicken/multipart/package.scala delete mode 100644 src/test/java/codechicken/multipart/PackageObjectCharacterizationTest.java diff --git a/JAVA_MIGRATION.md b/JAVA_MIGRATION.md index aae40c7..3a58e66 100644 --- a/JAVA_MIGRATION.md +++ b/JAVA_MIGRATION.md @@ -535,3 +535,7 @@ These ranges were formed before designing the characterization suite and should - Converted the first Scala `object` that extends a class rather than being a plain singleton. `ControlKeyHandler` is now an ordinary `KeyBinding` subclass with an `INSTANCE` constant, which drops the twelve static forwarders Scala generated for inherited `KeyBinding` members along with the companion. The registration sites pass `ControlKeyHandler.INSTANCE`. - Found that a Forge `FakePlayer` makes the server half of player-dependent code testable. `FakePlayerFactory.getMinecraft(worldServer)` yields a player whose world is not remote, which is exactly the branch the server takes, and it needs no connection. - Recorded that the client half remains untestable in both harnesses. `isClientPressing` is only ever set from a client tick, so the key binding, the press/release packet and the client branch of `isControlDown` are all manual checks. +- Removed both package objects rather than porting them. A Scala package object compiles to a class named `package`, which is a Java keyword, so no Java source can declare or even name it; the characterization had to reach both through `Class.forName`. Each held one member, a `logger` alias for its proxy, so the indirection was inlined at its eight call sites instead of being given an invented Java home. +- Proved the substitution was an identity before making it. The functional test asserted that each alias returned the very instance its proxy holds, so replacing `logger` with `MultipartProxy.logger` / `MicroblockProxy.logger` cannot change behavior. `ConfigContent` and `MultipartGenerator` are descriptor-identical afterwards. +- Noted the first case where the migration's answer is deletion. Zero downstream references plus an impossible target name means there is nothing to preserve, and inventing a `MultipartLogger` type to hold a one-line forwarder would add API rather than remove it. +- Emptied the low-risk queue. Everything remaining is the medium group or later, so the next piece of work should be chosen deliberately rather than taken off the top of a list. diff --git a/JAVA_MIGRATION_DIVERGENCES.md b/JAVA_MIGRATION_DIVERGENCES.md index f2c28fa..ef77a0e 100644 --- a/JAVA_MIGRATION_DIVERGENCES.md +++ b/JAVA_MIGRATION_DIVERGENCES.md @@ -1128,3 +1128,43 @@ did. Both are reached solely from the client proxy's `postInit`, so nothing chan - What stays manual, and it is most of the client half: the key binding appearing in the controls screen, the packet actually reaching the server on press and release, and `isControlDown` reporting the pressed state on a client world. No harness has a client, so `isClientPressing` is never exercised. + +## 2026-08-15 — Package objects removed rather than ported + +`codechicken.multipart.package` and `codechicken.microblock.package`. This entry records a **removal**, not a +conversion, because a conversion is impossible. + +### Why they have no Java form + +A Scala package object compiles to a class literally named `package`, plus its `package$` companion. `package` is a +Java keyword, so no Java source can declare or even name those types. The characterization test had to reach both of +them through `Class.forName`, which is the whole argument in one line. + +That left three options: leave two six-line Scala files in the tree forever, invent a Java type to hold the member, or +remove the indirection. Each package object held exactly one member — `def logger = MultipartProxy.logger` and +`def logger = MicroblockProxy.logger` — so the third is the honest one. The alias existed only to let Scala code inside +the package write `logger` unqualified; Java has no such sugar, and the proxies already publish the value. + +### Observable behavior + +None changed. The functional characterization asserted before the removal that each alias returned the *very instance* +its proxy holds, so replacing `logger` with `MultipartProxy.logger` / `MicroblockProxy.logger` at the call sites is an +identity substitution. The eight call sites are `ConfigContent` (7) and `MultipartGenerator` (1); both classes are +descriptor-identical after the change, since only method bodies moved. + +### Removed API + +`codechicken/multipart/package`, `codechicken/multipart/package$`, `codechicken/microblock/package` and +`codechicken/microblock/package$`. Exactly those four classes leave the jar and nothing else changes. No jar in the +pack references any of them, in bytecode or by reflective name — no Java consumer *could* reference them by name, and +no Scala consumer did. + +### Validation + +- Complete plain-JVM suite: 120 tests, 0 failures, 0 errors. +- Clean `spotlessApply checkstyleTest build`: passing. +- Java 8 Forge dedicated-server suite: 22 tests, 0 failures, 0 errors. +- Class list diffed against a reference jar built from the pre-removal `src/main`: exactly four classes removed. +- Test churn, both expected for a removal rather than a port: `PackageObjectCharacterizationTest` was deleted with its + subject, and `ProxyLoggerFunctionalTest` lost only its alias-identity assertion. What survives is the assertion worth + keeping — that both proxies publish a non-null logger during preInit, which is what the inlined call sites now read. diff --git a/JAVA_MIGRATION_HANDOFF.md b/JAVA_MIGRATION_HANDOFF.md index 79d094c..c7d3e77 100644 --- a/JAVA_MIGRATION_HANDOFF.md +++ b/JAVA_MIGRATION_HANDOFF.md @@ -10,7 +10,7 @@ what breaks, and what is left. The other documents hold the reasoning. | `JAVA_MIGRATION_DIVERGENCES.md` | Every intentional difference from the reference, one entry per port | | `JAVA_MIGRATION_MANUAL_CHECKS.md` | What no automated test can cover, and must be checked by hand in a client | -Branch: `algent/java`. Base: `master`. 50 commits so far. +Branch: `algent/java`. Base: `master`. 52 commits so far. ## The one rule that matters @@ -56,7 +56,7 @@ awk -F'"' '/ MMap} import net.minecraft.block.Block import net.minecraft.item.Item import java.lang.Exception +import codechicken.microblock.handler.MicroblockProxy import cpw.mods.fml.common.event.FMLInterModComms.IMCMessage import net.minecraft.item.ItemStack import BlockMicroMaterial.createAndRegister @@ -27,7 +28,8 @@ object ConfigContent { else loadLines(cfgFile) } catch { - case e: IOException => logger.error("Error parsing config", e) + case e: IOException => + MicroblockProxy.logger.error("Error parsing config", e) } } @@ -95,8 +97,10 @@ object ConfigContent { loadLine(s) } catch { case e: Exception => - logger.error("Invalid line in microblocks.cfg: " + s) - logger.error(e.getMessage) + MicroblockProxy.logger.error( + "Invalid line in microblocks.cfg: " + s + ) + MicroblockProxy.logger.error(e.getMessage) } } } while (s != null) @@ -114,12 +118,12 @@ object ConfigContent { createAndRegister(block, m) } catch { case e: IllegalStateException => - logger.error( + MicroblockProxy.logger.error( "Unable to register micro material: " + materialKey(block, m) + "\n\t" + e.getMessage ) case e: Exception => - logger.error( + MicroblockProxy.logger.error( "Unable to register micro material: " + materialKey(block, m), e ) @@ -128,7 +132,7 @@ object ConfigContent { } nameMap.foreach(e => - logger.warn( + MicroblockProxy.logger.warn( "Unable to add micro material for block with unlocalised name " + e._1 + " as it doesn't exist" ) ) @@ -137,7 +141,7 @@ object ConfigContent { def handleIMC(messages: Seq[IMCMessage]) { messages.filter(_.key == "microMaterial").foreach { msg => def error(s: String) { - logger.error( + MicroblockProxy.logger.error( "Invalid microblock IMC message from " + msg.getSender + ": " + s ) } diff --git a/src/main/scala/codechicken/microblock/package.scala b/src/main/scala/codechicken/microblock/package.scala deleted file mode 100644 index e346958..0000000 --- a/src/main/scala/codechicken/microblock/package.scala +++ /dev/null @@ -1,7 +0,0 @@ -package codechicken - -import codechicken.microblock.handler.MicroblockProxy - -package object microblock { - def logger = MicroblockProxy.logger -} diff --git a/src/main/scala/codechicken/multipart/MultipartGenerator.scala b/src/main/scala/codechicken/multipart/MultipartGenerator.scala index 5bcaef6..2220f95 100644 --- a/src/main/scala/codechicken/multipart/MultipartGenerator.scala +++ b/src/main/scala/codechicken/multipart/MultipartGenerator.scala @@ -185,7 +185,9 @@ object MultipartGenerator extends ScratchBitSet { traitName != null ) { if (map.contains(s_interface)) - logger.error("Trait already registered for " + s_interface) + MultipartProxy.logger.error( + "Trait already registered for " + s_interface + ) else { map.put(s_interface, traitName) MultipartMixinFactory.registerTrait(traitName) diff --git a/src/main/scala/codechicken/multipart/package.scala b/src/main/scala/codechicken/multipart/package.scala deleted file mode 100644 index 646cfa0..0000000 --- a/src/main/scala/codechicken/multipart/package.scala +++ /dev/null @@ -1,7 +0,0 @@ -package codechicken - -import codechicken.multipart.handler.MultipartProxy - -package object multipart { - def logger = MultipartProxy.logger -} diff --git a/src/test/java/codechicken/multipart/PackageObjectCharacterizationTest.java b/src/test/java/codechicken/multipart/PackageObjectCharacterizationTest.java deleted file mode 100644 index 5c967e6..0000000 --- a/src/test/java/codechicken/multipart/PackageObjectCharacterizationTest.java +++ /dev/null @@ -1,55 +0,0 @@ -package codechicken.multipart; - -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; - -import org.apache.logging.log4j.Logger; -import org.junit.jupiter.api.Test; - -/** - * Both package objects hold one member, a {@code logger} alias for their proxy's logger. - *

- * Everything here goes through reflection because {@code package} is a Java keyword: - * {@code codechicken.multipart.package} cannot be written as a type name in Java source at all. That is not a style - * choice, it is the reason these two classes have no Java form. - */ -class PackageObjectCharacterizationTest { - - @Test - void bothPackageObjectsExposeAStaticLoggerAccessor() throws Exception { - for (String name : new String[] { "codechicken.multipart.package", "codechicken.microblock.package" }) { - Method logger = Class.forName(name).getDeclaredMethod("logger"); - - assertSame(Logger.class, logger.getReturnType(), name); - assertTrue(Modifier.isPublic(logger.getModifiers()), name); - assertTrue(Modifier.isStatic(logger.getModifiers()), name); - } - } - - @Test - void bothCarryACompanionSingleton() throws Exception { - for (String name : new String[] { "codechicken.multipart.package$", "codechicken.microblock.package$" }) { - Class companion = Class.forName(name); - - assertSame(companion, companion.getField("MODULE$").getType(), name); - } - } - - /** - * The alias reads the proxy's var rather than holding anything, so before preInit has run it reports null instead - * of failing. The Forge server suite asserts the other half, that after preInit it is the very same instance. - */ - @Test - void theAliasReportsWhateverTheProxyHoldsAndIsNullBeforePreInit() throws Exception { - assertNull(invokeLogger("codechicken.multipart.package")); - assertNull(invokeLogger("codechicken.microblock.package")); - } - - private static Object invokeLogger(String name) throws Exception { - return Class.forName(name).getDeclaredMethod("logger").invoke(null); - } -}