From 8d3b2b850737974c59725667e21f2c54c4cc2a0e Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Tue, 20 May 2025 16:13:56 +0200 Subject: [PATCH 1/8] test case 1 --- ...voidNPlusOneProblemInJPAEntitiesCheck.java | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java new file mode 100644 index 00000000..8d056dfd --- /dev/null +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -0,0 +1,109 @@ +/* + * creedengo - Java language - Provides rules to reduce the environmental footprint of your Java programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org/) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.greencodeinitiative.creedengo.java.checks; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.jpa.repository.JpaRepository; + +import java.util.*; + +public class AvoidNPlusOneProblemInJPAEntitiesCheck { + + @Autowired + private AuthorRepository authorRepository; + + public List smellGetAllAuthors() { + List authors = authorRepository.findAll(); + for (Author author : authors) { + List books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur + } + return authors; + } + + + public class Author { + + private Long id; + private String name; + + private List books; + + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getBooks() { + return books; + } + + public void setBooks(List books) { + this.books = books; + } + } + + + public class Book { + + private Long id; + private String title; + + private Author author; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + public Author getAuthor() { + return author; + } + + public void setAuthor(Author author) { + this.author = author; + } + } + + public interface AuthorRepository extends JpaRepository { + + } + +} From 002a140643ed914157eb1a569dcd1d6fda352f06 Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Tue, 20 May 2025 16:53:39 +0200 Subject: [PATCH 2/8] Add first test case and failing rule --- ...lusOneProblemInJPAEntitiesCheckIssue.java} | 2 +- ...voidNPlusOneProblemInJPAEntitiesCheck.java | 105 +++++++++++++++++ ...PlusOneProblemInJPAEntitiesCheckIssue.java | 109 ++++++++++++++++++ ...NPlusOneProblemInJPAEntitiesCheckTest.java | 35 ++++++ 4 files changed, 250 insertions(+), 1 deletion(-) rename src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/{AvoidNPlusOneProblemInJPAEntitiesCheck.java => AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java} (97%) create mode 100644 src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java create mode 100644 src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java create mode 100644 src/test/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckTest.java diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java similarity index 97% rename from src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java rename to src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index 8d056dfd..61f1a325 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -22,7 +22,7 @@ import java.util.*; -public class AvoidNPlusOneProblemInJPAEntitiesCheck { +public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { @Autowired private AuthorRepository authorRepository; diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java new file mode 100644 index 00000000..da047021 --- /dev/null +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -0,0 +1,105 @@ +package org.greencodeinitiative.creedengo.java.checks; + +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.tree.*; +import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; + +import java.util.Arrays; +import java.util.List; + +@Rule(key = "GRC3") +@DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GRC3") +@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GRC3") +public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { + protected static final String RULE_MESSAGE = "Avoid N+1 with nested JPA Entities"; + + private static final String BASE_STREAM = "java.util.stream.BaseStream"; + private static final String SPRING_REPOSITORY = "org.springframework.data.repository.Repository"; + + private static final MethodMatchers SPRING_REPOSITORY_METHOD = + MethodMatchers + .create() + .ofSubTypes(SPRING_REPOSITORY) + .anyName() + .withAnyParameters() + .build(); + + private static final MethodMatchers STREAM_FOREACH_METHOD = + MethodMatchers + .create() + .ofSubTypes(BASE_STREAM) + .names("forEach", "forEachOrdered", "map", "peek") + .withAnyParameters() + .build(); + + private final AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor visitorInFile = new AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor(); + private final AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor streamVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor(); + + private final AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor ancestorMethodVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor(); + + @Override + public List nodesToVisit() { + return Arrays.asList( + Tree.Kind.FOR_EACH_STATEMENT, // loop + Tree.Kind.FOR_STATEMENT, // loop + Tree.Kind.WHILE_STATEMENT, // loop + Tree.Kind.DO_STATEMENT, // loop + Tree.Kind.METHOD_INVOCATION // stream + ); + } + + @Override + public void visitNode(Tree tree) { + if (tree.is(Tree.Kind.METHOD_INVOCATION)) { // stream process + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree; + if (STREAM_FOREACH_METHOD.matches(methodInvocationTree)) { + tree.accept(streamVisitor); + } + } else { // loop process + tree.accept(visitorInFile); + } + } + + private class AvoidSpringRepositoryCallInLoopCheckVisitor extends BaseTreeVisitor { + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + if (SPRING_REPOSITORY_METHOD.matches(tree)) { + reportIssue(tree, RULE_MESSAGE); + } else { + super.visitMethodInvocation(tree); + } + } + + } + + private class StreamVisitor extends BaseTreeVisitor { + + @Override + public void visitLambdaExpression(LambdaExpressionTree tree) { + tree.accept(ancestorMethodVisitor); + } + + } + + private class AncestorMethodVisitor extends BaseTreeVisitor { + + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + // if the method is a spring repository method, report an issue + if (SPRING_REPOSITORY_METHOD.matches(tree)) { + reportIssue(tree, RULE_MESSAGE); + } else { // else, check if the method is a method invocation and check recursively + if (tree.methodSelect().is(Tree.Kind.MEMBER_SELECT)) { + MemberSelectExpressionTree memberSelectTree = (MemberSelectExpressionTree) tree.methodSelect(); + if ( memberSelectTree.expression().is(Tree.Kind.METHOD_INVOCATION)) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) memberSelectTree.expression(); + methodInvocationTree.accept(ancestorMethodVisitor); + } + } + } + } + + } +} diff --git a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java new file mode 100644 index 00000000..61f1a325 --- /dev/null +++ b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -0,0 +1,109 @@ +/* + * creedengo - Java language - Provides rules to reduce the environmental footprint of your Java programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org/) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.greencodeinitiative.creedengo.java.checks; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.jpa.repository.JpaRepository; + +import java.util.*; + +public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { + + @Autowired + private AuthorRepository authorRepository; + + public List smellGetAllAuthors() { + List authors = authorRepository.findAll(); + for (Author author : authors) { + List books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur + } + return authors; + } + + + public class Author { + + private Long id; + private String name; + + private List books; + + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getBooks() { + return books; + } + + public void setBooks(List books) { + this.books = books; + } + } + + + public class Book { + + private Long id; + private String title; + + private Author author; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + public Author getAuthor() { + return author; + } + + public void setAuthor(Author author) { + this.author = author; + } + } + + public interface AuthorRepository extends JpaRepository { + + } + +} diff --git a/src/test/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckTest.java b/src/test/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckTest.java new file mode 100644 index 00000000..a960c569 --- /dev/null +++ b/src/test/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckTest.java @@ -0,0 +1,35 @@ +/* + * creedengo - Java language - Provides rules to reduce the environmental footprint of your Java programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org/) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.greencodeinitiative.creedengo.java.checks; + +import org.greencodeinitiative.creedengo.java.utils.FilesUtils; +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +class AvoidNPlusOneProblemInJPAEntitiesCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile("src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java") + .withCheck(new AvoidNPlusOneProblemInJPAEntitiesCheck()) + .withClassPath(FilesUtils.getClassPath("target/test-jars")) + .verifyIssues(); + } + +} From 48eee8c2c6e8edb1b306cfdfbd5d7518406e4a46 Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Wed, 21 May 2025 09:42:55 +0200 Subject: [PATCH 3/8] Implement N+1 problem detection for JPA entities with enhanced reporting --- ...PlusOneProblemInJPAEntitiesCheckIssue.java | 2 +- .../creedengo/java/JavaCheckRegistrar.java | 3 +- ...voidNPlusOneProblemInJPAEntitiesCheck.java | 124 +++++++++--------- .../creedengo/java/creedengo_way_profile.json | 3 +- ...PlusOneProblemInJPAEntitiesCheckIssue.java | 2 +- 5 files changed, 68 insertions(+), 66 deletions(-) diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index 61f1a325..f4b51e15 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { public List smellGetAllAuthors() { List authors = authorRepository.findAll(); for (Author author : authors) { - List books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur + List books = author.getBooks(); // // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }} } return authors; } diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java b/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java index 791f0cef..0fe301ae 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java @@ -50,7 +50,8 @@ public class JavaCheckRegistrar implements CheckRegistrar { FreeResourcesOfAutoCloseableInterface.class, AvoidMultipleIfElseStatement.class, UseOptionalOrElseGetVsOrElse.class, - MakeNonReassignedVariablesConstants.class + MakeNonReassignedVariablesConstants.class, + AvoidNPlusOneProblemInJPAEntitiesCheck.class ); /** diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java index da047021..1d79d64e 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -3,103 +3,103 @@ import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Symbol.VariableSymbol; import org.sonar.plugins.java.api.tree.*; import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; -import java.util.Arrays; -import java.util.List; +import java.util.*; @Rule(key = "GRC3") @DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GRC3") @DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GRC3") public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { - protected static final String RULE_MESSAGE = "Avoid N+1 with nested JPA Entities"; - private static final String BASE_STREAM = "java.util.stream.BaseStream"; + protected static final String RULE_MESSAGE = " Evitez le N+1 : utilisez un fetch join ou une récupération eager. "; + private static final String SPRING_REPOSITORY = "org.springframework.data.repository.Repository"; - private static final MethodMatchers SPRING_REPOSITORY_METHOD = - MethodMatchers - .create() + private static final MethodMatchers SPRING_REPOSITORY_METHOD_FIND_ALL = + MethodMatchers.create() .ofSubTypes(SPRING_REPOSITORY) - .anyName() - .withAnyParameters() - .build(); - - private static final MethodMatchers STREAM_FOREACH_METHOD = - MethodMatchers - .create() - .ofSubTypes(BASE_STREAM) - .names("forEach", "forEachOrdered", "map", "peek") + .names("findAll") .withAnyParameters() .build(); - private final AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor visitorInFile = new AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor(); - private final AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor streamVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor(); - - private final AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor ancestorMethodVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor(); + private final Map repositoryFindAllVars = new HashMap<>(); @Override public List nodesToVisit() { - return Arrays.asList( - Tree.Kind.FOR_EACH_STATEMENT, // loop - Tree.Kind.FOR_STATEMENT, // loop - Tree.Kind.WHILE_STATEMENT, // loop - Tree.Kind.DO_STATEMENT, // loop - Tree.Kind.METHOD_INVOCATION // stream - ); + return Arrays.asList(Tree.Kind.VARIABLE, Tree.Kind.METHOD_INVOCATION, Tree.Kind.FOR_EACH_STATEMENT); } @Override public void visitNode(Tree tree) { - if (tree.is(Tree.Kind.METHOD_INVOCATION)) { // stream process - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree; - if (STREAM_FOREACH_METHOD.matches(methodInvocationTree)) { - tree.accept(streamVisitor); + if (tree.is(Tree.Kind.VARIABLE)) { + VariableTree variableTree = (VariableTree) tree; + ExpressionTree initializer = variableTree.initializer(); + if (initializer != null && initializer.is(Tree.Kind.METHOD_INVOCATION)) { + MethodInvocationTree methodInvocation = (MethodInvocationTree) initializer; + if (SPRING_REPOSITORY_METHOD_FIND_ALL.matches(methodInvocation)) { + VariableSymbol symbol = (VariableSymbol) variableTree.symbol(); + repositoryFindAllVars.put(symbol, tree); + } } - } else { // loop process - tree.accept(visitorInFile); } - } - private class AvoidSpringRepositoryCallInLoopCheckVisitor extends BaseTreeVisitor { - @Override - public void visitMethodInvocation(MethodInvocationTree tree) { - if (SPRING_REPOSITORY_METHOD.matches(tree)) { - reportIssue(tree, RULE_MESSAGE); - } else { - super.visitMethodInvocation(tree); + // Cas d'un foreach sur une variable issue de findAll() + if (tree.is(Tree.Kind.FOR_EACH_STATEMENT)) { + ForEachStatement forEach = (ForEachStatement) tree; + ExpressionTree iterable = forEach.expression(); + if (iterable.is(Tree.Kind.IDENTIFIER)) { + Symbol symbol = ((IdentifierTree) iterable).symbol(); + if (repositoryFindAllVars.containsKey(symbol)) { + // On marque la variable d'itération comme issue d'un findAll() + VariableSymbol loopVar = (VariableSymbol) forEach.variable().symbol(); + repositoryFindAllVars.put(loopVar, tree); + } } } - } - - private class StreamVisitor extends BaseTreeVisitor { + // Détection d'un appel de getter sur une variable issue de findAll() + if (tree.is(Tree.Kind.METHOD_INVOCATION)) { + MethodInvocationTree methodInvocation = (MethodInvocationTree) tree; - @Override - public void visitLambdaExpression(LambdaExpressionTree tree) { - tree.accept(ancestorMethodVisitor); - } + // Check if the call is something like post.getAuthor() or post.getAuthor().getName() + ExpressionTree select = methodInvocation.methodSelect(); + if (select.is(Tree.Kind.MEMBER_SELECT)) { + MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) select; + ExpressionTree root = memberSelect.expression(); - } + if (root.is(Tree.Kind.IDENTIFIER)) { + Symbol symbol = ((IdentifierTree) root).symbol(); + if (repositoryFindAllVars.containsKey(symbol) && isGetter(memberSelect.identifier().name())) { + reportIssue(methodInvocation, RULE_MESSAGE); + } + } - private class AncestorMethodVisitor extends BaseTreeVisitor { - - @Override - public void visitMethodInvocation(MethodInvocationTree tree) { - // if the method is a spring repository method, report an issue - if (SPRING_REPOSITORY_METHOD.matches(tree)) { - reportIssue(tree, RULE_MESSAGE); - } else { // else, check if the method is a method invocation and check recursively - if (tree.methodSelect().is(Tree.Kind.MEMBER_SELECT)) { - MemberSelectExpressionTree memberSelectTree = (MemberSelectExpressionTree) tree.methodSelect(); - if ( memberSelectTree.expression().is(Tree.Kind.METHOD_INVOCATION)) { - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) memberSelectTree.expression(); - methodInvocationTree.accept(ancestorMethodVisitor); + // Handle nested getter chains (e.g., post.getAuthor().getName()) + if (root.is(Tree.Kind.METHOD_INVOCATION)) { + MethodInvocationTree rootInvocation = (MethodInvocationTree) root; + ExpressionTree deeperSelect = rootInvocation.methodSelect(); + if (deeperSelect.is(Tree.Kind.MEMBER_SELECT)) { + MemberSelectExpressionTree deeperMemberSelect = (MemberSelectExpressionTree) deeperSelect; + ExpressionTree deeperRoot = deeperMemberSelect.expression(); + + if (deeperRoot.is(Tree.Kind.IDENTIFIER)) { + Symbol rootSymbol = ((IdentifierTree) deeperRoot).symbol(); + if (repositoryFindAllVars.containsKey(rootSymbol) && isGetter(deeperMemberSelect.identifier().name())) { + reportIssue(methodInvocation, RULE_MESSAGE); + } + } } } } } + } + // Méthode utilitaire pour détecter un getter + private boolean isGetter(String methodName) { + return methodName.startsWith("get") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3)); } } diff --git a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json index 059bf0f5..d044f58c 100644 --- a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json +++ b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json @@ -18,6 +18,7 @@ "GCI78", "GCI79", "GCI82", - "GCI94" + "GCI94", + "GRC3" ] } diff --git a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index 61f1a325..fd7f479c 100644 --- a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java +++ b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { public List smellGetAllAuthors() { List authors = authorRepository.findAll(); for (Author author : authors) { - List books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur + List books = author.getBooks(); // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }} } return authors; } From 4e3fb1be29021e1fc3b255da77a64c59148ea603 Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Wed, 21 May 2025 10:02:09 +0200 Subject: [PATCH 4/8] Update N+1 problem detection rule key and adjust related configurations --- pom.xml | 2 +- .../java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java | 6 +++--- .../creedengo/java/creedengo_way_profile.json | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index ae43a190..1874414b 100644 --- a/pom.xml +++ b/pom.xml @@ -78,7 +78,7 @@ 1.8 - 2.2.2 + main-snapshot https://repo1.maven.org/maven2 diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java index 1d79d64e..928faa3d 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -10,9 +10,9 @@ import java.util.*; -@Rule(key = "GRC3") -@DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GRC3") -@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GRC3") +@Rule(key = "GCI604") +@DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GCI604") +@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GCI604") public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { protected static final String RULE_MESSAGE = " Evitez le N+1 : utilisez un fetch join ou une récupération eager. "; diff --git a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json index d044f58c..299073b9 100644 --- a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json +++ b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json @@ -19,6 +19,6 @@ "GCI79", "GCI82", "GCI94", - "GRC3" + "GCI604" ] } From 6f0abb6f389ffa3c5232ea69b7aa149264fbfced Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Wed, 21 May 2025 13:05:57 +0200 Subject: [PATCH 5/8] remove deprecated annotations --- .../java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java index 928faa3d..a37b71fd 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -6,13 +6,10 @@ import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.semantic.Symbol.VariableSymbol; import org.sonar.plugins.java.api.tree.*; -import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; import java.util.*; @Rule(key = "GCI604") -@DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GCI604") -@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GCI604") public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { protected static final String RULE_MESSAGE = " Evitez le N+1 : utilisez un fetch join ou une récupération eager. "; From 79cd8c7f8865d4625fdfe07e170fffd0692f3711 Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Wed, 21 May 2025 15:11:28 +0200 Subject: [PATCH 6/8] Change rule message to english --- .../java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java | 2 +- src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java index a37b71fd..2fe0ed3f 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -12,7 +12,7 @@ @Rule(key = "GCI604") public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { - protected static final String RULE_MESSAGE = " Evitez le N+1 : utilisez un fetch join ou une récupération eager. "; + protected static final String RULE_MESSAGE = " Avoid the N+1 problem: use a fetch join or eager fetching. "; private static final String SPRING_REPOSITORY = "org.springframework.data.repository.Repository"; diff --git a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index fd7f479c..c2f30881 100644 --- a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java +++ b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { public List smellGetAllAuthors() { List authors = authorRepository.findAll(); for (Author author : authors) { - List books = author.getBooks(); // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }} + List books = author.getBooks(); // Noncompliant {{ Avoid the N+1 problem: use a fetch join or eager fetching. }} } return authors; } From 0b9e962075671722e2f02e9d3eb5e2fc7eb64d40 Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Mon, 26 May 2025 20:46:55 +0200 Subject: [PATCH 7/8] Update rule message for N+1 problem detection in JPA entities --- .../java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java | 2 +- src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java index 2fe0ed3f..c06c3bf4 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java @@ -12,7 +12,7 @@ @Rule(key = "GCI604") public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { - protected static final String RULE_MESSAGE = " Avoid the N+1 problem: use a fetch join or eager fetching. "; + protected static final String RULE_MESSAGE = " Detection of the \"N+1 problem\" on Spring Data JPA repositories "; private static final String SPRING_REPOSITORY = "org.springframework.data.repository.Repository"; diff --git a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index c2f30881..4c698a6f 100644 --- a/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java +++ b/src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { public List smellGetAllAuthors() { List authors = authorRepository.findAll(); for (Author author : authors) { - List books = author.getBooks(); // Noncompliant {{ Avoid the N+1 problem: use a fetch join or eager fetching. }} + List books = author.getBooks(); // Noncompliant {{ Detection of the "N+1 problem" on Spring Data JPA repositories }} } return authors; } From 934fe68730442824d2ffa8f5fe0c86f80525d6ff Mon Sep 17 00:00:00 2001 From: Nassim Amghar <2727128+NassimAMGHAR@users.noreply.github.com.> Date: Mon, 26 May 2025 20:53:23 +0200 Subject: [PATCH 8/8] Update noncompliant message for N+1 problem detection in JPA entities --- .../checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java index f4b51e15..4c698a6f 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java @@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue { public List smellGetAllAuthors() { List authors = authorRepository.findAll(); for (Author author : authors) { - List books = author.getBooks(); // // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }} + List books = author.getBooks(); // Noncompliant {{ Detection of the "N+1 problem" on Spring Data JPA repositories }} } return authors; }