Skip to content

Commit

Permalink
Merge pull request #13619 from mkouba/detect-wrong-singleton
Browse files Browse the repository at this point in the history
Detect wrong usage of singleton annotations
  • Loading branch information
mkouba authored Dec 2, 2020
2 parents 5e57dda + e995483 commit 9256986
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 48 deletions.
98 changes: 53 additions & 45 deletions extensions/arc/deployment/pom.xml
Original file line number Diff line number Diff line change
@@ -1,53 +1,61 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>quarkus-arc-parent</artifactId>
<groupId>io.quarkus</groupId>
<version>999-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>quarkus-arc-parent</artifactId>
<groupId>io.quarkus</groupId>
<version>999-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>quarkus-arc-deployment</artifactId>
<name>Quarkus - ArC - Deployment</name>
<artifactId>quarkus-arc-deployment</artifactId>
<name>Quarkus - ArC - Deployment</name>

<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-core-deployment</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-arc</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus.arc</groupId>
<artifactId>arc-processor</artifactId>
</dependency>
<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-core-deployment</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-arc</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus.arc</groupId>
<artifactId>arc-processor</artifactId>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5-internal</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5-internal</artifactId>
<scope>test</scope>
</dependency>

</dependencies>
<!-- Used to test wrong @Singleton detection -->
<dependency>
<groupId>org.jboss.spec.javax.ejb</groupId>
<artifactId>jboss-ejb-api_3.1_spec</artifactId>
<version>1.0.2.Final</version>
<scope>test</scope>
</dependency>

<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-extension-processor</artifactId>
<version>${project.version}</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
</plugins>
</build>
</dependencies>

<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-extension-processor</artifactId>
<version>${project.version}</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,16 @@ public class ArcConfig {
@ConfigItem(defaultValue = "true")
public boolean detectUnusedFalsePositives;

/**
* If set to true then the container attempts to detect usage of <i>wrong</i> annotations.
* <p>
* A <i>wrong</i> annotation may lead to unexpected behavior in a Quarkus application. A typical example is
* {@code @javax.ejb.Singleton} which is often confused with {@code @javax.inject.Singleton}. As a result a component
* annotated with {@code @javax.ejb.Singleton} can be completely ignored.
*/
@ConfigItem(defaultValue = "true")
public boolean detectWrongAnnotations;

public final boolean isRemoveUnusedBeansFieldValid() {
return ALLOWED_REMOVE_UNUSED_BEANS_VALUES.contains(removeUnusedBeans.toLowerCase());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.AdditionalApplicationArchiveMarkerBuildItem;
import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem;
import io.quarkus.deployment.builditem.ApplicationClassPredicateBuildItem;
import io.quarkus.deployment.builditem.ApplicationIndexBuildItem;
import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
import io.quarkus.deployment.builditem.CapabilityBuildItem;
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
Expand Down Expand Up @@ -129,7 +129,7 @@ public ContextRegistrationPhaseBuildItem initialize(
ArcConfig arcConfig,
BeanArchiveIndexBuildItem beanArchiveIndex,
CombinedIndexBuildItem combinedIndex,
ApplicationArchivesBuildItem applicationArchivesBuildItem,
ApplicationIndexBuildItem applicationIndex,
List<AnnotationsTransformerBuildItem> annotationTransformers,
List<InjectionPointTransformerBuildItem> injectionPointTransformers,
List<ObserverTransformerBuildItem> observerTransformers,
Expand Down Expand Up @@ -172,7 +172,7 @@ public ContextRegistrationPhaseBuildItem initialize(
Set<DotName> generatedClassNames = beanArchiveIndex.getGeneratedClassNames();
IndexView index = beanArchiveIndex.getIndex();
BeanProcessor.Builder builder = BeanProcessor.builder();
IndexView applicationClassesIndex = applicationArchivesBuildItem.getRootArchive().getIndex();
IndexView applicationClassesIndex = applicationIndex.getIndex();
builder.setApplicationClassPredicate(new AbstractCompositeApplicationClassesPredicate<DotName>(
applicationClassesIndex, generatedClassNames, applicationClassPredicates, testClassPredicate) {
@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package io.quarkus.arc.deployment;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.DotName;
import org.jboss.jandex.IndexView;

import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.builditem.ApplicationIndexBuildItem;

public class WrongAnnotationsProcessor {

@BuildStep
void detect(ArcConfig config, ApplicationIndexBuildItem applicationIndex,
BuildProducer<ValidationErrorBuildItem> validationError) {

if (!config.detectWrongAnnotations) {
return;
}

IndexView index = applicationIndex.getIndex();
List<WrongAnnotation> wrongAnnotations = new ArrayList<>();
Function<AnnotationInstance, String> singletonFun = new Function<AnnotationInstance, String>() {

@Override
public String apply(AnnotationInstance annotationInstance) {
return String.format("%s declared on %s, use @javax.inject.Singleton instead",
annotationInstance.toString(false), getTargetInfo(annotationInstance));
}
};
wrongAnnotations.add(new WrongAnnotation(DotName.createSimple("com.google.inject.Singleton"), singletonFun));
wrongAnnotations.add(new WrongAnnotation(DotName.createSimple("javax.ejb.Singleton"), singletonFun));
wrongAnnotations.add(new WrongAnnotation(DotName.createSimple("groovy.lang.Singleton"), singletonFun));
wrongAnnotations.add(new WrongAnnotation(DotName.createSimple("jakarta.ejb.Singleton"), singletonFun));

Map<AnnotationInstance, String> wrongUsages = new HashMap<>();

for (WrongAnnotation wrongAnnotation : wrongAnnotations) {
for (AnnotationInstance annotationInstance : index.getAnnotations(wrongAnnotation.name)) {
wrongUsages.put(annotationInstance, wrongAnnotation.messageFun.apply(annotationInstance));
}
}

if (!wrongUsages.isEmpty()) {
for (Entry<AnnotationInstance, String> entry : wrongUsages.entrySet()) {
validationError.produce(new ValidationErrorBuildItem(
new IllegalStateException(entry.getValue())));
}
}
}

private static class WrongAnnotation {

final DotName name;
final Function<AnnotationInstance, String> messageFun;

WrongAnnotation(DotName name, Function<AnnotationInstance, String> messageFun) {
this.name = name;
this.messageFun = messageFun;
}

}

private static String getTargetInfo(AnnotationInstance annotationInstance) {
AnnotationTarget target = annotationInstance.target();
switch (target.kind()) {
case FIELD:
return target.asField().declaringClass().toString() + "."
+ target.asField().name();
case METHOD:
return target.asMethod().declaringClass().toString()
+ "."
+ target.asMethod().name() + "()";
default:
return target.toString();
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package io.quarkus.arc.test.wrongsingleton;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import java.util.Collections;
import java.util.List;

import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.inject.Produces;
import javax.inject.Inject;

import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.runtime.util.ExceptionUtil;
import io.quarkus.test.QuarkusUnitTest;

public class WrongSingletonTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(EjbSingleton.class))
.assertException(t -> {
Throwable rootCause = ExceptionUtil.getRootCause(t);
assertTrue(rootCause.getMessage().contains("javax.ejb.Singleton"), t.toString());
assertTrue(rootCause.getMessage().contains("com.google.inject.Singleton"), t.toString());
});

@Test
public void testValidationFailed() {
// This method should not be invoked
fail();
}

@javax.ejb.Singleton
static class EjbSingleton {

@Inject
@ConfigProperty(name = "unconfigured")
String foo;

}

@ApplicationScoped
static class GuiceProducers {

@com.google.inject.Singleton
@Produces
List<String> produceEjbSingleton() {
return Collections.emptyList();
}

}

}

0 comments on commit 9256986

Please sign in to comment.