Resolved SPR-6618. Restrictions were too tight on overloaded bean methods and were preventing it altogether. Overloading is now allowed, as long as there is no ambiguity at runtime which bean method should be invoked.

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java

@@ -16,6 +16,7 @@
 
 package org.springframework.context.annotation;
 
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -48,12 +49,10 @@ final class ConfigurationClass {
 
 	private final Resource resource;
 
-	private final Map<String, Class> importedResources = new LinkedHashMap<String, Class>();
+	private final Map<String, Class<?>> importedResources = new LinkedHashMap<String, Class<?>>();
 
 	private final Set<ConfigurationClassMethod> methods = new LinkedHashSet<ConfigurationClassMethod>();
 
-	private final Map<String, Integer> overloadedMethodMap = new LinkedHashMap<String, Integer>();
-
 	private String beanName;
 
 
@@ -90,39 +89,43 @@ final class ConfigurationClass {
 		return this.beanName;
 	}
 
-	public ConfigurationClass addMethod(ConfigurationClassMethod method) {
+	public void addMethod(ConfigurationClassMethod method) {
 		this.methods.add(method);
-		String name = method.getMetadata().getMethodName();
-		Integer count = this.overloadedMethodMap.get(name);
-		if (count != null) {
-			this.overloadedMethodMap.put(name, count + 1);
-		}
-		else {
-			this.overloadedMethodMap.put(name, 1);
-		}
-		return this;
 	}
 
 	public Set<ConfigurationClassMethod> getMethods() {
 		return this.methods;
 	}
 
-	public void addImportedResource(String importedResource, Class readerClass) {
+	public void addImportedResource(String importedResource, Class<?> readerClass) {
 		this.importedResources.put(importedResource, readerClass);
 	}
 
-	public Map<String, Class> getImportedResources() {
+	public Map<String, Class<?>> getImportedResources() {
 		return this.importedResources;
 	}
 
 
 	public void validate(ProblemReporter problemReporter) {
-		// No overloading of factory methods allowed
+		
-		for (Map.Entry<String, Integer> entry : this.overloadedMethodMap.entrySet()) {
+		// a @Bean method may only be overloaded through inheritance. No single
-			String methodName = entry.getKey();
+		// @Configuration class may declare two @Bean methods with the same name.
-			int count = entry.getValue();
+		final char hashDelim = '#';
+		Map<String, Integer> methodNameCounts = new HashMap<String, Integer>();
+		for (ConfigurationClassMethod method : methods) {
+			String dClassName = method.getMetadata().getDeclaringClassName();
+			String methodName = method.getMetadata().getMethodName();
+			String fqMethodName = dClassName + hashDelim + methodName;
+			Integer currentCount = methodNameCounts.get(fqMethodName);
+			int newCount = currentCount != null ? currentCount + 1 : 1;
+			methodNameCounts.put(fqMethodName, newCount);
+		}
+		
+		for (String methodName : methodNameCounts.keySet()) {
+			int count = methodNameCounts.get(methodName);
 			if (count > 1) {
-				problemReporter.error(new OverloadedMethodProblem(methodName, count));
+				String shortMethodName = methodName.substring(methodName.indexOf(hashDelim)+1);
+				problemReporter.error(new BeanMethodOverloadingProblem(shortMethodName, count));
 			}
 		}
 		
@@ -149,7 +152,6 @@ final class ConfigurationClass {
 		return getMetadata().getClassName().hashCode();
 	}
 
-
 	/** Configuration classes must be non-final to accommodate CGLIB subclassing. */
 	private class FinalConfigurationProblem extends Problem {
 
@@ -159,15 +161,15 @@ final class ConfigurationClass {
 		}
 	}
 
+	/** Bean methods on configuration classes may only be overloaded through inheritance. */
+	private class BeanMethodOverloadingProblem extends Problem {
 
-	/** Factory methods on configuration classes must not be overloaded. */
+		public BeanMethodOverloadingProblem(String methodName, int count) {
-	private class OverloadedMethodProblem extends Problem {
+			super(String.format("@Configuration class '%s' has %s overloaded @Bean methods named '%s'. " +
-
+					"Only one @Bean method of a given name is allowed within each @Configuration class.",
-		public OverloadedMethodProblem(String methodName, int count) {
-			super(String.format("@Configuration class '%s' has %s overloaded factory methods of name '%s'. " +
-					"Only one factory method of the same name allowed.",
 					getSimpleName(), count, methodName), new Location(getResource(), getMetadata()));
 		}
 	}
 	
+
 }

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

@@ -119,7 +119,7 @@ class ConfigurationClassBeanDefinitionReader {
 	 * the BeanDefinitionRegistry based on its contents.
 	 */
 	private void loadBeanDefinitionsForModelMethod(ConfigurationClassMethod method) {
-		ConfigurationClass configClass = method.getDeclaringClass();
+		ConfigurationClass configClass = method.getConfigurationClass();
 		MethodMetadata metadata = method.getMetadata();
 
 		RootBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass);
@@ -213,11 +213,11 @@ class ConfigurationClassBeanDefinitionReader {
 		registry.registerBeanDefinition(beanName, beanDefToRegister);
 	}
 	
-	private void loadBeanDefinitionsFromImportedResources(Map<String, Class> importedResources) {
+	private void loadBeanDefinitionsFromImportedResources(Map<String, Class<?>> importedResources) {
-		Map<Class, BeanDefinitionReader> readerInstanceCache = new HashMap<Class, BeanDefinitionReader>();
+		Map<Class<?>, BeanDefinitionReader> readerInstanceCache = new HashMap<Class<?>, BeanDefinitionReader>();
-		for (Map.Entry<String, Class> entry : importedResources.entrySet()) {
+		for (Map.Entry<String, Class<?>> entry : importedResources.entrySet()) {
 			String resource = entry.getKey();
-			Class readerClass = entry.getValue();
+			Class<?> readerClass = entry.getValue();
 			if (!readerInstanceCache.containsKey(readerClass)) {
 				try {
 					BeanDefinitionReader readerInstance = (BeanDefinitionReader)

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassMethod.java

@@ -16,6 +16,8 @@
 
 package org.springframework.context.annotation;
 
+import static java.lang.String.format;
+
 import org.springframework.beans.factory.parsing.Location;
 import org.springframework.beans.factory.parsing.Problem;
 import org.springframework.beans.factory.parsing.ProblemReporter;
@@ -35,33 +37,38 @@ final class ConfigurationClassMethod {
 
 	private final MethodMetadata metadata;
 
-	private final ConfigurationClass declaringClass;
+	private final ConfigurationClass configurationClass;
 
 
-	public ConfigurationClassMethod(MethodMetadata metadata, ConfigurationClass declaringClass) {
+	public ConfigurationClassMethod(MethodMetadata metadata, ConfigurationClass configurationClass) {
 		this.metadata = metadata;
-		this.declaringClass = declaringClass;
+		this.configurationClass = configurationClass;
 	}
 
-
 	public MethodMetadata getMetadata() {
 		return this.metadata;
 	}
 
-	public ConfigurationClass getDeclaringClass() {
+	public ConfigurationClass getConfigurationClass() {
-		return this.declaringClass;
+		return this.configurationClass;
 	}
 
 	public Location getResourceLocation() {
-		return new Location(this.declaringClass.getResource(), metadata);
+		return new Location(this.configurationClass.getResource(), metadata);
 	}
 
 	public void validate(ProblemReporter problemReporter) {
-		if (this.declaringClass.getMetadata().isAnnotated(Configuration.class.getName()) && !getMetadata().isOverridable()) {
+		if (this.configurationClass.getMetadata().isAnnotated(Configuration.class.getName()) && !getMetadata().isOverridable()) {
 			problemReporter.error(new NonOverridableMethodError());
 		}
 	}
 	
+	@Override
+	public String toString() {
+		return format("[%s:name=%s,declaringClass=%s]",
+				this.getClass().getSimpleName(), this.getMetadata().getMethodName(), this.getMetadata().getDeclaringClassName());
+	}
+
 
 	/**
 	 * {@link Bean} methods must be overridable in order to accommodate CGLIB.
@@ -74,4 +81,5 @@ final class ConfigurationClassMethod {
 		}
 	}
 
+
 }

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

@@ -131,7 +131,7 @@ class ConfigurationClassParser {
 		}
 		if (metadata.isAnnotated(ImportResource.class.getName())) {
 			String[] resources = (String[]) metadata.getAnnotationAttributes(ImportResource.class.getName()).get("value");
-			Class readerClass = (Class) metadata.getAnnotationAttributes(ImportResource.class.getName()).get("reader");
+			Class<?> readerClass = (Class<?>) metadata.getAnnotationAttributes(ImportResource.class.getName()).get("reader");
 			if (readerClass == null) {
 				throw new IllegalStateException("No reader class associated with imported resources: " +
 						StringUtils.arrayToCommaDelimitedString(resources));
@@ -140,8 +140,8 @@ class ConfigurationClassParser {
 				configClass.addImportedResource(resource, readerClass);
 			}
 		}
-		Set<MethodMetadata> methods = metadata.getAnnotatedMethods(Bean.class.getName());
+		Set<MethodMetadata> beanMethods = metadata.getAnnotatedMethods(Bean.class.getName());
-		for (MethodMetadata methodMetadata : methods) {
+		for (MethodMetadata methodMetadata : beanMethods) {
 			configClass.addMethod(new ConfigurationClassMethod(methodMetadata, configClass));
 		}
 	}

org.springframework.context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java

@@ -0,0 +1,71 @@
+package org.springframework.context.annotation;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
+
+/**
+ * Tests regarding overloading and overriding of bean methods.
+ * Related to SPR-6618.
+ *
+ * Bean-annotated methods should be able to be overridden, just as any regular
+ * method. This is straightforward.
+ *
+ * Bean-annotated methods should be able to be overloaded, though supporting this
+ * is more subtle. Essentially, it must be unambiguous to the container which bean
+ * method to call.  A simple way to think about this is that no one Configuration
+ * class may declare two bean methods with the same name.  In the case of inheritance,
+ * the most specific subclass bean method will always be the one that is invoked.
+ *
+ * @author Chris Beams
+ */
+public class BeanMethodPolymorphismTests {
+
+	@Test
+	public void beanMethodOverloadingWithoutInheritance() {
+		@SuppressWarnings("unused")
+		@Configuration class Config {
+			@Bean String aString() { return "na"; }
+			@Bean String aString(Integer dependency) { return "na"; }
+		}
+		try {
+			new AnnotationConfigApplicationContext(Config.class);
+			fail("expected bean method overloading exception");
+		} catch (BeanDefinitionParsingException ex) {
+			assertTrue(ex.getMessage(), ex.getMessage().contains("2 overloaded @Bean methods named 'aString'"));
+		}
+	}
+
+	@Test
+	public void beanMethodOverloadingWithInheritance() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfig.class);
+		assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
+	}
+	static @Configuration class SuperConfig {
+		@Bean String aString() { return "super"; }
+	}
+	static @Configuration class SubConfig {
+		@Bean Integer anInt() { return 5; }
+		@Bean String aString(Integer dependency) { return "overloaded"+dependency; }
+	}
+	
+	/**
+	 * When inheritance is not involved, it is still possible to override a bean method from
+	 * the container's point of view. This is not strictly 'overloading' of a method per se,
+	 * so it's referred to here as 'shadowing' to distinguish the difference.
+	 */
+	@Test
+	public void beanMethodShadowing() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ShadowConfig.class);
+		assertThat(ctx.getBean(String.class), equalTo("shadow"));
+	}
+	@Import(SubConfig.class)
+	static @Configuration class ShadowConfig {
+		@Bean String aString() { return "shadow"; }
+	}
+
+}

org.springframework.core/src/main/java/org/springframework/core/type/MethodMetadata.java

@@ -24,6 +24,7 @@ import java.util.Map;
  *
  * @author Juergen Hoeller
  * @author Mark Pollack
+ * @author Chris Beams
  * @since 3.0
  * @see StandardMethodMetadata
  * @see AnnotationMetadata#getAnnotatedMethods
@@ -35,6 +36,11 @@ public interface MethodMetadata {
 	 */
 	String getMethodName();
 
+	/**
+	 * Return the fully-qualified name of the class that declares this method.
+	 */
+	public String getDeclaringClassName();
+
 	/**
 	 * Return whether the underlying method is declared as 'static'.
 	 */

org.springframework.core/src/main/java/org/springframework/core/type/StandardMethodMetadata.java

@@ -30,6 +30,7 @@ import org.springframework.util.Assert;
  *
  * @author Juergen Hoeller
  * @author Mark Pollack
+ * @author Chris Beams
  * @since 3.0
  */
 public class StandardMethodMetadata implements MethodMetadata {
@@ -58,6 +59,10 @@ public class StandardMethodMetadata implements MethodMetadata {
 		return this.introspectedMethod.getName();
 	}
 	
+	public String getDeclaringClassName() {
+		return this.introspectedMethod.getDeclaringClass().getName();
+	}
+
 	public boolean isStatic() {
 		return Modifier.isStatic(this.introspectedMethod.getModifiers());
 	}

org.springframework.core/src/main/java/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.java

@@ -57,7 +57,7 @@ final class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisitor
 
 	@Override
 	public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
-		MethodMetadataReadingVisitor mm = new MethodMetadataReadingVisitor(name, access, this.classLoader);
+		MethodMetadataReadingVisitor mm = new MethodMetadataReadingVisitor(name, access, this.getClassName(), this.classLoader);
 		this.methodMetadataSet.add(mm);
 		return mm;
 	}

org.springframework.core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java

@@ -41,15 +41,18 @@ final class MethodMetadataReadingVisitor extends MethodAdapter implements Method
 
 	private final int access;
 
+	private String declaringClassName;
+
 	private final ClassLoader classLoader;
 
 	private final Map<String, Map<String, Object>> attributeMap = new LinkedHashMap<String, Map<String, Object>>();
 
 
-	public MethodMetadataReadingVisitor(String name, int access, ClassLoader classLoader) {
+	public MethodMetadataReadingVisitor(String name, int access, String declaringClassName, ClassLoader classLoader) {
 		super(new EmptyVisitor());
 		this.name = name;
 		this.access = access;
+		this.declaringClassName = declaringClassName;
 		this.classLoader = classLoader;
 	}
 
@@ -85,4 +88,8 @@ final class MethodMetadataReadingVisitor extends MethodAdapter implements Method
 		return this.attributeMap.get(annotationType);
 	}
 
+	public String getDeclaringClassName() {
+		return this.declaringClassName;
+	}
+
 }