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); - } -}