From 096255882e98cf88a6af85a70bbe9dc150b6621b Mon Sep 17 00:00:00 2001 From: aboyko Date: Fri, 4 Apr 2025 17:47:25 -0400 Subject: [PATCH] Validation, quick fix for component annotations on bean registrar --- .../boot/java/Boot4JavaProblemType.java | 3 +- .../BeanRegistrarDeclarationReconciler.java | 114 ++++++++++------- .../src/main/resources/problem-types.json | 10 +- ...eanRegistrarDeclarationReconcilerTest.java | 121 ++++++++++++++++++ .../vscode-spring-boot/package.json | 14 +- 5 files changed, 215 insertions(+), 47 deletions(-) diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot4JavaProblemType.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot4JavaProblemType.java index 701f00ec1..489303ccd 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot4JavaProblemType.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot4JavaProblemType.java @@ -22,7 +22,8 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTy public enum Boot4JavaProblemType implements ProblemType { - REGISTRAR_BEAN_DECLARATION(WARNING, "Bean derived from BeanRegistrar should be registered via `@Import` over configuration bean", "Not registered via `@Import` in a configuration bean"); + REGISTRAR_BEAN_INVALID_ANNOTATION(WARNING, "Bean Registrar cannot be registered as a bean via `@Component` annotations", "Invalid annotation over bean registrar"), + REGISTRAR_BEAN_DECLARATION(WARNING, "Bean Registrar should be added to a configurarion bean via `@Import`", "Not added to configurartion via `@Import`"); private final ProblemSeverity defaultSeverity; private final String description; diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/BeanRegistrarDeclarationReconciler.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/BeanRegistrarDeclarationReconciler.java index 87702ae46..01487b806 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/BeanRegistrarDeclarationReconciler.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/BeanRegistrarDeclarationReconciler.java @@ -22,12 +22,15 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.Annotation; import org.eclipse.jdt.core.dom.CompilationUnit; import org.eclipse.jdt.core.dom.ITypeBinding; import org.eclipse.jdt.core.dom.TypeDeclaration; +import org.openrewrite.java.RemoveAnnotation; import org.springframework.ide.vscode.boot.index.SpringMetamodelIndex; import org.springframework.ide.vscode.boot.java.Annotations; import org.springframework.ide.vscode.boot.java.Boot4JavaProblemType; +import org.springframework.ide.vscode.boot.java.annotations.AnnotationHierarchies; import org.springframework.ide.vscode.boot.java.utils.ASTUtils; import org.springframework.ide.vscode.commons.Version; import org.springframework.ide.vscode.commons.java.IClasspathUtil; @@ -89,56 +92,81 @@ public class BeanRegistrarDeclarationReconciler implements JdtAstReconciler { if (!context.isIndexComplete()) { throw new RequiredCompleteIndexException(); } + + checkComponentAnnotations(context, project, docURI, cu, node, type); + checkRegistrationViaImport(context, project, docURI, node, type); - List configBeans = new ArrayList<>(); - Path p = Path.of(docURI); - List sourceFolders = IClasspathUtil.getSourceFolders(project.getClasspath()).map(f -> f.toPath()).filter(f -> p.startsWith(f)).collect(Collectors.toList()); - - for (Bean b : springIndex.getBeansOfProject(project.getElementName())) { - // if (b.getType().equals(type.getQualifiedName())) { - // return true; - // } - if (b.isConfiguration() && b.getLocation() != null) { - Path configBeanPath = Path.of(URI.create(b.getLocation().getUri())); - if (sourceFolders.stream().anyMatch(configBeanPath::startsWith)) { - configBeans.add(b); - } - } - } - - List importingBeanRegistrarConfigs = getImportedBeanRegistrarConfigs(configBeans, type); - if (configBeans.isEmpty() || importingBeanRegistrarConfigs.size() == 0) { - - ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), "No @Import found for bean registrar", node.getName().getStartPosition(), node.getName().getLength()); - List fixes = configBeans.stream() - .filter(b -> b.getLocation() != null && b.getLocation().getUri() != null) - .map(b -> new FixDescriptor(ImportBeanRegistrarInConfigRecipe.class.getName(), List.of(b.getLocation().getUri()), "Add %s to `@Import` in %s".formatted(type.getName(), b.getName())) - .withParameters(Map.of( - "configBeanFqn", b.getType(), - "beanRegFqn", type.getQualifiedName() - )) - .withRecipeScope(RecipeScope.FILE) - ).toList(); - ReconcileUtils.setRewriteFixes(registry, problem, fixes); - context.getProblemCollector().accept(problem); - - // record dependencies on types where we found import annotations for this bean registrar - // mark this file - - } - else { - // record dependencies on types where we found import annotations for this bean registrar - for (String typeOfConfigClassWithImport : importingBeanRegistrarConfigs) { - context.addDependency(typeOfConfigClassWithImport); - } - - } return true; } }; } + private void checkRegistrationViaImport(ReconcilingContext context, IJavaProject project, URI docURI, TypeDeclaration node, ITypeBinding type) { + List configBeans = new ArrayList<>(); + Path p = Path.of(docURI); + List sourceFolders = IClasspathUtil.getSourceFolders(project.getClasspath()).map(f -> f.toPath()).filter(f -> p.startsWith(f)).collect(Collectors.toList()); + + for (Bean b : springIndex.getBeansOfProject(project.getElementName())) { + if (b.isConfiguration() && b.getLocation() != null) { + Path configBeanPath = Path.of(URI.create(b.getLocation().getUri())); + if (sourceFolders.stream().anyMatch(configBeanPath::startsWith)) { + configBeans.add(b); + } + } + } + + List importingBeanRegistrarConfigs = getImportedBeanRegistrarConfigs(configBeans, type); + if (configBeans.isEmpty() || importingBeanRegistrarConfigs.size() == 0) { + + ReconcileProblemImpl problem = new ReconcileProblemImpl(Boot4JavaProblemType.REGISTRAR_BEAN_DECLARATION, "No @Import found for bean registrar", node.getName().getStartPosition(), node.getName().getLength()); + List fixes = configBeans.stream() + .filter(b -> b.getLocation() != null && b.getLocation().getUri() != null) + .map(b -> new FixDescriptor(ImportBeanRegistrarInConfigRecipe.class.getName(), List.of(b.getLocation().getUri()), "Add %s to `@Import` in %s".formatted(type.getName(), b.getName())) + .withParameters(Map.of( + "configBeanFqn", b.getType(), + "beanRegFqn", type.getQualifiedName() + )) + .withRecipeScope(RecipeScope.FILE) + ).toList(); + ReconcileUtils.setRewriteFixes(registry, problem, fixes); + context.getProblemCollector().accept(problem); + + // record dependencies on types where we found import annotations for this bean registrar + // mark this file + + } + else { + // record dependencies on types where we found import annotations for this bean registrar + for (String typeOfConfigClassWithImport : importingBeanRegistrarConfigs) { + context.addDependency(typeOfConfigClassWithImport); + } + + } + } + + private void checkComponentAnnotations(ReconcilingContext context, IJavaProject project, URI docURI, CompilationUnit cu, TypeDeclaration node, ITypeBinding type) { + AnnotationHierarchies annotationHierarchies = AnnotationHierarchies.get(node); + for (Object o : node.modifiers()) { + if (o instanceof Annotation a) { + ITypeBinding ab = a.resolveTypeBinding(); + if (ab != null && annotationHierarchies.isAnnotatedWith(ab, Annotations.COMPONENT)) { + ReconcileProblemImpl problem = new ReconcileProblemImpl( + Boot4JavaProblemType.REGISTRAR_BEAN_INVALID_ANNOTATION, + Boot4JavaProblemType.REGISTRAR_BEAN_INVALID_ANNOTATION.getLabel(), + a.getTypeName().getStartPosition(), a.getTypeName().getLength()); + ReconcileUtils.setRewriteFixes(registry, problem, List.of( + new FixDescriptor(RemoveAnnotation.class.getName(), List.of(docURI.toASCIIString()), "Remove `@%s`".formatted(ab.getName())) + .withParameters(Map.of("annotationPattern", "@" + ab.getQualifiedName())) + .withRecipeScope(RecipeScope.NODE) + .withRangeScope(ReconcileUtils.createOpenRewriteRange(cu, node, null)) + )); + context.getProblemCollector().accept(problem); + } + } + } + } + private List getImportedBeanRegistrarConfigs(List configBeans, ITypeBinding beanRegType) { return configBeans.stream() .filter(configBean -> isImportingBeanRegistrar(configBean, beanRegType)) diff --git a/headless-services/spring-boot-language-server/src/main/resources/problem-types.json b/headless-services/spring-boot-language-server/src/main/resources/problem-types.json index aae424725..05ea6b824 100644 --- a/headless-services/spring-boot-language-server/src/main/resources/problem-types.json +++ b/headless-services/spring-boot-language-server/src/main/resources/problem-types.json @@ -138,10 +138,16 @@ }, "order": 3, "problemTypes": [ + { + "code": "REGISTRAR_BEAN_INVALID_ANNOTATION", + "label": "Invalid annotation over bean registrar", + "description": "Bean Registrar cannot be registered as a bean via `@Component` annotations", + "defaultSeverity": "WARNING" + }, { "code": "REGISTRAR_BEAN_DECLARATION", - "label": "Not registered via `@Import` in a configuration bean", - "description": "Bean derived from BeanRegistrar should be registered via `@Import` over configuration bean", + "label": "Not added to configurartion via `@Import`", + "description": "Bean Registrar should be added to a configurarion bean via `@Import`", "defaultSeverity": "WARNING" } ] diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/BeanRegistrarDeclarationReconcilerTest.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/BeanRegistrarDeclarationReconcilerTest.java index 26166bb34..70fb97733 100644 --- a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/BeanRegistrarDeclarationReconcilerTest.java +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/BeanRegistrarDeclarationReconcilerTest.java @@ -20,16 +20,20 @@ import org.eclipse.lsp4j.Location; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.openrewrite.java.RemoveAnnotation; import org.springframework.ide.vscode.boot.index.SpringMetamodelIndex; import org.springframework.ide.vscode.boot.java.Annotations; +import org.springframework.ide.vscode.boot.java.Boot4JavaProblemType; import org.springframework.ide.vscode.boot.java.reconcilers.BeanRegistrarDeclarationReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.JdtAstReconciler; import org.springframework.ide.vscode.commons.java.IClasspathUtil; +import org.springframework.ide.vscode.commons.languageserver.quickfix.Quickfix.QuickfixData; import org.springframework.ide.vscode.commons.languageserver.quickfix.QuickfixRegistry; import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem; import org.springframework.ide.vscode.commons.protocol.spring.AnnotationAttributeValue; import org.springframework.ide.vscode.commons.protocol.spring.AnnotationMetadata; import org.springframework.ide.vscode.commons.protocol.spring.Bean; +import org.springframework.ide.vscode.commons.rewrite.java.FixDescriptor; public class BeanRegistrarDeclarationReconcilerTest extends BaseReconcilerTest { @@ -268,4 +272,121 @@ public class BeanRegistrarDeclarationReconcilerTest extends BaseReconcilerTest { assertEquals(1, problems.size()); } + + @Test + void componentAnnotationOver() throws Throwable { + String source = """ + package com.example.demo; + + import org.springframework.beans.factory.BeanRegistrar; + import org.springframework.beans.factory.BeanRegistry; + import org.springframework.core.env.Environment; + import org.springframework.stereotype.Component; + + @Component + public class MyBeanRegistrar implements BeanRegistrar { + + public void register(BeanRegistry registry, Environment env) { + } + + } + """; + List problems = reconcile(() -> { + SpringMetamodelIndex springIndex = new SpringMetamodelIndex(); + + Location l = new Location(); + Path sourceFolder = IClasspathUtil.getSourceFolders(project.getClasspath()).map(f -> f.toPath()).filter(p -> p.endsWith(Path.of("src", "main", "java"))).findFirst().orElseThrow(); + l.setUri(sourceFolder.resolve("com/example/demo/B.java").toUri().toASCIIString()); + + AnnotationMetadata annotationMetadata = new AnnotationMetadata(Annotations.CONFIGURATION, false, null, Map.of()); + AnnotationMetadata importMetadata = new AnnotationMetadata(Annotations.IMPORT, false, null, + Map.of("value", new AnnotationAttributeValue[] { + new AnnotationAttributeValue("com.example.demo.MyBeanRegistrar", null) })); + AnnotationMetadata[] annotations = new AnnotationMetadata[] {annotationMetadata, importMetadata}; + Bean configBean = new Bean("conf", "com.example.demo.Conf", l, null, null, annotations, true, "symbolLabel"); + Bean[] beans = new Bean[] {configBean}; + springIndex.updateBeans(getProjectName(), beans); + + BeanRegistrarDeclarationReconciler r = new BeanRegistrarDeclarationReconciler(new QuickfixRegistry(), springIndex); + + return r; + }, "A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem p = problems.get(0); + + assertEquals(Boot4JavaProblemType.REGISTRAR_BEAN_INVALID_ANNOTATION.getLabel(), p.getType().getLabel()); + + assertEquals(1, p.getQuickfixes().size()); + + QuickfixData qf = p.getQuickfixes().get(0); + + assertEquals("Remove `@Component`", qf.title); + + FixDescriptor fd = (FixDescriptor) qf.params; + + assertEquals(RemoveAnnotation.class.getName(), fd.getRecipeId()); + assertEquals("@org.springframework.stereotype.Component", fd.getParameters().get("annotationPattern")); + + } + + @Test + void serviceAnnotationOver() throws Throwable { + String source = """ + package com.example.demo; + + import org.springframework.beans.factory.BeanRegistrar; + import org.springframework.beans.factory.BeanRegistry; + import org.springframework.core.env.Environment; + import org.springframework.stereotype.Service; + + @Service("myService") + public class MyBeanRegistrar implements BeanRegistrar { + + public void register(BeanRegistry registry, Environment env) { + } + + } + """; + List problems = reconcile(() -> { + SpringMetamodelIndex springIndex = new SpringMetamodelIndex(); + + Location l = new Location(); + Path sourceFolder = IClasspathUtil.getSourceFolders(project.getClasspath()).map(f -> f.toPath()).filter(p -> p.endsWith(Path.of("src", "main", "java"))).findFirst().orElseThrow(); + l.setUri(sourceFolder.resolve("com/example/demo/B.java").toUri().toASCIIString()); + + AnnotationMetadata annotationMetadata = new AnnotationMetadata(Annotations.CONFIGURATION, false, null, Map.of()); + AnnotationMetadata importMetadata = new AnnotationMetadata(Annotations.IMPORT, false, null, + Map.of("value", new AnnotationAttributeValue[] { + new AnnotationAttributeValue("com.example.demo.MyBeanRegistrar", null) })); + AnnotationMetadata[] annotations = new AnnotationMetadata[] {annotationMetadata, importMetadata}; + Bean configBean = new Bean("conf", "com.example.demo.Conf", l, null, null, annotations, true, "symbolLabel"); + Bean[] beans = new Bean[] {configBean}; + springIndex.updateBeans(getProjectName(), beans); + + BeanRegistrarDeclarationReconciler r = new BeanRegistrarDeclarationReconciler(new QuickfixRegistry(), springIndex); + + return r; + }, "A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem p = problems.get(0); + + assertEquals(Boot4JavaProblemType.REGISTRAR_BEAN_INVALID_ANNOTATION.getLabel(), p.getType().getLabel()); + + assertEquals(1, p.getQuickfixes().size()); + + QuickfixData qf = p.getQuickfixes().get(0); + + assertEquals("Remove `@Service`", qf.title); + + FixDescriptor fd = (FixDescriptor) qf.params; + + assertEquals(RemoveAnnotation.class.getName(), fd.getRecipeId()); + assertEquals("@org.springframework.stereotype.Service", fd.getParameters().get("annotationPattern")); + + } + } diff --git a/vscode-extensions/vscode-spring-boot/package.json b/vscode-extensions/vscode-spring-boot/package.json index 4d3823969..59d90e680 100644 --- a/vscode-extensions/vscode-spring-boot/package.json +++ b/vscode-extensions/vscode-spring-boot/package.json @@ -739,10 +739,22 @@ "ON" ] }, + "spring-boot.ls.problem.boot4.REGISTRAR_BEAN_INVALID_ANNOTATION": { + "type": "string", + "default": "WARNING", + "description": "Bean Registrar cannot be registered as a bean via `@Component` annotations", + "enum": [ + "IGNORE", + "INFO", + "WARNING", + "HINT", + "ERROR" + ] + }, "spring-boot.ls.problem.boot4.REGISTRAR_BEAN_DECLARATION": { "type": "string", "default": "WARNING", - "description": "Bean derived from BeanRegistrar should be registered via `@Import` over configuration bean", + "description": "Bean Registrar should be added to a configurarion bean via `@Import`", "enum": [ "IGNORE", "INFO",