Skip to content

Commit

Permalink
Introduce deprecated overloads of clear mis-uses of Modules.override(…
Browse files Browse the repository at this point in the history
…) and Modules.combine() to help guide users away from these unnecessary calls.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=265810850
  • Loading branch information
dimo414 authored and kluever committed Aug 28, 2019
1 parent f253439 commit 35b9a3c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 7 deletions.
47 changes: 44 additions & 3 deletions core/src/com/google/inject/util/Modules.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,13 @@ public void configure(Binder binder) {}
* @param modules the modules whose bindings are open to be overridden
*/
public static OverriddenModuleBuilder override(Module... modules) {
return new RealOverriddenModuleBuilder(Arrays.asList(modules));
return override(Arrays.asList(modules));
}

/** @deprecated there's no reason to use {@code Modules.override()} without any arguments. */
@Deprecated
public static OverriddenModuleBuilder override() {
return override(Arrays.asList());
}

/**
Expand All @@ -104,12 +110,36 @@ public static OverriddenModuleBuilder override(Iterable<? extends Module> module
return new RealOverriddenModuleBuilder(modules);
}

/** Returns a new module that installs all of {@code modules}. */
/**
* Returns a new module that installs all of {@code modules}.
*
* <p>Although sometimes helpful, this method is rarely necessary. Most Guice APIs accept multiple
* arguments or (like {@code install()}) can be called repeatedly. Where possible, external APIs
* that require a single module should similarly be adapted to permit multiple modules.
*/
public static Module combine(Module... modules) {
return combine(ImmutableSet.copyOf(modules));
}

/** Returns a new module that installs all of {@code modules}. */
/** @deprecated there's no need to "combine" one module; just install it directly. */
@Deprecated
public static Module combine(Module module) {
return module;
}

/** @deprecated this method call is effectively a no-op, just remove it. */
@Deprecated
public static Module combine() {
return EMPTY_MODULE;
}

/**
* Returns a new module that installs all of {@code modules}.
*
* <p>Although sometimes helpful, this method is rarely necessary. Most Guice APIs accept multiple
* arguments or (like {@code install()}) can be called repeatedly. Where possible, external APIs
* that require a single module should similarly be adapted to permit multiple modules.
*/
public static Module combine(Iterable<? extends Module> modules) {
return new CombinedModule(modules);
}
Expand All @@ -136,13 +166,18 @@ public interface OverriddenModuleBuilder {
/** See the EDSL example at {@link Modules#override(Module[]) override()}. */
Module with(Module... overrides);

/** @deprecated there's no reason to use {@code .with()} without any arguments. */
@Deprecated
public Module with();

/** See the EDSL example at {@link Modules#override(Module[]) override()}. */
Module with(Iterable<? extends Module> overrides);
}

private static final class RealOverriddenModuleBuilder implements OverriddenModuleBuilder {
private final ImmutableSet<Module> baseModules;

// TODO(diamondm) checkArgument(!baseModules.isEmpty())?
private RealOverriddenModuleBuilder(Iterable<? extends Module> baseModules) {
this.baseModules = ImmutableSet.copyOf(baseModules);
}
Expand All @@ -152,6 +187,11 @@ public Module with(Module... overrides) {
return with(Arrays.asList(overrides));
}

@Override
public Module with() {
return with(Arrays.asList());
}

@Override
public Module with(Iterable<? extends Module> overrides) {
return new OverrideModule(overrides, baseModules);
Expand All @@ -162,6 +202,7 @@ static class OverrideModule extends AbstractModule {
private final ImmutableSet<Module> overrides;
private final ImmutableSet<Module> baseModules;

// TODO(diamondm) checkArgument(!overrides.isEmpty())?
OverrideModule(Iterable<? extends Module> overrides, ImmutableSet<Module> baseModules) {
this.overrides = ImmutableSet.copyOf(overrides);
this.baseModules = baseModules;
Expand Down
7 changes: 3 additions & 4 deletions core/test/com/google/inject/ModulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,15 @@ protected void configure() {
install(combined1);
}
};
final Module combined2 = Modules.combine(skipSourcesModule);
final Module combined2 = Modules.combine(skipSourcesModule); // returns skipSourcesModule
Injector injector = Guice.createInjector(combined2);
ElementSource source = (ElementSource) injector.getBinding(Integer.class).getSource();
assertEquals(4, source.getModuleClassNames().size());
assertEquals(3, source.getModuleClassNames().size());
assertEquals(
ImmutableList.of(
m1.getClass().getName(),
combined1.getClass().getName(),
skipSourcesModule.getClass().getName(),
combined2.getClass().getName()),
skipSourcesModule.getClass().getName()),
source.getModuleClassNames());
StackTraceElement stackTraceElement = (StackTraceElement) source.getDeclaringSource();
assertEquals(skipSourcesModule.getClass().getName(), stackTraceElement.getClassName());
Expand Down

0 comments on commit 35b9a3c

Please sign in to comment.