diff --git a/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java b/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java index d6a5cdbf4..7c9d0b495 100644 --- a/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java +++ b/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java @@ -24,25 +24,7 @@ import java.util.Collections; import java.util.List; -import fr.greencodeinitiative.java.checks.ArrayCopyCheck; -import fr.greencodeinitiative.java.checks.AvoidConcatenateStringsInLoop; -import fr.greencodeinitiative.java.checks.AvoidFullSQLRequest; -import fr.greencodeinitiative.java.checks.AvoidGettingSizeCollectionInLoop; -import fr.greencodeinitiative.java.checks.AvoidMultipleIfElseStatement; -import fr.greencodeinitiative.java.checks.AvoidRegexPatternNotStatic; -import fr.greencodeinitiative.java.checks.AvoidSQLRequestInLoop; -import fr.greencodeinitiative.java.checks.AvoidSetConstantInBatchUpdate; -import fr.greencodeinitiative.java.checks.AvoidSpringRepositoryCallInLoopCheck; -import fr.greencodeinitiative.java.checks.AvoidStatementForDMLQueries; -import fr.greencodeinitiative.java.checks.AvoidUsageOfStaticCollections; -import fr.greencodeinitiative.java.checks.AvoidUsingGlobalVariablesCheck; -import fr.greencodeinitiative.java.checks.FreeResourcesOfAutoCloseableInterface; -import fr.greencodeinitiative.java.checks.IncrementCheck; -import fr.greencodeinitiative.java.checks.InitializeBufferWithAppropriateSize; -import fr.greencodeinitiative.java.checks.NoFunctionCallWhenDeclaringForLoop; -import fr.greencodeinitiative.java.checks.OptimizeReadFileExceptions; -import fr.greencodeinitiative.java.checks.UnnecessarilyAssignValuesToVariables; -import fr.greencodeinitiative.java.checks.UseCorrectForLoop; +import fr.greencodeinitiative.java.checks.*; import org.sonar.plugins.java.api.JavaCheck; public final class RulesList { @@ -77,7 +59,8 @@ public static List> getJavaChecks() { AvoidUsingGlobalVariablesCheck.class, AvoidSetConstantInBatchUpdate.class, FreeResourcesOfAutoCloseableInterface.class, - AvoidMultipleIfElseStatement.class + AvoidMultipleIfElseStatement.class, + UseFetchTypeLazyRule.class )); } diff --git a/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRule.java b/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRule.java new file mode 100644 index 000000000..bb70c75f3 --- /dev/null +++ b/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRule.java @@ -0,0 +1,60 @@ +package fr.greencodeinitiative.java.checks; + +import org.sonar.check.Priority; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.JavaFileScanner; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.tree.*; + +@Rule( + key = "EC_CRJVM205", + name = "Developpement", + description = UseFetchTypeLazyRule.MESSAGE_RULE, + priority = Priority.MINOR, + tags = {"bug"}) +public class UseFetchTypeLazyRule extends BaseTreeVisitor implements JavaFileScanner { + protected static final String MESSAGE_RULE = "Avoid Using FetchType.EAGER instead of FetchType.LAZY on collections in JPA Entity"; + + private JavaFileScannerContext context; + + @Override + public void scanFile(JavaFileScannerContext javaFileScannerContext) { + this.context = javaFileScannerContext; + // The call to the scan method on the root of the tree triggers the visit of the AST by this visitor + scan(context.getTree()); + } + + @Override + public void visitAnnotation(AnnotationTree annotationTree) { + String annotationName = ((IdentifierTree) annotationTree.annotationType()).name(); + if (annotationName.equals("OneToMany") + || annotationName.equals("ManyToMany")) { + boolean fetchExist = false; + ExpressionTree fetchTypeArg = null; + + for (ExpressionTree argument : annotationTree.arguments()) { + if (argument.is(Tree.Kind.ASSIGNMENT)) { + AssignmentExpressionTree assignmentInvocation = (AssignmentExpressionTree) argument; + if (assignmentInvocation.variable().toString().equals("fetch")) { + fetchExist = true; + fetchTypeArg = assignmentInvocation.expression(); + } + } + } + + this.reportFetchTypeIssue(fetchExist,fetchTypeArg,annotationTree); + } + super.visitAnnotation(annotationTree); + } + + private void reportFetchTypeIssue(boolean fetchExist, ExpressionTree fetchTypeArg,AnnotationTree annotationTree){ + if (!fetchExist) { + context.reportIssue(this, annotationTree, "JPA annotation without FetchType detected"); + } else if (fetchTypeArg != null) { + String fetchType = ((MemberSelectExpressionTree) fetchTypeArg).identifier().name(); + if (!fetchType.strip().equals("LAZY")) { + context.reportIssue(this, annotationTree, MESSAGE_RULE); + } + } + } +} diff --git a/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.html b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.html new file mode 100644 index 000000000..ec55146c3 --- /dev/null +++ b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.html @@ -0,0 +1,34 @@ +

Avoid using a FetchType other than FetchType.LAZY

+

Using FetchType.EAGER can lead to performance issues and should be avoided. Always use FetchType.LAZY instead.

+

Noncompliant Code Example

+
+    @OneToMany(fetch = FetchType.EAGER, mappedBy = "otherEntity_id")
+    @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
+
+    @ManyToMany(fetch = FetchType.EAGER)
+    @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
+
+    @OneToMany
+    @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
+
+    @ManyToMany
+    @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
+

Compliant Solution

+
+    @OneToMany(fetch = FetchType.LAZY, mappedBy = "otherEntity_id")
+    @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
+
+    @ManyToMany(fetch = FetchType.LAZY, mappedBy = "otherEntity_id")
+        @Column(name = "otherEntity_id")
+    private Collection<otherEntity> otherEntities;
+
\ No newline at end of file diff --git a/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.json b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.json new file mode 100644 index 000000000..71d17749e --- /dev/null +++ b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC_CRJVM205.json @@ -0,0 +1,20 @@ +{ + "title": "Avoid using a FetchType other than FetchType.LAZY", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "jpa", + "fetch", + "lazy", + "eco-design", + "performance", + "memory", + "bug", + "ecocode" + ], + "defaultSeverity": "Minor" +} \ No newline at end of file diff --git a/java-plugin/src/test/files/UseFetchTypeLazyRule.java b/java-plugin/src/test/files/UseFetchTypeLazyRule.java new file mode 100644 index 000000000..92954df22 --- /dev/null +++ b/java-plugin/src/test/files/UseFetchTypeLazyRule.java @@ -0,0 +1,34 @@ +package fr.greencodeinitiative.java.checks; + +import javax.persistence.OneToMany; +import javax.persistence.ManyToOne; +import java.util.*; + +public class UseFetchTypeLazyRuleTest { + + @OneToMany(mappedBy = "firstEntity") // Noncompliant + private Collection firstEntities; + + @ManyToMany(mappedBy = "firstEntity1") // Noncompliant + private Collection firstEntities1; + + @OneToMany // Noncompliant + private Collection secondEntities1; + + @ManyToMany // Noncompliant + private Collection secondEntities2; + + @OneToMany(mappedBy = "thirdEntity1", fetch= FetchType.EAGER) // Noncompliant + private Collection thirdEntities1; + + @ManyToMany(mappedBy = "thirdEntity1", fetch= FetchType.EAGER) // Noncompliant + private Collection thirdEntities2; + + @OneToMany(fetch = FetchType.LAZY) // Compliant + private Collection fourthEntities1; + + @ManyToMany(fetch = FetchType.LAZY) // Compliant + private Collection fourthEntities2; + +} + diff --git a/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java b/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java index 3225b1a56..fe7f03adf 100644 --- a/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java +++ b/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java @@ -32,7 +32,7 @@ void checkNumberRules() { final JavaCheckRegistrar registrar = new JavaCheckRegistrar(); registrar.register(context); - assertThat(context.checkClasses()).hasSize(19); + assertThat(context.checkClasses()).hasSize(20); assertThat(context.testCheckClasses()).isEmpty(); } } diff --git a/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRuleTest.java b/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRuleTest.java new file mode 100644 index 000000000..7c78b776a --- /dev/null +++ b/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/UseFetchTypeLazyRuleTest.java @@ -0,0 +1,16 @@ +package fr.greencodeinitiative.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + + +class UseFetchTypeLazyRuleTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile("src/test/files/UseFetchTypeLazyRule.java") + .withCheck(new UseFetchTypeLazyRule()) + .verifyIssues(); + } +} \ No newline at end of file