diff --git a/CHANGELOG.md b/CHANGELOG.md index fdcbf3346..13d386505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ## [Unreleased] ### Spring Core -- feat: Report two `@Bean` methods with different names that declare the same bean name in one configuration class, which fails Spring Boot startup unless bean definition overriding is allowed; methods annotated with `@Profile` or another `@Conditional` are not reported, since Spring skips them before it registers the name (#597) +- feat: Report two `@Bean` methods with different names that declare the same bean name in one configuration class, which fails Spring Boot startup unless bean definition overriding is allowed; Spring Framework 7 fails for a `@Configuration` class with `enforceUniqueMethods` even when overriding is allowed, so the problem stays an error there whatever `spring.main.allow-bean-definition-overriding` says, and is a warning for an allowed overriding when the Spring Framework version cannot be determined; methods annotated with `@Profile` or another `@Conditional` are not reported, since Spring skips them before it registers the name (#597, #686) - feat: Report a stereotype annotation such as `@Component` on an abstract class without a `@Lookup` method, which Spring does not register, with a quick fix that removes it (#601) - fix: Bean usages and the abstract-component gutter now list consumers declared in modules that depend on the bean's module, and count as injection points only the constructors Spring injects through: the `@Autowired` ones, otherwise the only constructor when it takes parameters, or a Kotlin primary constructor next to a no-arg secondary one, so the other constructors of a class with several and no `@Autowired` are no longer listed (#600) - fix: Type a bean declared by a superclass `@Bean` method and overridden with a narrower return type by the override's type, as Spring predicts it: the bean keeps the superclass declaration's name and annotations, and now autowires by the override's return type, in the bean model, the autowiring inspection and the Spring MCP bean tools (#594) diff --git a/modules/spring-core/src/main/kotlin/com/explyt/spring/core/inspections/SpringDuplicateBeanNameInspection.kt b/modules/spring-core/src/main/kotlin/com/explyt/spring/core/inspections/SpringDuplicateBeanNameInspection.kt index 1d908d15d..d16121d2d 100644 --- a/modules/spring-core/src/main/kotlin/com/explyt/spring/core/inspections/SpringDuplicateBeanNameInspection.kt +++ b/modules/spring-core/src/main/kotlin/com/explyt/spring/core/inspections/SpringDuplicateBeanNameInspection.kt @@ -8,14 +8,17 @@ package com.explyt.spring.core.inspections import com.explyt.inspection.SpringBaseUastLocalInspectionTool import com.explyt.spring.core.SpringCoreBundle import com.explyt.spring.core.SpringCoreClasses +import com.explyt.spring.core.service.SpringSearchService import com.explyt.spring.core.service.conditional.ConditionPropertyValue import com.explyt.spring.core.service.conditional.PropertyConditionValues import com.explyt.spring.core.util.BeanFactoryMethods +import com.explyt.spring.core.util.SpringBootUtil import com.explyt.spring.core.util.SpringCoreUtil.resolveBeanName import com.explyt.util.ExplytPsiUtil.getMetaAnnotation import com.intellij.codeInspection.InspectionManager import com.intellij.codeInspection.ProblemDescriptor import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.openapi.module.Module import com.intellij.openapi.module.ModuleUtilCore import com.intellij.openapi.progress.ProgressManager import com.intellij.psi.PsiClass @@ -33,7 +36,7 @@ class SpringDuplicateBeanNameInspection : SpringBaseUastLocalInspectionTool() { ): Array? { val psiClass = aClass.javaPsi val collisions = collisionsIn(psiClass).ifEmpty { return null } - val overriding = overridingOf(psiClass).takeUnless { it == Overriding.ALLOWED } ?: return null + val overriding = overridingOf(aClass).takeUnless { it == Overriding.ALLOWED } ?: return null return collisions.mapNotNull { it.toProblem(manager, isOnTheFly, overriding) }.toTypedArray() } @@ -52,17 +55,35 @@ class SpringDuplicateBeanNameInspection : SpringBaseUastLocalInspectionTool() { private fun PsiMethod.isConditional(): Boolean = getMetaAnnotation(SpringCoreClasses.CONDITIONAL) != null - private fun overridingOf(psiClass: PsiClass): Overriding { - val module = ModuleUtilCore.findModuleForPsiElement(psiClass) ?: return Overriding.DISALLOWED + private fun overridingOf(aClass: UClass): Overriding { + val module = ModuleUtilCore.findModuleForPsiElement(aClass.javaPsi) ?: return Overriding.DISALLOWED + val ignoredOverriding = if (aClass.enforcesUniqueMethods(module)) ignoredOverridingOf(module) else null + if (ignoredOverriding == Overriding.IGNORED_ON_SPRING_7) return ignoredOverriding return when (val value = PropertyConditionValues(module).valueOf(ALLOW_OVERRIDING_KEY)) { is ConditionPropertyValue.Known -> - if (value.text.trim().equals("true", ignoreCase = true)) Overriding.ALLOWED else Overriding.DISALLOWED + if (value.text.trim().equals("true", ignoreCase = true)) ignoredOverriding ?: Overriding.ALLOWED + else Overriding.DISALLOWED ConditionPropertyValue.Missing -> Overriding.DISALLOWED - ConditionPropertyValue.Unresolvable -> Overriding.UNKNOWN + ConditionPropertyValue.Unresolvable -> Overriding.UNRESOLVED } } + private fun ignoredOverridingOf(module: Module): Overriding? { + val springMajor = SpringBootUtil.getSpringCoreMajorVersion(module) ?: return Overriding.IGNORED_ON_UNKNOWN_SPRING + return Overriding.IGNORED_ON_SPRING_7.takeIf { springMajor >= SPRING_7 } + } + + private fun UClass.enforcesUniqueMethods(module: Module): Boolean { + val configurations = SpringSearchService.getInstance(module.project) + .getMetaAnnotations(module, SpringCoreClasses.CONFIGURATION) + val configuration = uAnnotations.firstOrNull { it.qualifiedName == SpringCoreClasses.CONFIGURATION } + ?: uAnnotations.firstOrNull { configurations.contains(it) } + ?: return false + val values = configurations.getAnnotationMemberValues(configuration, setOf(ENFORCE_UNIQUE_METHODS)) + return values.firstOrNull()?.evaluate() != false + } + private fun Collision.toProblem( manager: InspectionManager, isOnTheFly: Boolean, @@ -70,7 +91,9 @@ class SpringDuplicateBeanNameInspection : SpringBaseUastLocalInspectionTool() { ): ProblemDescriptor? { val anchor = second.toUElementOfType()?.uastAnchor?.sourcePsi ?: return null val (messageKey, highlightType) = when (overriding) { - Overriding.UNKNOWN -> UNRESOLVED_OVERRIDING_MESSAGE to ProblemHighlightType.WARNING + Overriding.UNRESOLVED -> UNRESOLVED_OVERRIDING_MESSAGE to ProblemHighlightType.WARNING + Overriding.IGNORED_ON_UNKNOWN_SPRING -> UNKNOWN_SPRING_OVERRIDING_MESSAGE to ProblemHighlightType.WARNING + Overriding.IGNORED_ON_SPRING_7 -> SPRING_7_OVERRIDING_MESSAGE to ProblemHighlightType.GENERIC_ERROR_OR_WARNING else -> MESSAGE to ProblemHighlightType.GENERIC_ERROR_OR_WARNING } return manager.createProblemDescriptor( @@ -84,11 +107,16 @@ class SpringDuplicateBeanNameInspection : SpringBaseUastLocalInspectionTool() { private data class Collision(val beanName: String, val first: PsiMethod, val second: PsiMethod) - private enum class Overriding { ALLOWED, DISALLOWED, UNKNOWN } + private enum class Overriding { ALLOWED, DISALLOWED, UNRESOLVED, IGNORED_ON_SPRING_7, IGNORED_ON_UNKNOWN_SPRING } private companion object { const val ALLOW_OVERRIDING_KEY = "spring.main.allow-bean-definition-overriding" + const val ENFORCE_UNIQUE_METHODS = "enforceUniqueMethods" + const val SPRING_7 = 7 const val MESSAGE = "explyt.spring.inspection.bean.name.duplicate" const val UNRESOLVED_OVERRIDING_MESSAGE = "explyt.spring.inspection.bean.name.duplicate.overriding.unresolved" + const val SPRING_7_OVERRIDING_MESSAGE = "explyt.spring.inspection.bean.name.duplicate.overriding.spring7" + const val UNKNOWN_SPRING_OVERRIDING_MESSAGE = + "explyt.spring.inspection.bean.name.duplicate.overriding.unknown.spring" } } diff --git a/modules/spring-core/src/main/resources/inspectionDescriptions/SpringDuplicateBeanName.html b/modules/spring-core/src/main/resources/inspectionDescriptions/SpringDuplicateBeanName.html index f69e3e1d5..9ff728616 100644 --- a/modules/spring-core/src/main/resources/inspectionDescriptions/SpringDuplicateBeanName.html +++ b/modules/spring-core/src/main/resources/inspectionDescriptions/SpringDuplicateBeanName.html @@ -4,12 +4,18 @@ of the same configuration class.

Spring Boot disallows bean definition overriding by default, so such a configuration fails at startup with BeanDefinitionOverrideException. Overloaded @Bean methods with the same method name are merged by -Spring and are not reported. Nothing is reported when spring.main.allow-bean-definition-overriding=true -is set in the module configuration.

+Spring and are not reported.

+

When spring.main.allow-bean-definition-overriding=true is set in the module configuration, nothing is +reported before Spring Framework 7, for a lite class such as a @Component with @Bean methods, or for +a @Configuration(enforceUniqueMethods = false) class, also when enforceUniqueMethods is set through +a composed annotation. Spring Framework 7 fails at startup for any other @Configuration class whatever the +property says, so the problem is always an error there, and it is reported as a warning when the property allows +overriding but the Spring Framework version cannot be determined.

Methods annotated with @Profile or another @Conditional annotation, such as @ConditionalOnProperty, are not reported: Spring evaluates their conditions before it registers the bean name, so only one of them may be active.

When the value of spring.main.allow-bean-definition-overriding cannot be resolved, for example because it is -a placeholder or depends on a profile, the problem is reported as a warning instead of an error.

+a placeholder or depends on a profile, the problem is reported as a warning instead of an error, except for a +@Configuration class on Spring Framework 7, which fails whatever the value is.

diff --git a/modules/spring-core/src/main/resources/messages/SpringCoreBundle.properties b/modules/spring-core/src/main/resources/messages/SpringCoreBundle.properties index b9085d9f1..a845fac81 100644 --- a/modules/spring-core/src/main/resources/messages/SpringCoreBundle.properties +++ b/modules/spring-core/src/main/resources/messages/SpringCoreBundle.properties @@ -119,6 +119,8 @@ explyt.spring.inspection.configuration.proxy.quickfix=Use dependency injection explyt.spring.inspection.bean.name.duplicate.display=Duplicate @Bean name in one configuration class explyt.spring.inspection.bean.name.duplicate=Bean ''{0}'' is also declared by ''{1}'' in this class; Spring Boot fails at startup unless bean definition overriding is allowed explyt.spring.inspection.bean.name.duplicate.overriding.unresolved=Bean ''{0}'' is also declared by ''{1}'' in this class; Spring Boot fails at startup unless spring.main.allow-bean-definition-overriding is true, and its value cannot be resolved here +explyt.spring.inspection.bean.name.duplicate.overriding.spring7=Bean ''{0}'' is also declared by ''{1}'' in this class; Spring 7 fails at startup for a @Configuration class even when bean definition overriding is allowed, unless enforceUniqueMethods is false +explyt.spring.inspection.bean.name.duplicate.overriding.unknown.spring=Bean ''{0}'' is also declared by ''{1}'' in this class; spring.main.allow-bean-definition-overriding=true does not help a @Configuration class on Spring 7, and the Spring version cannot be determined here explyt.spring.inspection.bean.lookup=@Lookup explyt.spring.inspection.bean.lookup.unknown=Cannot resolve bean ''{0}'' explyt.spring.inspection.bean.lookup.wrongType=Expected bean of ''{0}'' type but found ''{1}'' diff --git a/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/java/SpringDuplicateBeanNameInspectionTest.kt b/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/java/SpringDuplicateBeanNameInspectionTest.kt index 298bf9e49..a42fcbfcc 100644 --- a/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/java/SpringDuplicateBeanNameInspectionTest.kt +++ b/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/java/SpringDuplicateBeanNameInspectionTest.kt @@ -7,21 +7,169 @@ package com.explyt.spring.core.inspections.java import com.explyt.spring.core.SpringCoreBundle import com.explyt.spring.core.SpringCoreClasses +import com.explyt.spring.core.inspections.BeanNameConventionConflictFixture import com.explyt.spring.core.inspections.SpringDuplicateBeanNameInspection import com.explyt.spring.core.tracker.ModificationTrackerManager +import com.explyt.spring.core.util.SpringBootUtil import com.explyt.spring.test.ExplytBaseLightTestCase import com.explyt.spring.test.TestLibrary import com.intellij.codeInsight.daemon.impl.HighlightInfo import com.intellij.lang.annotation.HighlightSeverity -class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { - override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_3_1_1) +private const val FULL_CONFIGURATION = """ + @Configuration + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +private const val LITE_CONFIGURATION = """ + @Component + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +private const val CONFIGURATION_WITHOUT_UNIQUE_METHODS = """ + @Configuration(enforceUniqueMethods = false) + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +private const val COMPOSED_CONFIGURATION = """ + @Configuration + @Retention(RetentionPolicy.RUNTIME) + @interface MyConfig {} + + @MyConfig + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +private const val COMPOSED_CONFIGURATION_WITHOUT_UNIQUE_METHODS = """ + @Configuration(enforceUniqueMethods = false) + @Retention(RetentionPolicy.RUNTIME) + @interface MyConfig {} + + @MyConfig + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +private fun aliasedComposedConfiguration(useSite: String) = """ + @Configuration + @Retention(RetentionPolicy.RUNTIME) + @interface MyConfig { + @AliasFor(annotation = Configuration.class, attribute = "enforceUniqueMethods") boolean unique() default false; + } + + $useSite + public class Config { + @Bean("svc") Object a() { return new Object(); } + @Bean("svc") Object b() { return new Object(); } + } +""" + +abstract class SpringDuplicateBeanNameJavaTestCase : ExplytBaseLightTestCase() { + + protected abstract val springMajor: Int? override fun setUp() { super.setUp() myFixture.enableInspections(SpringDuplicateBeanNameInspection::class.java) } + protected fun allowBeanDefinitionOverriding(value: String) { + myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=$value") + ModificationTrackerManager.getInstance(project).invalidateAll() + } + + protected fun highlights(body: String): List { + myFixture.configureByText( + "Config.java", + "package demo;\n\nimport java.lang.annotation.Retention;\n" + + "import java.lang.annotation.RetentionPolicy;\n" + + "import org.springframework.core.annotation.AliasFor;\n" + + "import org.springframework.context.annotation.Bean;\n" + + "import org.springframework.context.annotation.Configuration;\n" + + "import org.springframework.context.annotation.Profile;\n" + + "import org.springframework.stereotype.Component;\n" + + "import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;\n\n" + body.trimIndent() + ) + assertEquals( + "Precondition: spring-core major version of the module", + springMajor, + SpringBootUtil.getSpringCoreMajorVersion(module) + ) + assertTrue( + "Precondition: the inspection is available for the file", + SpringDuplicateBeanNameInspection().isAvailableForFile(myFixture.file) + ) + assertBeanAnnotationResolves() + return myFixture.doHighlighting().filter { it.inspectionToolId == INSPECTION_ID } + } + + protected fun assertConditionalAnnotationResolves(annotation: String) { + assertNotNull("Precondition: $annotation must come from the fixture library", myFixture.findClass(annotation)) + assertTrue( + "Precondition: demo.Config must use resolved $annotation", + myFixture.findClass("demo.Config").methods.any { it.hasAnnotation(annotation) } + ) + } + + private fun assertBeanAnnotationResolves() { + assertNotNull("Precondition: @Bean must come from the fixture library", myFixture.findClass(SpringCoreClasses.BEAN)) + val beanMethods = myFixture.findClass("demo.Config").methods + assertTrue( + "Precondition: every method of demo.Config must carry a resolved ${SpringCoreClasses.BEAN}", + beanMethods.isNotEmpty() && beanMethods.all { it.hasAnnotation(SpringCoreClasses.BEAN) } + ) + } + + protected fun assertProblem(problem: HighlightInfo, anchor: String, beanName: String, firstMethod: String) { + assertEquals(HighlightSeverity.ERROR, problem.severity) + assertEquals(anchor, problem.text) + assertEquals( + "Bean '$beanName' is also declared by '$firstMethod' in this class; " + + "Spring Boot fails at startup unless bean definition overriding is allowed", + problem.description + ) + } + + protected fun assertSingleProblem( + problems: List, + severity: HighlightSeverity, + anchor: String, + beanName: String, + firstMethod: String + ) { + assertEquals("one problem: $problems", 1, problems.size) + val problem = problems.single() + assertEquals(severity, problem.severity) + assertEquals(anchor, problem.text) + assertTrue( + "the message names bean '$beanName' and method '$firstMethod': ${problem.description}", + problem.description.contains("'$beanName'") && problem.description.contains("'$firstMethod'") + ) + } + + private companion object { + const val INSPECTION_ID = "SpringDuplicateBeanName" + } +} + +class SpringDuplicateBeanNameInspectionTest : SpringDuplicateBeanNameJavaTestCase() { + override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_3_1_1) + override val springMajor: Int = 6 + fun testExplicitNamesOnDifferentMethodsCollide() { val problem = highlights( """ @@ -186,35 +334,15 @@ class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { } fun testAllowedOverridingSuppressesTheProblem() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=true") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("true") - assertEmpty( - highlights( - """ - @Configuration - public class Config { - @Bean("svc") Object a() { return new Object(); } - @Bean("svc") Object b() { return new Object(); } - } - """ - ) - ) + assertEmpty(highlights(FULL_CONFIGURATION)) } fun testUnresolvableOverridingPropertyUsesWarning() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=\${OVERRIDING}") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("\${OVERRIDING}") - val problem = highlights( - """ - @Configuration - public class Config { - @Bean("svc") Object a() { return new Object(); } - @Bean("svc") Object b() { return new Object(); } - } - """ - ).single() + val problem = highlights(FULL_CONFIGURATION).single() assertEquals(HighlightSeverity.WARNING, problem.severity) assertEquals( @@ -228,62 +356,115 @@ class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { } fun testDisallowedOverridingKeepsTheProblem() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=false") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("false") - val problem = highlights( - """ - @Configuration - public class Config { - @Bean("svc") Object a() { return new Object(); } - @Bean("svc") Object b() { return new Object(); } - } - """ - ).single() + val problem = highlights(FULL_CONFIGURATION).single() assertProblem(problem, anchor = "b", beanName = "svc", firstMethod = "a") } +} - private fun highlights(body: String): List { - myFixture.configureByText( - "Config.java", -"package demo;\n\nimport org.springframework.context.annotation.Bean;\n" + - "import org.springframework.context.annotation.Configuration;\n" + - "import org.springframework.context.annotation.Profile;\n" + - "import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;\n\n" + body.trimIndent() - ) - assertBeanAnnotationResolves() - return myFixture.doHighlighting().filter { it.inspectionToolId == INSPECTION_ID } +class SpringDuplicateBeanNameSpring7InspectionTest : SpringDuplicateBeanNameJavaTestCase() { + override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_4_1_0) + override val springMajor: Int = 7 + + fun testMissingOverridingPropertyKeepsTheProblemInFullConfiguration() { + assertSpring7Problem(highlights(FULL_CONFIGURATION)) } - private fun assertConditionalAnnotationResolves(annotation: String) { - assertNotNull("Precondition: $annotation must come from the fixture library", myFixture.findClass(annotation)) - assertTrue( - "Precondition: demo.Config must use resolved $annotation", - myFixture.findClass("demo.Config").methods.any { it.hasAnnotation(annotation) } - ) + fun testDisallowedOverridingKeepsTheProblemInFullConfiguration() { + allowBeanDefinitionOverriding("false") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) } - private fun assertBeanAnnotationResolves() { - assertNotNull("Precondition: @Bean must come from the fixture library", myFixture.findClass(SpringCoreClasses.BEAN)) - val beanMethods = myFixture.findClass("demo.Config").methods - assertTrue( - "Precondition: every method of demo.Config must carry a resolved ${SpringCoreClasses.BEAN}", - beanMethods.isNotEmpty() && beanMethods.all { it.hasAnnotation(SpringCoreClasses.BEAN) } + fun testAllowedOverridingKeepsTheProblemInFullConfiguration() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) + } + + fun testUnresolvableOverridingPropertyKeepsTheErrorInFullConfiguration() { + allowBeanDefinitionOverriding("\${OVERRIDING}") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) + } + + fun testComposedConfigurationWithoutUniqueMethodEnforcementSuppressesTheProblem() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(COMPOSED_CONFIGURATION)) + assertEmpty(highlights(COMPOSED_CONFIGURATION_WITHOUT_UNIQUE_METHODS)) + } + + fun testComposedConfigurationAliasDefaultWithoutUniqueMethodEnforcementSuppressesTheProblem() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(aliasedComposedConfiguration("@MyConfig(unique = true)"))) + assertEmpty(highlights(aliasedComposedConfiguration("@MyConfig"))) + } + + fun testDisallowedOverridingKeepsTheProblemInLiteClass() { + allowBeanDefinitionOverriding("false") + + assertProblem(highlights(LITE_CONFIGURATION).single(), anchor = "b", beanName = "svc", firstMethod = "a") + } + + fun testAllowedOverridingSuppressesTheProblemInLiteClass() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(LITE_CONFIGURATION)) + } + + fun testDisallowedOverridingKeepsTheProblemWithoutUniqueMethodEnforcement() { + allowBeanDefinitionOverriding("false") + + assertProblem( + highlights(CONFIGURATION_WITHOUT_UNIQUE_METHODS).single(), anchor = "b", beanName = "svc", firstMethod = "a" ) } - private fun assertProblem(problem: HighlightInfo, anchor: String, beanName: String, firstMethod: String) { - assertEquals(HighlightSeverity.ERROR, problem.severity) - assertEquals(anchor, problem.text) + fun testAllowedOverridingSuppressesTheProblemWithoutUniqueMethodEnforcement() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(CONFIGURATION_WITHOUT_UNIQUE_METHODS)) + } + + private fun assertSpring7Problem(problems: List) { + assertSingleProblem(problems, HighlightSeverity.ERROR, anchor = "b", beanName = "svc", firstMethod = "a") assertEquals( - "Bean '$beanName' is also declared by '$firstMethod' in this class; " + - "Spring Boot fails at startup unless bean definition overriding is allowed", - problem.description + SpringCoreBundle.message("explyt.spring.inspection.bean.name.duplicate.overriding.spring7", "svc", "a"), + problems.single().description ) } +} - private companion object { - const val INSPECTION_ID = "SpringDuplicateBeanName" +class SpringDuplicateBeanNameUnknownSpringVersionInspectionTest : SpringDuplicateBeanNameJavaTestCase() { + override val libraries: Array = arrayOf(TestLibrary("org.springframework:spring-context:6.0.7", false)) + override val springMajor: Int? = null + + override fun setUp() { + super.setUp() + BeanNameConventionConflictFixture.addSpringCoreWithoutCoordinates(module, testRootDisposable) + } + + fun testDisallowedOverridingKeepsTheProblem() { + allowBeanDefinitionOverriding("false") + + assertProblem(highlights(FULL_CONFIGURATION).single(), anchor = "b", beanName = "svc", firstMethod = "a") + } + + fun testAllowedOverridingInFullConfigurationUsesWarning() { + allowBeanDefinitionOverriding("true") + + assertSingleProblem( + highlights(FULL_CONFIGURATION), HighlightSeverity.WARNING, anchor = "b", beanName = "svc", firstMethod = "a" + ) + } + + fun testAllowedOverridingSuppressesTheProblemInLiteClass() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(LITE_CONFIGURATION)) } } diff --git a/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/kotlin/SpringDuplicateBeanNameInspectionTest.kt b/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/kotlin/SpringDuplicateBeanNameInspectionTest.kt index d4cc56cf3..37a906ef3 100644 --- a/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/kotlin/SpringDuplicateBeanNameInspectionTest.kt +++ b/modules/spring-core/src/test/kotlin/com/explyt/spring/core/inspections/kotlin/SpringDuplicateBeanNameInspectionTest.kt @@ -7,21 +7,164 @@ package com.explyt.spring.core.inspections.kotlin import com.explyt.spring.core.SpringCoreBundle import com.explyt.spring.core.SpringCoreClasses +import com.explyt.spring.core.inspections.BeanNameConventionConflictFixture import com.explyt.spring.core.inspections.SpringDuplicateBeanNameInspection import com.explyt.spring.core.tracker.ModificationTrackerManager +import com.explyt.spring.core.util.SpringBootUtil import com.explyt.spring.test.ExplytBaseLightTestCase import com.explyt.spring.test.TestLibrary import com.intellij.codeInsight.daemon.impl.HighlightInfo import com.intellij.lang.annotation.HighlightSeverity -class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { - override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_3_1_1) +private const val FULL_CONFIGURATION = """ + @Configuration + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +private const val LITE_CONFIGURATION = """ + @Component + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +private const val CONFIGURATION_WITHOUT_UNIQUE_METHODS = """ + @Configuration(enforceUniqueMethods = false) + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +private const val COMPOSED_CONFIGURATION = """ + @Configuration + annotation class MyConfig + + @MyConfig + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +private const val COMPOSED_CONFIGURATION_WITHOUT_UNIQUE_METHODS = """ + @Configuration(enforceUniqueMethods = false) + annotation class MyConfig + + @MyConfig + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +private fun aliasedComposedConfiguration(useSite: String) = """ + @Configuration + annotation class MyConfig( + @get:AliasFor(annotation = Configuration::class, attribute = "enforceUniqueMethods") val unique: Boolean = false, + ) + + $useSite + open class Config { + @Bean("svc") fun a(): Any = Any() + @Bean("svc") fun b(): Any = Any() + } +""" + +abstract class SpringDuplicateBeanNameKotlinTestCase : ExplytBaseLightTestCase() { + + protected abstract val springMajor: Int? override fun setUp() { super.setUp() myFixture.enableInspections(SpringDuplicateBeanNameInspection::class.java) } + protected fun allowBeanDefinitionOverriding(value: String) { + myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=$value") + ModificationTrackerManager.getInstance(project).invalidateAll() + } + + protected fun highlights(body: String): List { + myFixture.configureByText( + "Config.kt", + "package demo\n\nimport org.springframework.core.annotation.AliasFor\n" + + "import org.springframework.context.annotation.Bean\n" + + "import org.springframework.context.annotation.Configuration\n" + + "import org.springframework.context.annotation.Profile\n" + + "import org.springframework.stereotype.Component\n" + + "import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty\n\n" + body.trimIndent() + ) + assertEquals( + "Precondition: spring-core major version of the module", + springMajor, + SpringBootUtil.getSpringCoreMajorVersion(module) + ) + assertTrue( + "Precondition: the inspection is available for the file", + SpringDuplicateBeanNameInspection().isAvailableForFile(myFixture.file) + ) + assertBeanAnnotationResolves() + return myFixture.doHighlighting().filter { it.inspectionToolId == INSPECTION_ID } + } + + protected fun assertConditionalAnnotationResolves(annotation: String) { + assertNotNull("Precondition: $annotation must come from the fixture library", myFixture.findClass(annotation)) + assertTrue( + "Precondition: demo.Config must use resolved $annotation", + myFixture.findClass("demo.Config").methods.any { it.hasAnnotation(annotation) } + ) + } + + private fun assertBeanAnnotationResolves() { + assertNotNull("Precondition: @Bean must come from the fixture library", myFixture.findClass(SpringCoreClasses.BEAN)) + val beanMethods = myFixture.findClass("demo.Config").methods.filterNot { it.isConstructor } + assertTrue( + "Precondition: every function of demo.Config must carry a resolved ${SpringCoreClasses.BEAN}", + beanMethods.isNotEmpty() && beanMethods.all { it.hasAnnotation(SpringCoreClasses.BEAN) } + ) + } + + protected fun assertProblem(problem: HighlightInfo, anchor: String, beanName: String, firstMethod: String) { + assertEquals(HighlightSeverity.ERROR, problem.severity) + assertEquals(anchor, problem.text) + assertEquals( + "Bean '$beanName' is also declared by '$firstMethod' in this class; " + + "Spring Boot fails at startup unless bean definition overriding is allowed", + problem.description + ) + } + + protected fun assertSingleProblem( + problems: List, + severity: HighlightSeverity, + anchor: String, + beanName: String, + firstMethod: String + ) { + assertEquals("one problem: $problems", 1, problems.size) + val problem = problems.single() + assertEquals(severity, problem.severity) + assertEquals(anchor, problem.text) + assertTrue( + "the message names bean '$beanName' and method '$firstMethod': ${problem.description}", + problem.description.contains("'$beanName'") && problem.description.contains("'$firstMethod'") + ) + } + + private companion object { + const val INSPECTION_ID = "SpringDuplicateBeanName" + } +} + +class SpringDuplicateBeanNameInspectionTest : SpringDuplicateBeanNameKotlinTestCase() { + override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_3_1_1) + override val springMajor: Int = 6 + fun testExplicitNamesOnDifferentMethodsCollide() { val problem = highlights( """ @@ -186,35 +329,15 @@ class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { } fun testAllowedOverridingSuppressesTheProblem() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=true") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("true") - assertEmpty( - highlights( - """ - @Configuration - open class Config { - @Bean("svc") fun a(): Any = Any() - @Bean("svc") fun b(): Any = Any() - } - """ - ) - ) + assertEmpty(highlights(FULL_CONFIGURATION)) } fun testUnresolvableOverridingPropertyUsesWarning() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=${'$'}{OVERRIDING}") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("${'$'}{OVERRIDING}") - val problem = highlights( - """ - @Configuration - open class Config { - @Bean("svc") fun a(): Any = Any() - @Bean("svc") fun b(): Any = Any() - } - """ - ).single() + val problem = highlights(FULL_CONFIGURATION).single() assertEquals(HighlightSeverity.WARNING, problem.severity) assertEquals( @@ -228,62 +351,115 @@ class SpringDuplicateBeanNameInspectionTest : ExplytBaseLightTestCase() { } fun testDisallowedOverridingKeepsTheProblem() { - myFixture.addFileToProject("application.properties", "spring.main.allow-bean-definition-overriding=false") - ModificationTrackerManager.getInstance(project).invalidateAll() + allowBeanDefinitionOverriding("false") - val problem = highlights( - """ - @Configuration - open class Config { - @Bean("svc") fun a(): Any = Any() - @Bean("svc") fun b(): Any = Any() - } - """ - ).single() + val problem = highlights(FULL_CONFIGURATION).single() assertProblem(problem, anchor = "b", beanName = "svc", firstMethod = "a") } +} - private fun highlights(body: String): List { - myFixture.configureByText( - "Config.kt", -"package demo\n\nimport org.springframework.context.annotation.Bean\n" + - "import org.springframework.context.annotation.Configuration\n" + - "import org.springframework.context.annotation.Profile\n" + - "import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty\n\n" + body.trimIndent() - ) - assertBeanAnnotationResolves() - return myFixture.doHighlighting().filter { it.inspectionToolId == INSPECTION_ID } +class SpringDuplicateBeanNameSpring7InspectionTest : SpringDuplicateBeanNameKotlinTestCase() { + override val libraries: Array = arrayOf(TestLibrary.springBootAutoConfigure_4_1_0) + override val springMajor: Int = 7 + + fun testMissingOverridingPropertyKeepsTheProblemInFullConfiguration() { + assertSpring7Problem(highlights(FULL_CONFIGURATION)) } - private fun assertConditionalAnnotationResolves(annotation: String) { - assertNotNull("Precondition: $annotation must come from the fixture library", myFixture.findClass(annotation)) - assertTrue( - "Precondition: demo.Config must use resolved $annotation", - myFixture.findClass("demo.Config").methods.any { it.hasAnnotation(annotation) } - ) + fun testDisallowedOverridingKeepsTheProblemInFullConfiguration() { + allowBeanDefinitionOverriding("false") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) } - private fun assertBeanAnnotationResolves() { - assertNotNull("Precondition: @Bean must come from the fixture library", myFixture.findClass(SpringCoreClasses.BEAN)) - val beanMethods = myFixture.findClass("demo.Config").methods.filterNot { it.isConstructor } - assertTrue( - "Precondition: every function of demo.Config must carry a resolved ${SpringCoreClasses.BEAN}", - beanMethods.isNotEmpty() && beanMethods.all { it.hasAnnotation(SpringCoreClasses.BEAN) } + fun testAllowedOverridingKeepsTheProblemInFullConfiguration() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) + } + + fun testUnresolvableOverridingPropertyKeepsTheErrorInFullConfiguration() { + allowBeanDefinitionOverriding("${'$'}{OVERRIDING}") + + assertSpring7Problem(highlights(FULL_CONFIGURATION)) + } + + fun testComposedConfigurationWithoutUniqueMethodEnforcementSuppressesTheProblem() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(COMPOSED_CONFIGURATION)) + assertEmpty(highlights(COMPOSED_CONFIGURATION_WITHOUT_UNIQUE_METHODS)) + } + + fun testComposedConfigurationAliasDefaultWithoutUniqueMethodEnforcementSuppressesTheProblem() { + allowBeanDefinitionOverriding("true") + + assertSpring7Problem(highlights(aliasedComposedConfiguration("@MyConfig(unique = true)"))) + assertEmpty(highlights(aliasedComposedConfiguration("@MyConfig"))) + } + + fun testDisallowedOverridingKeepsTheProblemInLiteClass() { + allowBeanDefinitionOverriding("false") + + assertProblem(highlights(LITE_CONFIGURATION).single(), anchor = "b", beanName = "svc", firstMethod = "a") + } + + fun testAllowedOverridingSuppressesTheProblemInLiteClass() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(LITE_CONFIGURATION)) + } + + fun testDisallowedOverridingKeepsTheProblemWithoutUniqueMethodEnforcement() { + allowBeanDefinitionOverriding("false") + + assertProblem( + highlights(CONFIGURATION_WITHOUT_UNIQUE_METHODS).single(), anchor = "b", beanName = "svc", firstMethod = "a" ) } - private fun assertProblem(problem: HighlightInfo, anchor: String, beanName: String, firstMethod: String) { - assertEquals(HighlightSeverity.ERROR, problem.severity) - assertEquals(anchor, problem.text) + fun testAllowedOverridingSuppressesTheProblemWithoutUniqueMethodEnforcement() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(CONFIGURATION_WITHOUT_UNIQUE_METHODS)) + } + + private fun assertSpring7Problem(problems: List) { + assertSingleProblem(problems, HighlightSeverity.ERROR, anchor = "b", beanName = "svc", firstMethod = "a") assertEquals( - "Bean '$beanName' is also declared by '$firstMethod' in this class; " + - "Spring Boot fails at startup unless bean definition overriding is allowed", - problem.description + SpringCoreBundle.message("explyt.spring.inspection.bean.name.duplicate.overriding.spring7", "svc", "a"), + problems.single().description ) } +} - private companion object { - const val INSPECTION_ID = "SpringDuplicateBeanName" +class SpringDuplicateBeanNameUnknownSpringVersionInspectionTest : SpringDuplicateBeanNameKotlinTestCase() { + override val libraries: Array = arrayOf(TestLibrary("org.springframework:spring-context:6.0.7", false)) + override val springMajor: Int? = null + + override fun setUp() { + super.setUp() + BeanNameConventionConflictFixture.addSpringCoreWithoutCoordinates(module, testRootDisposable) + } + + fun testDisallowedOverridingKeepsTheProblem() { + allowBeanDefinitionOverriding("false") + + assertProblem(highlights(FULL_CONFIGURATION).single(), anchor = "b", beanName = "svc", firstMethod = "a") + } + + fun testAllowedOverridingInFullConfigurationUsesWarning() { + allowBeanDefinitionOverriding("true") + + assertSingleProblem( + highlights(FULL_CONFIGURATION), HighlightSeverity.WARNING, anchor = "b", beanName = "svc", firstMethod = "a" + ) + } + + fun testAllowedOverridingSuppressesTheProblemInLiteClass() { + allowBeanDefinitionOverriding("true") + + assertEmpty(highlights(LITE_CONFIGURATION)) } }