Prioritize plugins own bindings

forge/src/launch/java/org/spongepowered/forge/launch/plugin/PluginModContainer.java

@@ -41,7 +41,7 @@
 import net.minecraftforge.forgespi.language.ModFileScanData;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.spongepowered.common.inject.plugin.PluginModule;
+import org.spongepowered.common.inject.plugin.PluginGuice;
 import org.spongepowered.common.launch.Launch;
 import org.spongepowered.forge.launch.event.ForgeEventManager;
 
@@ -84,7 +84,7 @@ private void constructPlugin() {
             LOGGER.trace(Logging.LOADING, "Loading plugin instance {} of type {}", getModId(), this.modClass.getName());
 
             final ForgePluginContainer pluginContainer = ForgePluginContainer.of(this);
-            final Injector pluginInjector = PluginModule.create(pluginContainer, this.modClass, Launch.instance().lifecycle().platformInjector());
+            final Injector pluginInjector = PluginGuice.create(pluginContainer, this.modClass, Launch.instance().lifecycle().platformInjector());
             this.modInstance = pluginInjector.getInstance(this.modClass);
             pluginContainer.initializeInstance(pluginInjector);
             ((ForgeEventManager) MinecraftForge.EVENT_BUS).registerListeners(pluginContainer, this.modInstance);

src/main/java/org/spongepowered/common/inject/plugin/BindingHelper.java

@@ -72,12 +72,12 @@ void bindFrom(final Injector fromInjector) {
                 continue;
             }
 
-            this.bind(fromInjector, binding);
+            this.bindFrom(fromInjector, binding);
         }
     }
 
     @SuppressWarnings("rawtypes")
-    void bind(final Injector fromInjector, final Binding binding) {
+    void bindFrom(final Injector fromInjector, final Binding binding) {
         if (binding instanceof final ExposedBinding<?> exposedBinding) {
             final PrivateElements privateElements = exposedBinding.getPrivateElements();
             for (final Element privateElement : privateElements.getElements()) {
@@ -86,17 +86,18 @@ void bind(final Injector fromInjector, final Binding binding) {
                     continue;
                 }
 
-                this.bind(fromInjector, privateBinding);
+                this.bindFrom(fromInjector, privateBinding);
             }
 
             return;
         }
 
-        this.bind(fromInjector, binding.getKey());
+        this.bind(fromInjector, binding);
     }
 
     @SuppressWarnings({"rawtypes", "unchecked"})
-    private void bind(final Injector fromInjector, final Key key) {
+    private void bind(final Injector fromInjector, final Binding<?> binding) {
+        final Key key = binding.getKey();
         final Class<?> clazz = key.getTypeLiteral().getRawType();
         if (Iterable.class.isAssignableFrom(clazz)) {
             final Collection destinationCollection;

src/main/java/org/spongepowered/common/inject/plugin/PluginGuice.java

@@ -0,0 +1,56 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.common.inject.plugin;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.util.Modules;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.spongepowered.plugin.PluginContainer;
+
+public final class PluginGuice {
+
+    public static Injector create(final PluginContainer container, final Class<?> pluginClass, final @Nullable Injector platformInjector) {
+        Module module = Modules.combine(new PrivatePluginModule(container, pluginClass), new PublicPluginModule(container));
+
+        final @Nullable Object customModule = container.metadata().property("guice-module").orElse(null);
+        if (customModule != null) {
+            try {
+                final Class<?> moduleClass = Class.forName(customModule.toString(), true, pluginClass.getClassLoader());
+                final Module moduleInstance = (Module) moduleClass.getConstructor().newInstance();
+                module = Modules.override(module).with(moduleInstance);
+            } catch (final Exception ex) {
+                throw new RuntimeException("Failed to instantiate the custom module!", ex);
+            }
+        }
+
+        if (platformInjector != null) {
+            return platformInjector.createChildInjector(module);
+        } else {
+            return Guice.createInjector(module);
+        }
+    }
+}

src/main/java/org/spongepowered/common/inject/plugin/PrivatePluginModule.java

@@ -0,0 +1,65 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.common.inject.plugin;
+
+import com.google.inject.PrivateModule;
+import com.google.inject.Scopes;
+import org.apache.logging.log4j.Logger;
+import org.spongepowered.common.inject.InjectionPointProvider;
+import org.spongepowered.common.inject.provider.PluginConfigurationModule;
+import org.spongepowered.plugin.PluginContainer;
+
+
+/**
+ * A module installed for each plugin.
+ *
+ * Contains the values that are not shared for dependencies or
+ * that require access to the private resources, like the plugin class.
+ */
+public final class PrivatePluginModule extends PrivateModule {
+
+    private final PluginContainer container;
+    private final Class<?> pluginClass;
+
+    PrivatePluginModule(final PluginContainer container, final Class<?> pluginClass) {
+        this.container = container;
+        this.pluginClass = pluginClass;
+    }
+
+    @Override
+    protected void configure() {
+        this.bind(this.pluginClass).in(Scopes.SINGLETON);
+        this.expose(this.pluginClass);
+
+        this.install(new InjectionPointProvider());
+
+        this.bind(PluginContainer.class).toInstance(this.container);
+        this.bind(Logger.class).toInstance(this.container.logger());
+        this.bind(System.Logger.class).toProvider(() -> System.getLogger(this.container.logger().getName())).in(Scopes.SINGLETON);
+
+        this.install(new PluginConfigurationModule());
+    }
+
+}

src/main/java/org/spongepowered/common/inject/plugin/PluginModule.java → src/main/java/org/spongepowered/common/inject/plugin/PublicPluginModule.java

@@ -24,47 +24,28 @@
  */
 package org.spongepowered.common.inject.plugin;
 
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import com.google.inject.Module;
-import com.google.inject.PrivateModule;
-import com.google.inject.Scopes;
-import org.apache.logging.log4j.Logger;
-import org.checkerframework.checker.nullness.qual.Nullable;
+import com.google.inject.AbstractModule;
 import org.spongepowered.api.Sponge;
-import org.spongepowered.common.inject.InjectionPointProvider;
 import org.spongepowered.common.inject.SpongePluginInjectorProvider;
-import org.spongepowered.common.inject.provider.PluginConfigurationModule;
 import org.spongepowered.plugin.PluginContainer;
 import org.spongepowered.plugin.metadata.model.PluginDependency;
 
-import java.util.ArrayList;
-
 /**
  * A module installed for each plugin.
+ * Contains the values that are publicly visible and shared
+ * across dependencies.
  */
-public final class PluginModule extends PrivateModule {
+public final class PublicPluginModule extends AbstractModule {
 
     private final PluginContainer container;
-    private final Class<?> pluginClass;
 
-    private PluginModule(final PluginContainer container, final Class<?> pluginClass) {
+    PublicPluginModule(final PluginContainer container) {
         this.container = container;
-        this.pluginClass = pluginClass;
     }
 
     @Override
     protected void configure() {
-        this.bind(this.pluginClass).in(Scopes.SINGLETON);
-        this.expose(this.pluginClass);
-
-        this.install(new InjectionPointProvider());
-
-        this.bind(PluginContainer.class).toInstance(this.container);
-        this.bind(Logger.class).toInstance(this.container.logger());
-        this.bind(System.Logger.class).toProvider(() -> System.getLogger(this.container.logger().getName())).in(Scopes.SINGLETON);
-
-        this.install(new PluginConfigurationModule());
+        this.requestStaticInjection(PreserveHelper.class);
 
         final BindingHelper bindingHelper = new BindingHelper(this.binder());
         for (final PluginDependency dependency : this.container.metadata().dependencies()) {
@@ -97,25 +78,11 @@ protected void configure() {
         bindingHelper.bind();
     }
 
-    public static Injector create(final PluginContainer container, final Class<?> pluginClass, final @Nullable Injector platformInjector) {
-        final ArrayList<Module> modules = new ArrayList<>(2);
-        modules.add(new PluginModule(container, pluginClass));
-
-        final @Nullable Object customModule = container.metadata().property("guice-module").orElse(null);
-        if (customModule != null) {
-            try {
-                final Class<?> moduleClass = Class.forName(customModule.toString(), true, pluginClass.getClassLoader());
-                final com.google.inject.Module moduleInstance = (com.google.inject.Module) moduleClass.getConstructor().newInstance();
-                modules.add(moduleInstance);
-            } catch (final Exception ex) {
-                throw new RuntimeException("Failed to instantiate the custom module!", ex);
-            }
-        }
-
-        if (platformInjector != null) {
-            return platformInjector.createChildInjector(modules);
-        } else {
-            return Guice.createInjector(modules);
-        }
+    /**
+     * If no public module has any bindings, Guice will silently promote
+     * the private module as the "main" one which leads to everything
+     * being marked as private, including the plugin provided custom module.
+     */
+    private static final class PreserveHelper {
     }
 }

testplugins/src/main/java/org/spongepowered/test/serviceloader/ParentModuleTestPlugin.java

@@ -24,7 +24,9 @@
  */
 package org.spongepowered.test.serviceloader;
 
+import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
+import com.google.inject.multibindings.Multibinder;
 import org.apache.logging.log4j.Logger;
 import org.spongepowered.api.Sponge;
 import org.spongepowered.plugin.builtin.jvm.Plugin;
@@ -58,4 +60,12 @@ public ParentModuleTestPlugin(final Logger logger, final ChildModuleTestPlugin c
             logger.error("Mismatched content of combinablePluginServices");
         }
     }
+
+    public static final class Module extends AbstractModule {
+
+        @Override
+        protected void configure() {
+            Multibinder.newSetBinder(this.binder(), CombinableTestPluginService.class);
+        }
+    }
 }

testplugins/src/main/resources/META-INF/sponge_plugins.json

@@ -364,6 +364,9 @@
             "name": "Parent Module Tests",
             "entrypoint": "org.spongepowered.test.serviceloader.ParentModuleTestPlugin",
             "description": "Parent Module Tests",
+            "properties": {
+                "guice-module": "org.spongepowered.test.serviceloader.ParentModuleTestPlugin$Module"
+            },
             "dependencies": [
                 {
                     "id": "childmoduletestplugin",

vanilla/src/launch/java/org/spongepowered/vanilla/launch/plugin/JavaPluginLoader.java

@@ -27,7 +27,7 @@
 import com.google.inject.Injector;
 import org.apache.maven.artifact.versioning.ArtifactVersion;
 import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
-import org.spongepowered.common.inject.plugin.PluginModule;
+import org.spongepowered.common.inject.plugin.PluginGuice;
 import org.spongepowered.common.launch.Launch;
 import org.spongepowered.plugin.Environment;
 import org.spongepowered.plugin.InvalidPluginException;
@@ -51,7 +51,7 @@ public VanillaJavaPluginContainer loadPlugin(final Environment environment, fina
             final String mainClass = container.metadata().entrypoint();
             final Class<?> pluginClass = Class.forName(mainClass, true, targetClassLoader);
 
-            final Injector pluginInjector = PluginModule.create(container, pluginClass, Launch.instance().lifecycle().platformInjector());
+            final Injector pluginInjector = PluginGuice.create(container, pluginClass, Launch.instance().lifecycle().platformInjector());
             final Object plugin = pluginInjector.getInstance(pluginClass);
             container.initializeInstance(plugin, pluginInjector);
             return container;