Big general cleanup (#2)

Introduce Environment#putPropertyIfAbsent
Delegates to ConcurrentHashMap#putIfAbsent, allows avoiding unnecessary allocating lambda usages

build.gradle

@@ -80,7 +80,7 @@ jar {
     }
 }
 
-tasks.withType(JavaCompile) {
+tasks.withType(JavaCompile).configureEach {
     options.encoding = 'UTF-8'
     options.compilerArgs << '-Xlint:unchecked'
 }

settings.gradle

@@ -6,13 +6,13 @@ pluginManagement {
 }
 
 plugins {
-    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.7.0'
+    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.8.0'
 }
 
 dependencyResolutionManagement {
     versionCatalogs {
         libs {
-            version('asm', '9.6')
+            version('asm', '9.7.1')
             library('asm',         'org.ow2.asm', 'asm'        ).versionRef('asm')
             library('asm-tree',    'org.ow2.asm', 'asm-tree'   ).versionRef('asm')
             library('asm-commons', 'org.ow2.asm', 'asm-commons').versionRef('asm')

src/main/java/cpw/mods/modlauncher/ArgumentHandler.java

@@ -55,7 +55,7 @@ void processArguments(Environment env, Consumer<OptionParser> parserConsumer, Bi
         env.computePropertyIfAbsent(IEnvironment.Keys.ASSETSDIR.get(), f -> this.optionSet.valueOf(assetsDirOption));
         env.computePropertyIfAbsent(IEnvironment.Keys.LAUNCHTARGET.get(), f -> this.optionSet.valueOf(launchTarget));
         env.computePropertyIfAbsent(IEnvironment.Keys.UUID.get(), f -> this.optionSet.valueOf(uuidOption));
-        resultConsumer.accept(this.optionSet, this::optionResults);
+        resultConsumer.accept(this.optionSet, ArgumentHandler::optionResults);
     }
 
     Path[] getSpecialJars() {
@@ -66,7 +66,7 @@ String getLaunchTarget() {
         return this.optionSet.valueOf(launchTarget);
     }
 
-    private ITransformationService.OptionResult optionResults(String serviceName, OptionSet set) {
+    private static ITransformationService.OptionResult optionResults(String serviceName, OptionSet set) {
         return new ITransformationService.OptionResult() {
             @Override
             public <V> V value(OptionSpec<V> option) {
@@ -95,11 +95,11 @@ public String[] buildArgumentList() {
         addOptionToString(assetsDirOption, optionSet, args);
         addOptionToString(uuidOption, optionSet, args);
         List<?> nonOptionList = this.optionSet.nonOptionArguments();
-        nonOptionList.stream().map(Object::toString).forEach(args::add);
+        args.addAll(nonOptionList.stream().map(Object::toString).toList());
         return args.toArray(new String[0]);
     }
 
-    private void addOptionToString(OptionSpec<?> option, OptionSet optionSet, List<String> appendTo) {
+    private static void addOptionToString(OptionSpec<?> option, OptionSet optionSet, List<String> appendTo) {
         if (optionSet.has(option)) {
             appendTo.add("--"+option.options().get(0));
             appendTo.add(option.value(optionSet).toString());

src/main/java/cpw/mods/modlauncher/ClassTransformer.java

@@ -27,7 +27,7 @@
 public class ClassTransformer {
     private static final byte[] EMPTY = new byte[0];
     private static final Logger LOGGER = LogManager.getLogger();
-    private final Marker CLASSDUMP = MarkerManager.getMarker("CLASSDUMP");
+    private static final Marker CLASSDUMP = MarkerManager.getMarker("CLASSDUMP");
     private final TransformStore transformers;
     private final LaunchPluginHandler pluginHandler;
     private final TransformingClassLoader transformingClassLoader;
@@ -125,7 +125,7 @@ byte[] transform(byte[] inputClass, String className, final String reason) {
     }
 
     private static Path tempDir;
-    private void dumpClass(final byte[] clazz, String className) {
+    private static void dumpClass(final byte[] clazz, String className) {
         if (tempDir == null) {
             synchronized (ClassTransformer.class) {
                 if (tempDir == null) {
@@ -150,24 +150,27 @@ private void dumpClass(final byte[] clazz, String className) {
     private <T> T performVote(List<ITransformer<T>> transformers, T node, VotingContext context) {
         context.setNode(node);
         do {
-            final Stream<TransformerVote<T>> voteResultStream = transformers.stream().map(t -> gatherVote(t, context));
-            final Map<TransformerVoteResult, List<TransformerVote<T>>> results = voteResultStream.collect(Collectors.groupingBy(TransformerVote::getResult));
+            EnumMap<TransformerVoteResult, List<TransformerVote<T>>> results = gatherVotes(transformers, context);
+
             // Someone rejected the current state. We're done here, and cannot proceed.
-            if (results.containsKey(TransformerVoteResult.REJECT)) {
+            if (results.containsKey(TransformerVoteResult.REJECT))
                 throw new VoteRejectedException(results.get(TransformerVoteResult.REJECT), node.getClass());
-            }
+
             // Remove all the "NO" voters - they don't wish to participate in further voting rounds
-            if (results.containsKey(TransformerVoteResult.NO)) {
-                transformers.removeAll(results.get(TransformerVoteResult.NO).stream().map(TransformerVote::getTransformer).collect(Collectors.toList()));
-            }
+            var noVotes = results.get(TransformerVoteResult.NO);
+            if (noVotes != null)
+                transformers.removeAll(noVotes.stream().map(TransformerVote::transformer).toList());
+
             // If there's at least one YES voter, let's apply the first one we find, remove them, and continue.
-            if (results.containsKey(TransformerVoteResult.YES)) {
-                final ITransformer<T> transformer = results.get(TransformerVoteResult.YES).get(0).getTransformer();
+            var yesVotes = results.get(TransformerVoteResult.YES);
+            if (yesVotes != null) {
+                final ITransformer<T> transformer = yesVotes.get(0).transformer();
                 node = transformer.transform(node, context);
-                auditTrail.addTransformerAuditTrail(context.getClassName(), ((TransformerHolder<?>)transformer).owner(), transformer);
+                auditTrail.addTransformerAuditTrail(context.getClassName(), ((TransformerHolder<?>) transformer).owner(), transformer);
                 transformers.remove(transformer);
                 continue;
             }
+
             // If we get here and find a DEFER, it means everyone just voted to DEFER. That's an untenable state and we cannot proceed.
             if (results.containsKey(TransformerVoteResult.DEFER)) {
                 throw new VoteDeadlockException(results.get(TransformerVoteResult.DEFER), node.getClass());
@@ -177,17 +180,32 @@ private <T> T performVote(List<ITransformer<T>> transformers, T node, VotingCont
         return node;
     }
 
-    private <T> TransformerVote<T> gatherVote(ITransformer<T> transformer, VotingContext context) {
-        TransformerVoteResult vr = transformer.castVote(context);
-        return new TransformerVote<>(vr, transformer);
+    private static <T> EnumMap<TransformerVoteResult, List<TransformerVote<T>>> gatherVotes(List<ITransformer<T>> transformers, VotingContext context) {
+        var results = new EnumMap<TransformerVoteResult, List<TransformerVote<T>>>(TransformerVoteResult.class);
+        for (ITransformer<T> transformer : transformers) {
+            var voteResult = transformer.castVote(context);
+            results.computeIfAbsent(voteResult, k -> new ArrayList<>())
+                    .add(new TransformerVote<>(voteResult, transformer));
+        }
+        return results;
     }
 
-    private MessageDigest getSha256() {
-        try {
-            return MessageDigest.getInstance("SHA-256");
-        } catch (NoSuchAlgorithmException e) {
-            throw new RuntimeException("HUH");
+    private static MessageDigest getSha256() {
+        final class LazyInit {
+            private static final MessageDigest SHA256;
+
+            static {
+                try {
+                    SHA256 = MessageDigest.getInstance("SHA-256");
+                } catch (NoSuchAlgorithmException e) {
+                    throw new RuntimeException("HUH");
+                }
+            }
+
+            private LazyInit() {}
         }
+
+        return LazyInit.SHA256;
     }
 
     TransformingClassLoader getTransformingClassLoader() {

src/main/java/cpw/mods/modlauncher/EnumerationHelper.java

@@ -36,6 +36,6 @@ public static <T> T firstElementOrNull(final Enumeration<T> enumeration) {
     }
 
     public static <T> Function<String, Enumeration<T>> fromOptional(final Function<String, Optional<T>> additionalClassBytesLocator) {
-        return input -> Collections.enumeration(additionalClassBytesLocator.apply(input).map(Stream::of).orElseGet(Stream::empty).collect(Collectors.toList()));
+        return input -> Collections.enumeration(additionalClassBytesLocator.apply(input).stream().toList());
     }
 }

src/main/java/cpw/mods/modlauncher/Environment.java

@@ -11,6 +11,7 @@
 import java.util.*;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 /**
  * Environment implementation class
@@ -53,4 +54,9 @@ public Optional<BiFunction<INameMappingService.Domain, String, String>> findName
     public <T> T computePropertyIfAbsent(final TypesafeMap.Key<T> key, final Function<? super TypesafeMap.Key<T>, ? extends T> valueFunction) {
         return environment.computeIfAbsent(key, valueFunction);
     }
+
+    @Override
+    public <T> T putPropertyIfAbsent(final TypesafeMap.Key<T> key, final T value) {
+        return environment.putIfAbsent(key, value);
+    }
 }

src/main/java/cpw/mods/modlauncher/LaunchPluginHandler.java

@@ -81,7 +81,9 @@ public EnumMap<Phase, List<ILaunchPluginService>> computeLaunchPluginTransformer
     }
 
     void offerScanResultsToPlugins(List<SecureJar> scanResults) {
-        plugins.values().forEach(p -> p.addResources(scanResults));
+        for (ILaunchPluginService p : plugins.values()) {
+            p.addResources(scanResults);
+        }
     }
 
     int offerClassNodeToPlugins(Phase phase, List<ILaunchPluginService> plugins, @Nullable ClassNode node, Type className, TransformerAuditTrail auditTrail, String reason) {

src/main/java/cpw/mods/modlauncher/LaunchServiceHandler.java

@@ -29,7 +29,7 @@
 /**
  * Identifies the launch target and dispatches to it
  */
-class LaunchServiceHandler {
+final class LaunchServiceHandler {
     private static final Logger LOGGER = LogManager.getLogger();
     private final Map<String, ILaunchHandlerService> handlers = new HashMap<>();
 
@@ -66,7 +66,7 @@ private void launch(String target, String[] arguments, ModuleLayer gameLayer, Tr
             try {
                 var virtual = lookup.findVirtual(service.getClass(), "launchService", type);
                 Callable<Void> callable = (Callable<Void>)virtual.invokeExact(arguments, gameLayer);
-                runner = () -> callable.call();
+                runner = callable::call;
             } catch (Throwable t) {
                 sneak(t);
             }
@@ -83,7 +83,7 @@ private void launch(String target, String[] arguments, ModuleLayer gameLayer, Tr
     static List<String> hideAccessToken(String[] arguments) {
         var output = new ArrayList<String>();
         for (int i = 0; i < arguments.length; i++) {
-            if (i > 0 && Objects.equals(arguments[i-1], "--accessToken"))
+            if (i > 0 && "--accessToken".equals(arguments[i - 1]))
                 output.add("**********");
             else
                 output.add(arguments[i]);

src/main/java/cpw/mods/modlauncher/Launcher.java

@@ -45,15 +45,15 @@ public class Launcher {
 
     private Launcher() {
         INSTANCE = this;
-        LOGGER.info(MODLAUNCHER,"ModLauncher {} starting: java version {} by {}; OS {} arch {} version {}", ()->IEnvironment.class.getPackage().getImplementationVersion(),  () -> System.getProperty("java.version"), ()->System.getProperty("java.vendor"), ()->System.getProperty("os.name"), ()->System.getProperty("os.arch"), ()->System.getProperty("os.version"));
+        LOGGER.info(MODLAUNCHER,"ModLauncher {} starting: java version {} by {}; OS {} arch {} version {}", IEnvironment.class.getPackage().getImplementationVersion(),  System.getProperty("java.version"), System.getProperty("java.vendor"), System.getProperty("os.name"), System.getProperty("os.arch"), System.getProperty("os.version"));
         this.moduleLayerHandler = new ModuleLayerHandler();
         this.launchService = new LaunchServiceHandler(this.moduleLayerHandler);
         this.blackboard = new TypesafeMap();
         this.environment = new Environment(this);
-        environment.computePropertyIfAbsent(IEnvironment.Keys.MLSPEC_VERSION.get(), s->IEnvironment.class.getPackage().getSpecificationVersion());
-        environment.computePropertyIfAbsent(IEnvironment.Keys.MLIMPL_VERSION.get(), s->IEnvironment.class.getPackage().getImplementationVersion());
-        environment.computePropertyIfAbsent(IEnvironment.Keys.MODLIST.get(), s->new ArrayList<>());
-        environment.computePropertyIfAbsent(IEnvironment.Keys.SECURED_JARS_ENABLED.get(), k-> ProtectionDomainHelper.canHandleSecuredJars());
+        environment.putPropertyIfAbsent(IEnvironment.Keys.MLSPEC_VERSION.get(), IEnvironment.class.getPackage().getSpecificationVersion());
+        environment.putPropertyIfAbsent(IEnvironment.Keys.MLIMPL_VERSION.get(), IEnvironment.class.getPackage().getImplementationVersion());
+        environment.computePropertyIfAbsent(IEnvironment.Keys.MODLIST.get(), k -> new ArrayList<>());
+        environment.putPropertyIfAbsent(IEnvironment.Keys.SECURED_JARS_ENABLED.get(), ProtectionDomainHelper.canHandleSecuredJars());
         this.transformStore = new TransformStore();
         this.transformationServicesHandler = new TransformationServicesHandler(this.transformStore, this.moduleLayerHandler);
         this.argumentHandler = new ArgumentHandler();
@@ -70,7 +70,7 @@ public static void main(String... args) {
             JVM information: %s %s %s
             """, props.getProperty("java.vm.vendor"), props.getProperty("java.vm.name"), props.getProperty("java.vm.version"));
         }
-        LOGGER.info(MODLAUNCHER,"ModLauncher running: args {}", () -> LaunchServiceHandler.hideAccessToken(args));
+        LOGGER.info(MODLAUNCHER,"ModLauncher running: args {}", LaunchServiceHandler.hideAccessToken(args));
         LOGGER.info(MODLAUNCHER, "JVM identified as {} {} {}", props.getProperty("java.vm.vendor"), props.getProperty("java.vm.name"), props.getProperty("java.vm.version"));
         new Launcher().run(args);
     }

src/main/java/cpw/mods/modlauncher/LogMarkers.java

@@ -7,8 +7,10 @@
 
 import org.apache.logging.log4j.*;
 
-class LogMarkers {
+final class LogMarkers {
     static final Marker MODLAUNCHER = MarkerManager.getMarker("MODLAUNCHER");
-    static final Marker CLASSLOADING = MarkerManager.getMarker("CLASSLOADING").addParents(MODLAUNCHER);
+//    static final Marker CLASSLOADING = MarkerManager.getMarker("CLASSLOADING").addParents(MODLAUNCHER);
     static final Marker LAUNCHPLUGIN = MarkerManager.getMarker("LAUNCHPLUGIN").addParents(MODLAUNCHER);
+
+    private LogMarkers() {}
 }

src/main/java/cpw/mods/modlauncher/ModuleLayerHandler.java

@@ -58,7 +58,7 @@ LayerInfo build(Layer layer) {
     }
 
     LayerInfo build(Layer layer, ClassLoaderFactory classLoaderSupplier) {
-        var jars = layers.getOrDefault(layer, List.of()).stream().toArray(SecureJar[]::new);
+        var jars = layers.getOrDefault(layer, List.of()).toArray(SecureJar[]::new);
         var targets = Arrays.stream(jars).map(SecureJar::name).toList();
         var parentLayers = new ArrayList<ModuleLayer>();
         var parentConfigs = new ArrayList<Configuration>();

src/main/java/cpw/mods/modlauncher/NameMappingServiceHandler.java

@@ -21,7 +21,7 @@
 /**
  * Allow names to be transformed between naming domains.
  */
-class NameMappingServiceHandler {
+final class NameMappingServiceHandler {
     private static final Logger LOGGER = LogManager.getLogger();
     private final Map<String, INameMappingService> allKnown = new HashMap<>();
     private final Map<String, INameMappingService> active = new HashMap<>();

src/main/java/cpw/mods/modlauncher/PredicateVisitor.java

@@ -14,23 +14,29 @@
 public class PredicateVisitor extends ClassVisitor {
     private static final int ASM_API = Opcodes.ASM9;
 
-    private ITransformerVotingContext.MethodPredicate methodPredicate;
-    private ITransformerVotingContext.FieldPredicate fieldPredicate;
-    private ITransformerVotingContext.ClassPredicate classPredicate;
+    private final ITransformerVotingContext.MethodPredicate methodPredicate;
+    private final ITransformerVotingContext.FieldPredicate fieldPredicate;
+    private final ITransformerVotingContext.ClassPredicate classPredicate;
     private boolean result;
 
     PredicateVisitor(final ITransformerVotingContext.FieldPredicate fieldPredicate) {
         super(ASM_API);
+        this.methodPredicate = null;
         this.fieldPredicate = fieldPredicate;
+        this.classPredicate = null;
     }
 
     PredicateVisitor(final ITransformerVotingContext.MethodPredicate methodPredicate) {
         super(ASM_API);
         this.methodPredicate = methodPredicate;
+        this.fieldPredicate = null;
+        this.classPredicate = null;
     }
 
     PredicateVisitor(final ITransformerVotingContext.ClassPredicate classPredicate) {
         super(ASM_API);
+        this.methodPredicate = null;
+        this.fieldPredicate = null;
         this.classPredicate = classPredicate;
     }
 

src/main/java/cpw/mods/modlauncher/TransformTargetLabel.java

@@ -5,7 +5,6 @@
 
 package cpw.mods.modlauncher;
 
-
 import org.objectweb.asm.Type;
 import org.objectweb.asm.tree.ClassNode;
 import org.objectweb.asm.tree.FieldNode;
@@ -25,20 +24,22 @@
  * Detailed targetting information
  */
 public final class TransformTargetLabel {
-
     private final Type className;
     private final String elementName;
     private final Type elementDescriptor;
     private final LabelType labelType;
+
     TransformTargetLabel(ITransformer.Target target) {
-        this(target.getClassName(), target.getElementName(), target.getElementDescriptor(), LabelType.valueOf(target.getTargetType().name()));
+        this(target.className(), target.elementName(), target.elementDescriptor(), LabelType.valueOf(target.targetType().name()));
     }
+
     private TransformTargetLabel(String className, String elementName, String elementDescriptor, LabelType labelType) {
         this.className = Type.getObjectType(className.replace('.', '/'));
         this.elementName = elementName;
-        this.elementDescriptor = elementDescriptor.length() > 0 ? Type.getMethodType(elementDescriptor) : Type.VOID_TYPE;
+        this.elementDescriptor = !elementDescriptor.isEmpty() ? Type.getMethodType(elementDescriptor) : Type.VOID_TYPE;
         this.labelType = labelType;
     }
+
     public TransformTargetLabel(String className, String fieldName) {
         this(className, fieldName, "", FIELD);
     }
@@ -58,19 +59,19 @@ public TransformTargetLabel(String className, LabelType type) {
             throw new IllegalArgumentException("Invalid type " + type + ", must be for class!");
     }
 
-    final Type getClassName() {
+    Type getClassName() {
         return this.className;
     }
 
-    public final String getElementName() {
+    public String getElementName() {
         return this.elementName;
     }
 
-    public final Type getElementDescriptor() {
+    public Type getElementDescriptor() {
         return this.elementDescriptor;
     }
 
-    final LabelType getLabelType() {
+    LabelType getLabelType() {
         return this.labelType;
     }
 
@@ -80,14 +81,11 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object obj) {
-        try {
-            TransformTargetLabel tl = (TransformTargetLabel) obj;
-            return Objects.equals(this.className, tl.className)
-                    && Objects.equals(this.elementName, tl.elementName)
-                    && Objects.equals(this.elementDescriptor, tl.elementDescriptor);
-        } catch (ClassCastException cce) {
-            return false;
-        }
+        if (obj == this) return true;
+        return obj instanceof TransformTargetLabel tl
+                && Objects.equals(this.className, tl.className)
+                && Objects.equals(this.elementName, tl.elementName)
+                && Objects.equals(this.elementDescriptor, tl.elementDescriptor);
     }
 
     @Override