From 850a0de1b0c6aec5ff9e2e4573df9498e5b7171e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Wed, 31 Jul 2024 16:48:18 +0200 Subject: [PATCH] Suppress deprecated warnings for autowiring This commit extends our handling of deprecated with AOT to autowiring. Closes gh-33295 --- .../AutowiredAnnotationBeanPostProcessor.java | 39 +++++--- .../beans/factory/aot/CodeWarnings.java | 38 ++++++-- ...nBeanRegistrationAotContributionTests.java | 88 ++++++++++++++++++- .../beans/factory/aot/CodeWarningsTests.java | 60 ++++++++++--- .../DeprecatedInjectionSamples.java | 74 ++++++++++++++++ 5 files changed, 261 insertions(+), 38 deletions(-) create mode 100644 spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/factory/annotation/DeprecatedInjectionSamples.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java index ae45631192..cbfe62cdaf 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java @@ -63,6 +63,7 @@ import org.springframework.beans.factory.aot.AutowiredMethodArgumentsResolver; import org.springframework.beans.factory.aot.BeanRegistrationAotContribution; import org.springframework.beans.factory.aot.BeanRegistrationAotProcessor; import org.springframework.beans.factory.aot.BeanRegistrationCode; +import org.springframework.beans.factory.aot.CodeWarnings; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.DependencyDescriptor; import org.springframework.beans.factory.config.SmartInstantiationAwareBeanPostProcessor; @@ -984,8 +985,11 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA method.addParameter(RegisteredBean.class, REGISTERED_BEAN_PARAMETER); method.addParameter(this.target, INSTANCE_PARAMETER); method.returns(this.target); - method.addCode(generateMethodCode(generatedClass.getName(), - generationContext.getRuntimeHints())); + CodeWarnings codeWarnings = new CodeWarnings(); + codeWarnings.detectDeprecation(this.target); + method.addCode(generateMethodCode(codeWarnings, + generatedClass.getName(), generationContext.getRuntimeHints())); + codeWarnings.suppress(method); }); beanRegistrationCode.addInstancePostProcessor(generateMethod.toMethodReference()); @@ -994,35 +998,37 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA } } - private CodeBlock generateMethodCode(ClassName targetClassName, RuntimeHints hints) { + private CodeBlock generateMethodCode(CodeWarnings codeWarnings, + ClassName targetClassName, RuntimeHints hints) { + CodeBlock.Builder code = CodeBlock.builder(); for (AutowiredElement autowiredElement : this.autowiredElements) { code.addStatement(generateMethodStatementForElement( - targetClassName, autowiredElement, hints)); + codeWarnings, targetClassName, autowiredElement, hints)); } code.addStatement("return $L", INSTANCE_PARAMETER); return code.build(); } - private CodeBlock generateMethodStatementForElement(ClassName targetClassName, - AutowiredElement autowiredElement, RuntimeHints hints) { + private CodeBlock generateMethodStatementForElement(CodeWarnings codeWarnings, + ClassName targetClassName, AutowiredElement autowiredElement, RuntimeHints hints) { Member member = autowiredElement.getMember(); boolean required = autowiredElement.required; if (member instanceof Field field) { return generateMethodStatementForField( - targetClassName, field, required, hints); + codeWarnings, targetClassName, field, required, hints); } if (member instanceof Method method) { return generateMethodStatementForMethod( - targetClassName, method, required, hints); + codeWarnings, targetClassName, method, required, hints); } throw new IllegalStateException( "Unsupported member type " + member.getClass().getName()); } - private CodeBlock generateMethodStatementForField(ClassName targetClassName, - Field field, boolean required, RuntimeHints hints) { + private CodeBlock generateMethodStatementForField(CodeWarnings codeWarnings, + ClassName targetClassName, Field field, boolean required, RuntimeHints hints) { hints.reflection().registerField(field); CodeBlock resolver = CodeBlock.of("$T.$L($S)", @@ -1033,18 +1039,22 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA return CodeBlock.of("$L.resolveAndSet($L, $L)", resolver, REGISTERED_BEAN_PARAMETER, INSTANCE_PARAMETER); } - return CodeBlock.of("$L.$L = $L.resolve($L)", INSTANCE_PARAMETER, - field.getName(), resolver, REGISTERED_BEAN_PARAMETER); + else { + codeWarnings.detectDeprecation(field); + return CodeBlock.of("$L.$L = $L.resolve($L)", INSTANCE_PARAMETER, + field.getName(), resolver, REGISTERED_BEAN_PARAMETER); + } } - private CodeBlock generateMethodStatementForMethod(ClassName targetClassName, - Method method, boolean required, RuntimeHints hints) { + private CodeBlock generateMethodStatementForMethod(CodeWarnings codeWarnings, + ClassName targetClassName, Method method, boolean required, RuntimeHints hints) { CodeBlock.Builder code = CodeBlock.builder(); code.add("$T.$L", AutowiredMethodArgumentsResolver.class, (!required ? "forMethod" : "forRequiredMethod")); code.add("($S", method.getName()); if (method.getParameterCount() > 0) { + codeWarnings.detectDeprecation(method.getParameterTypes()); code.add(", $L", generateParameterTypesCode(method.getParameterTypes())); } code.add(")"); @@ -1054,6 +1064,7 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA code.add(".resolveAndInvoke($L, $L)", REGISTERED_BEAN_PARAMETER, INSTANCE_PARAMETER); } else { + codeWarnings.detectDeprecation(method); hints.reflection().registerMethod(method, ExecutableMode.INTROSPECT); CodeBlock arguments = new AutowiredArgumentsCodeGenerator(this.target, method).generateCode(method.getParameterTypes()); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/CodeWarnings.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/CodeWarnings.java index 108c0d6cc7..115f6427db 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/CodeWarnings.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/CodeWarnings.java @@ -21,12 +21,16 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; import java.util.StringJoiner; +import java.util.function.Consumer; import java.util.stream.Stream; import org.springframework.core.ResolvableType; import org.springframework.javapoet.AnnotationSpec; +import org.springframework.javapoet.AnnotationSpec.Builder; import org.springframework.javapoet.CodeBlock; +import org.springframework.javapoet.FieldSpec; import org.springframework.javapoet.MethodSpec; +import org.springframework.javapoet.TypeSpec; import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; @@ -38,7 +42,7 @@ import org.springframework.util.ClassUtils; * @since 6.1 * @see SuppressWarnings */ -class CodeWarnings { +public class CodeWarnings { private final Set warnings = new LinkedHashSet<>(); @@ -99,10 +103,31 @@ class CodeWarnings { * @param method the method to update */ public void suppress(MethodSpec.Builder method) { - if (this.warnings.isEmpty()) { - return; + suppress(annotationBuilder -> method.addAnnotation(annotationBuilder.build())); + } + + /** + * Include {@link SuppressWarnings} on the specified type if necessary. + * @param type the type to update + */ + public void suppress(TypeSpec.Builder type) { + suppress(annotationBuilder -> type.addAnnotation(annotationBuilder.build())); + } + + /** + * Consume the builder for {@link SuppressWarnings} if necessary. If this + * instance has no warnings registered, the consumer is not invoked. + * @param annotationSpec a consumer of the {@link AnnotationSpec.Builder} + * @see MethodSpec.Builder#addAnnotation(AnnotationSpec) + * @see TypeSpec.Builder#addAnnotation(AnnotationSpec) + * @see FieldSpec.Builder#addAnnotation(AnnotationSpec) + */ + protected void suppress(Consumer annotationSpec) { + if (!this.warnings.isEmpty()) { + Builder annotation = AnnotationSpec.builder(SuppressWarnings.class) + .addMember("value", generateValueCode()); + annotationSpec.accept(annotation); } - method.addAnnotation(buildAnnotationSpec()); } /** @@ -134,11 +159,6 @@ class CodeWarnings { } } - private AnnotationSpec buildAnnotationSpec() { - return AnnotationSpec.builder(SuppressWarnings.class) - .addMember("value", generateValueCode()).build(); - } - private CodeBlock generateValueCode() { if (this.warnings.size() == 1) { return CodeBlock.of("$S", this.warnings.iterator().next()); diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java index 96b9ae4492..0e38a02da2 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanRegistrationAotContributionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -21,6 +21,7 @@ import java.util.function.BiFunction; import javax.lang.model.element.Modifier; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.aot.generate.MethodReference; @@ -28,9 +29,17 @@ import org.springframework.aot.generate.MethodReference.ArgumentCodeGenerator; import org.springframework.aot.hint.predicate.RuntimeHintsPredicates; import org.springframework.aot.test.generate.TestGenerationContext; import org.springframework.beans.factory.aot.BeanRegistrationAotContribution; +import org.springframework.beans.factory.aot.CodeWarnings; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RegisteredBean; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedFieldInjectionPointSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedFieldInjectionTypeSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedMethodInjectionPointSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedMethodInjectionTypeSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedPrivateFieldInjectionTypeSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedPrivateMethodInjectionTypeSample; +import org.springframework.beans.testfixture.beans.factory.annotation.DeprecatedInjectionSamples.DeprecatedSample; import org.springframework.beans.testfixture.beans.factory.annotation.PackagePrivateFieldInjectionSample; import org.springframework.beans.testfixture.beans.factory.annotation.PackagePrivateMethodInjectionSample; import org.springframework.beans.testfixture.beans.factory.annotation.PrivateFieldInjectionSample; @@ -49,6 +58,7 @@ import org.springframework.javapoet.MethodSpec; import org.springframework.javapoet.ParameterizedTypeName; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; /** * Tests for {@link AutowiredAnnotationBeanPostProcessor} for AOT contributions. @@ -199,6 +209,69 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { assertThat(contribution).isNull(); } + @Nested + @SuppressWarnings("deprecation") + class DeprecationTests { + + private static final TestCompiler TEST_COMPILER = TestCompiler.forSystem() + .withCompilerOptions("-Xlint:all", "-Xlint:-rawtypes", "-Werror"); + + @Test + void contributeWhenTargetClassIsDeprecated() { + RegisteredBean registeredBean = getAndApplyContribution(DeprecatedSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenFieldInjectionsUsesADeprecatedType() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedFieldInjectionTypeSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenFieldInjectionsUsesADeprecatedTypeWithReflection() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedPrivateFieldInjectionTypeSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenFieldInjectionsIsDeprecated() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedFieldInjectionPointSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenMethodInjectionsUsesADeprecatedType() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedMethodInjectionTypeSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenMethodInjectionsUsesADeprecatedTypeWithReflection() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedPrivateMethodInjectionTypeSample.class); + compileAndCheckWarnings(registeredBean); + } + + @Test + void contributeWhenMethodInjectionsIsDeprecated() { + RegisteredBean registeredBean = getAndApplyContribution( + DeprecatedMethodInjectionPointSample.class); + compileAndCheckWarnings(registeredBean); + } + + + private void compileAndCheckWarnings(RegisteredBean registeredBean) { + assertThatNoException().isThrownBy(() -> compile(TEST_COMPILER, registeredBean, + ((instanceSupplier, compiled) -> {}))); + } + + } + private RegisteredBean getAndApplyContribution(Class beanClass) { RegisteredBean registeredBean = registerBean(beanClass); BeanRegistrationAotContribution contribution = this.beanPostProcessor.processAheadOfTime(registeredBean); @@ -218,11 +291,17 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { return compiled.getSourceFileFromPackage(sample.getPackageName()); } - @SuppressWarnings("unchecked") private void compile(RegisteredBean registeredBean, BiConsumer, Compiled> result) { + compile(TestCompiler.forSystem(), registeredBean, result); + } + + @SuppressWarnings("unchecked") + private void compile(TestCompiler testCompiler, RegisteredBean registeredBean, + BiConsumer, Compiled> result) { Class target = registeredBean.getBeanClass(); MethodReference methodReference = this.beanRegistrationCode.getInstancePostProcessors().get(0); + CodeWarnings codeWarnings = new CodeWarnings(); this.beanRegistrationCode.getTypeBuilder().set(type -> { CodeBlock methodInvocation = methodReference.toInvokeCodeBlock( ArgumentCodeGenerator.of(RegisteredBean.class, "registeredBean").and(target, "instance"), @@ -235,10 +314,11 @@ class AutowiredAnnotationBeanRegistrationAotContributionTests { .addParameter(target, "instance").returns(target) .addStatement("return $L", methodInvocation) .build()); - + codeWarnings.detectDeprecation(target); + codeWarnings.suppress(type); }); this.generationContext.writeGeneratedContent(); - TestCompiler.forSystem().with(this.generationContext).compile(compiled -> + testCompiler.with(this.generationContext).printFiles(System.out).compile(compiled -> result.accept(compiled.getInstance(BiFunction.class), compiled)); } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/CodeWarningsTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/CodeWarningsTests.java index 7b5db5e344..3767c91581 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/CodeWarningsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/CodeWarningsTests.java @@ -34,8 +34,10 @@ import org.springframework.beans.testfixture.beans.factory.generator.deprecation import org.springframework.core.ResolvableType; import org.springframework.core.test.tools.Compiled; import org.springframework.core.test.tools.TestCompiler; +import org.springframework.javapoet.FieldSpec; import org.springframework.javapoet.MethodSpec; import org.springframework.javapoet.MethodSpec.Builder; +import org.springframework.javapoet.TypeSpec; import static org.assertj.core.api.Assertions.assertThat; @@ -59,30 +61,49 @@ class CodeWarningsTests { } @Test - void registerNoWarningDoesNotIncludeAnnotation() { - compile(method -> { + void registerNoWarningDoesNotIncludeAnnotationOnMethod() { + compileWithMethod(method -> { this.codeWarnings.suppress(method); method.addStatement("$T bean = $S", String.class, "Hello"); }, compiled -> assertThat(compiled.getSourceFile()).doesNotContain("@SuppressWarnings")); } + @Test + void registerNoWarningDoesNotIncludeAnnotationOnType() { + compile(type -> { + this.codeWarnings.suppress(type); + type.addField(FieldSpec.builder(String.class, "type").build()); + }, compiled -> assertThat(compiled.getSourceFile()).doesNotContain("@SuppressWarnings")); + } + @Test @SuppressWarnings("deprecation") - void registerWarningSuppressesIt() { + void registerWarningSuppressesItOnMethod() { this.codeWarnings.register("deprecation"); - compile(method -> { + compileWithMethod(method -> { this.codeWarnings.suppress(method); method.addStatement("$T bean = new $T()", DeprecatedBean.class, DeprecatedBean.class); }, compiled -> assertThat(compiled.getSourceFile()) .contains("@SuppressWarnings(\"deprecation\")")); } + @Test + @SuppressWarnings("deprecation") + void registerWarningSuppressesItOnType() { + this.codeWarnings.register("deprecation"); + compile(type -> { + this.codeWarnings.suppress(type); + type.addField(FieldSpec.builder(DeprecatedBean.class, "bean").build()); + }, compiled -> assertThat(compiled.getSourceFile()) + .contains("@SuppressWarnings(\"deprecation\")")); + } + @Test @SuppressWarnings({ "deprecation", "removal" }) - void registerSeveralWarningsSuppressesThem() { + void registerSeveralWarningsSuppressesThemOnMethod() { this.codeWarnings.register("deprecation"); this.codeWarnings.register("removal"); - compile(method -> { + compileWithMethod(method -> { this.codeWarnings.suppress(method); method.addStatement("$T bean = new $T()", DeprecatedBean.class, DeprecatedBean.class); method.addStatement("$T another = new $T()", DeprecatedForRemovalBean.class, DeprecatedForRemovalBean.class); @@ -90,6 +111,19 @@ class CodeWarningsTests { .contains("@SuppressWarnings({ \"deprecation\", \"removal\" })")); } + @Test + @SuppressWarnings({ "deprecation", "removal" }) + void registerSeveralWarningsSuppressesThemOnType() { + this.codeWarnings.register("deprecation"); + this.codeWarnings.register("removal"); + compile(type -> { + this.codeWarnings.suppress(type); + type.addField(FieldSpec.builder(DeprecatedBean.class, "bean").build()); + type.addField(FieldSpec.builder(DeprecatedForRemovalBean.class, "another").build()); + }, compiled -> assertThat(compiled.getSourceFile()) + .contains("@SuppressWarnings({ \"deprecation\", \"removal\" })")); + } + @Test @SuppressWarnings("deprecation") void detectDeprecationOnAnnotatedElementWithDeprecated() { @@ -171,16 +205,20 @@ class CodeWarningsTests { assertThat(this.codeWarnings).hasToString("CodeWarnings[deprecation, rawtypes]"); } - private void compile(Consumer method, Consumer result) { - DeferredTypeBuilder typeBuilder = new DeferredTypeBuilder(); - this.generationContext.getGeneratedClasses().addForFeature("TestCode", typeBuilder); - typeBuilder.set(type -> { + private void compileWithMethod(Consumer method, Consumer result) { + compile(type -> { type.addModifiers(Modifier.PUBLIC); Builder methodBuilder = MethodSpec.methodBuilder("apply") .addModifiers(Modifier.PUBLIC); method.accept(methodBuilder); type.addMethod(methodBuilder.build()); - }); + }, result); + } + + private void compile(Consumer type, Consumer result) { + DeferredTypeBuilder typeBuilder = new DeferredTypeBuilder(); + this.generationContext.getGeneratedClasses().addForFeature("TestCode", typeBuilder); + typeBuilder.set(type); this.generationContext.writeGeneratedContent(); TEST_COMPILER.with(this.generationContext).compile(result); } diff --git a/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/factory/annotation/DeprecatedInjectionSamples.java b/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/factory/annotation/DeprecatedInjectionSamples.java new file mode 100644 index 0000000000..bb8faf0cbe --- /dev/null +++ b/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/factory/annotation/DeprecatedInjectionSamples.java @@ -0,0 +1,74 @@ +/* + * Copyright 2002-2024 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.beans.testfixture.beans.factory.annotation; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.env.Environment; + +public abstract class DeprecatedInjectionSamples { + + @Deprecated + public static class DeprecatedEnvironment {} + + @Deprecated + public static class DeprecatedSample { + + @Autowired + Environment environment; + + } + + public static class DeprecatedFieldInjectionPointSample { + + @Autowired + @Deprecated + Environment environment; + + } + + public static class DeprecatedFieldInjectionTypeSample { + + @Autowired + DeprecatedEnvironment environment; + } + + public static class DeprecatedPrivateFieldInjectionTypeSample { + + @Autowired + private DeprecatedEnvironment environment; + } + + public static class DeprecatedMethodInjectionPointSample { + + @Autowired + @Deprecated + void setEnvironment(Environment environment) {} + } + + public static class DeprecatedMethodInjectionTypeSample { + + @Autowired + void setEnvironment(DeprecatedEnvironment environment) {} + } + + public static class DeprecatedPrivateMethodInjectionTypeSample { + + @Autowired + private void setEnvironment(DeprecatedEnvironment environment) {} + } + +}