Don't detect private constructors on member classes for binding

Refine constructor binding detection logic so that `private`
constructors on member classes are no longer automatically picked for
constructor binding. This provides users a way of signalling that
they wish to use the constructor directly.

Closes gh-32639
This commit is contained in:
Phillip Webb 2022-10-18 00:45:05 -07:00
parent 754f39e6ef
commit a34b1d3c6e
5 changed files with 102 additions and 9 deletions

View File

@ -24,6 +24,8 @@ import java.util.stream.Stream;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.NestingKind;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror;
@ -186,24 +188,30 @@ class PropertyDescriptorResolver {
static ConfigurationPropertiesTypeElement of(TypeElement type, MetadataGenerationEnvironment env) {
List<ExecutableElement> constructors = ElementFilter.constructorsIn(type.getEnclosedElements());
List<ExecutableElement> boundConstructors = getBoundConstructors(env, constructors);
List<ExecutableElement> boundConstructors = getBoundConstructors(type, env, constructors);
return new ConfigurationPropertiesTypeElement(type, constructors, boundConstructors);
}
private static List<ExecutableElement> getBoundConstructors(MetadataGenerationEnvironment env,
private static List<ExecutableElement> getBoundConstructors(TypeElement type, MetadataGenerationEnvironment env,
List<ExecutableElement> constructors) {
ExecutableElement bindConstructor = deduceBindConstructor(constructors, env);
ExecutableElement bindConstructor = deduceBindConstructor(type, constructors, env);
if (bindConstructor != null) {
return Collections.singletonList(bindConstructor);
}
return constructors.stream().filter(env::hasConstructorBindingAnnotation).toList();
}
private static ExecutableElement deduceBindConstructor(List<ExecutableElement> constructors,
private static ExecutableElement deduceBindConstructor(TypeElement type, List<ExecutableElement> constructors,
MetadataGenerationEnvironment env) {
if (constructors.size() == 1 && constructors.get(0).getParameters().size() > 0
&& !env.hasAutowiredAnnotation(constructors.get(0))) {
return constructors.get(0);
if (constructors.size() == 1) {
ExecutableElement candidate = constructors.get(0);
if (candidate.getParameters().size() > 0 && !env.hasAutowiredAnnotation(constructors.get(0))) {
if (type.getNestingKind() == NestingKind.MEMBER
&& candidate.getModifiers().contains(Modifier.PRIVATE)) {
return null;
}
return candidate;
}
}
return null;
}

View File

@ -31,6 +31,7 @@ import org.springframework.boot.configurationsample.simple.DescriptionProperties
import org.springframework.boot.configurationsample.simple.HierarchicalProperties;
import org.springframework.boot.configurationsample.simple.HierarchicalPropertiesGrandparent;
import org.springframework.boot.configurationsample.simple.HierarchicalPropertiesParent;
import org.springframework.boot.configurationsample.simple.InnerClassWithPrivateConstructor;
import org.springframework.boot.configurationsample.simple.NotAnnotated;
import org.springframework.boot.configurationsample.simple.SimpleArrayProperties;
import org.springframework.boot.configurationsample.simple.SimpleCollectionProperties;
@ -459,4 +460,11 @@ class ConfigurationMetadataAnnotationProcessorTests extends AbstractMetadataGene
assertThat(metadata).doesNotHave(Metadata.withProperty("multi.some-integer"));
}
@Test
void innerClassWithPrivateConstructor() {
ConfigurationMetadata metadata = compile(InnerClassWithPrivateConstructor.class);
assertThat(metadata).has(Metadata.withProperty("config.nested.name"));
assertThat(metadata).doesNotHave(Metadata.withProperty("config.nested.ignored"));
}
}

View File

@ -0,0 +1,56 @@
/*
* Copyright 2012-2022 the original author or authors.
*
* 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
*
* https://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 org.springframework.boot.configurationsample.simple;
import org.springframework.boot.configurationsample.ConfigurationProperties;
/**
* Nested properties with a private constructor.
*
* @author Phillip Webb
*/
@ConfigurationProperties(prefix = "config")
public class InnerClassWithPrivateConstructor {
private Nested nested = new Nested("whatever");
public Nested getNested() {
return this.nested;
}
public void setNested(Nested nested) {
this.nested = nested;
}
public static final class Nested {
private String name;
private Nested(String ignored) {
}
public String getName() {
return this.name;
}
public void setName(String name) {
this.name = name;
}
}
}

View File

@ -80,7 +80,7 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
boolean hasAutowiredConstructor = isAutowiredPresent(candidateAnnotations);
Constructor<?> bind = getConstructorBindingAnnotated(type, candidates, candidateAnnotations);
if (bind == null && !hasAutowiredConstructor) {
bind = deduceBindConstructor(candidates);
bind = deduceBindConstructor(type, candidates);
}
if (bind == null && !hasAutowiredConstructor && isKotlinType(type)) {
bind = deduceKotlinBindConstructor(type);
@ -142,8 +142,11 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
}
private static Constructor<?> deduceBindConstructor(Constructor<?>[] candidates) {
private static Constructor<?> deduceBindConstructor(Class<?> type, Constructor<?>[] candidates) {
if (candidates.length == 1 && candidates[0].getParameterCount() > 0) {
if (type.isMemberClass() && Modifier.isPrivate(candidates[0].getModifiers())) {
return null;
}
return candidates[0];
}
Constructor<?> result = null;

View File

@ -90,6 +90,13 @@ class DefaultBindConstructorProviderTests {
.withMessageContaining("has more than one @ConstructorBinding");
}
@Test
void getBindConstructorWhenIsMemberTypeWithPrivateConstructorReturnsNull() {
Constructor<?> constructor = this.provider.getBindConstructor(MemberTypeWithPrivateConstructor.Member.class,
false);
assertThat(constructor).isNotNull();
}
static class OnlyDefaultConstructor {
}
@ -170,4 +177,15 @@ class DefaultBindConstructorProviderTests {
}
static class MemberTypeWithPrivateConstructor {
static final class Member {
private Member(String name) {
}
}
}
}