Skip to content

Commit

Permalink
Fix auto factory issue where multiple copies of the same method can b…
Browse files Browse the repository at this point in the history
…e generated when inherited from an interface.

Caveats:
parameter names have to match perfectly.  If an interface declares the exact same method with differently named params we will choke as before.
everything needs to be public.  Interface methods are inherently public, so if we have a collision on a create method, we can't downgrade its visibility to package-private.  I think we can automatically upgrade it though if you'd like.
I'm not emitting @OverRide or anything fancy like that
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=113658842
  • Loading branch information
oni authored and cgruber committed Mar 4, 2016
1 parent ebf4d1d commit 9e78da9
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.Map.Entry;

/**
Expand Down Expand Up @@ -58,7 +59,8 @@ public boolean matches(char c) {
this.implementingTypes = checkNotNull(implementingTypes);
this.publicType = publicType;
this.methodDescriptors = checkNotNull(methodDescriptors);
this.implementationMethodDescriptors = checkNotNull(implementationMethodDescriptors);
this.implementationMethodDescriptors = dedupeMethods(
methodDescriptors, implementationMethodDescriptors);
this.allowSubclasses = allowSubclasses;
ImmutableSetMultimap.Builder<Key, String> providerNamesBuilder = ImmutableSetMultimap.builder();
for (FactoryMethodDescriptor descriptor : methodDescriptors) {
Expand Down Expand Up @@ -115,4 +117,26 @@ ImmutableMap<Key, String> providerNames() {
boolean allowSubclasses() {
return allowSubclasses;
}

/** Removes methods with matching signatures from the set of ImplmentationMethods. */
private static ImmutableSet<ImplementationMethodDescriptor> dedupeMethods(
ImmutableSet<FactoryMethodDescriptor> methodDescriptors,
ImmutableSet<ImplementationMethodDescriptor> implementationMethodDescriptors) {

checkNotNull(implementationMethodDescriptors);
LinkedHashSet<ImplementationMethodDescriptor> dedupedMethods =
new LinkedHashSet<ImplementationMethodDescriptor>(implementationMethodDescriptors);

for (ImplementationMethodDescriptor implementationMethod : implementationMethodDescriptors) {
for (FactoryMethodDescriptor factoryMethod : methodDescriptors) {
if (implementationMethod.name().equals(factoryMethod.name())
&& Iterables.elementsEqual(
implementationMethod.passedParameters(), factoryMethod.passedParameters())) {
dedupedMethods.remove(implementationMethod);
break;
}
}
}
return ImmutableSet.copyOf(dedupedMethods);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -325,4 +325,16 @@ public void simpleClassProvidedProviderDeps() {
.and().generatesSources(
JavaFileObjects.forResource("expected/ClassUsingQualifierWithArgsFactory.java"));
}

@Test public void factoryImplementingInterfaceWhichRedeclaresCreateMethods() {
JavaFileObject file =
JavaFileObjects.forResource("good/FactoryImplementingCreateMethod.java");
assertAbout(javaSource())
.that(file)
.processedWith(new AutoFactoryProcessor())
.compilesWithoutError()
.and().generatesSources(
JavaFileObjects.forResource(
"expected/FactoryImplementingCreateMethod_ConcreteClassFactory.java"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (C) 2016 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package tests;

import javax.annotation.Generated;
import javax.inject.Inject;
import tests.FactoryImplementingCreateMethod.FactoryInterfaceWithCreateMethod;

@Generated("com.google.auto.factory.processor.AutoFactoryProcessor")
public final class FactoryImplementingCreateMethod_ConcreteClassFactory implements
FactoryInterfaceWithCreateMethod {

@Inject
public FactoryImplementingCreateMethod_ConcreteClassFactory() {}

public FactoryImplementingCreateMethod.ConcreteClass create() {
return new FactoryImplementingCreateMethod.ConcreteClass();
}

public FactoryImplementingCreateMethod.ConcreteClass create(int a) {
return new FactoryImplementingCreateMethod.ConcreteClass(a);
}

public FactoryImplementingCreateMethod.ConcreteClass create(int a, boolean b) {
return new FactoryImplementingCreateMethod.ConcreteClass(a, b);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2016 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package tests;

import com.google.auto.factory.AutoFactory;

final class FactoryImplementingCreateMethod {

interface Interface {}

interface FactoryInterfaceWithCreateMethod {
Interface create();

// Parameters names have to match unfortunately.
Interface create(int a);
}

@AutoFactory(implementing = FactoryInterfaceWithCreateMethod.class)
public static class ConcreteClass implements Interface {
// Will generate a method with a signature that matches one from the interface.
ConcreteClass() {}

// Will generate a method with a signature that matches one from the interface.
ConcreteClass(int a) {}

ConcreteClass(int a, boolean b) {}
}
}

0 comments on commit 9e78da9

Please sign in to comment.