-
Notifications
You must be signed in to change notification settings - Fork 47
GCI1111[PLANET][2025] #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package org.greencodeinitiative.creedengo.java.checks; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| // L'interface doit être package-private (pas "public") pour rester dans ce fichier | ||
| @Repository | ||
| interface DataInHibernateMustBePaginated extends JpaRepository<Object, Long> { | ||
|
|
||
| List<Object> findAll(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| @Query(value = "SELECT * FROM users", nativeQuery = true) | ||
| List<Object> loadAll(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| @Query("SELECT u FROM User u") | ||
| List<Object> getUsers(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| Page<Object> findAll(Pageable pageable); // OK | ||
|
|
||
| @Query(value = "SELECT * FROM users", countQuery = "SELECT count(*) FROM users", nativeQuery = true) | ||
| Page<Object> findAllNative(Pageable pageable); // OK | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import java.util.List; | ||
| import org.springframework.stereotype.Repository; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.data.domain.Page; | ||
|
|
||
| public interface DataInHibernateMustBePaginatedNoIssue { | ||
|
|
||
| List<Object> findAll(); | ||
|
|
||
| @Query(value = "SELECT * FROM users", nativeQuery = true) | ||
| List<Object> loadAll(); | ||
|
|
||
| @Query("SELECT u FROM User u") | ||
| List<Object> getUsers(); | ||
|
|
||
| Page<Object> findAll(Pageable pageable); // OK | ||
|
|
||
| @Query(value = "SELECT * FROM users", countQuery = "SELECT count(*) FROM users", nativeQuery = true) | ||
| Page<Object> findAllNative(Pageable pageable); // OK | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||||||||||||||||
| package org.greencodeinitiative.creedengo.java.checks; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.*; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import org.sonar.check.Rule; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.InputFileScannerContext; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.tree.*; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.tree.Tree.Kind; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.semantic.Symbol; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.semantic.Type; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import org.sonar.check.Rule; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.tree.*; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.semantic.Symbol; | ||||||||||||||||||||
| import org.sonar.plugins.java.api.semantic.Type; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Rule(key = "GCI1111") | ||||||||||||||||||||
| public class DataInHibernateMustBePaginated extends IssuableSubscriptionVisitor { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private static final String MESSAGE = "Hibernate queries must be paginated to avoid excessive data loading"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public List<Tree.Kind> nodesToVisit() { | ||||||||||||||||||||
| return Collections.singletonList(Kind.METHOD); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void visitNode(Tree tree) { | ||||||||||||||||||||
| MethodTree methodTree = (MethodTree) tree; | ||||||||||||||||||||
| ClassTree enclosingClass = getEnclosingClass(tree); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (enclosingClass == null || !isRepository(enclosingClass)) { | ||||||||||||||||||||
| return; // Ne rien faire si ce n’est pas un Repository | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| // Récupérer le nom de la méthode | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please translate comments to english |
||||||||||||||||||||
| String methodName = methodTree.simpleName().name(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Récupérer le type de retour | ||||||||||||||||||||
| String returnType = methodTree.returnType().symbolType().toString(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Vérifier s’il retourne une collection entière (sans pagination) | ||||||||||||||||||||
| boolean returnsAllData = retrournAllData(returnType); | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo error in method name (please use something like "isToto" because it returns a boolean |
||||||||||||||||||||
|
|
||||||||||||||||||||
| // Vérifier s’il utilise la pagination | ||||||||||||||||||||
| boolean hasPaginationParam = isHasPaginationParam(methodTree); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Vérifier la présence de l’annotation @Query | ||||||||||||||||||||
| boolean usesQueryAnnotation = isUsesQueryAnnotation(methodTree); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| extracted(returnsAllData, hasPaginationParam, methodTree, usesQueryAnnotation); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void extracted(boolean returnsAllData, boolean hasPaginationParam, MethodTree methodTree, boolean usesQueryAnnotation) { | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use a method name more explicit ... when I read the method name I must understand what it will do |
||||||||||||||||||||
| // Déclencher la règle | ||||||||||||||||||||
| if (returnsAllData && !hasPaginationParam) { | ||||||||||||||||||||
| reportIssue(methodTree.simpleName(), | ||||||||||||||||||||
| MESSAGE); | ||||||||||||||||||||
| } else if (usesQueryAnnotation && returnsAllData && !hasPaginationParam) { | ||||||||||||||||||||
| reportIssue(methodTree.simpleName(), | ||||||||||||||||||||
| MESSAGE); | ||||||||||||||||||||
|
Comment on lines
+67
to
+72
|
||||||||||||||||||||
| if (returnsAllData && !hasPaginationParam) { | |
| reportIssue(methodTree.simpleName(), | |
| MESSAGE); | |
| } else if (usesQueryAnnotation && returnsAllData && !hasPaginationParam) { | |
| reportIssue(methodTree.simpleName(), | |
| MESSAGE); | |
| if ((returnsAllData && !hasPaginationParam) || (usesQueryAnnotation && returnsAllData && !hasPaginationParam)) { | |
| reportIssue(methodTree.simpleName(), | |
| MESSAGE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please correct this Copilot feedback
Copilot
AI
Jul 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using annotationType().toString() may not reliably match fully qualified annotation names; consider checking the annotation's symbol (e.g., ann.annotationType().symbolType().fullyQualifiedName()) for robustness.
| .anyMatch(ann -> ann.annotationType().toString().equals("Query")); | |
| .anyMatch(ann -> "org.springframework.data.jpa.repository.Query".equals(ann.annotationType().symbolType().fullyQualifiedName())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please delete "is" prefix because "has" prefix is enough
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| "GCI78", | ||
| "GCI79", | ||
| "GCI82", | ||
| "GCI94" | ||
| "GCI94", | ||
| "GCI1111" | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import java.util.List; | ||
| import org.springframework.stereotype.Repository; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.data.domain.Page; | ||
|
|
||
| @Repository | ||
| public interface UserRepository extends JpaRepository<User, Long> { | ||
|
|
||
| List<User> findAll(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| @Query(value = "SELECT * FROM users", nativeQuery = true) | ||
| List<User> loadAll(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| @Query("SELECT u FROM User u") | ||
| List<User> getUsers(); // Noncompliant {{Hibernate queries must be paginated to avoid excessive data loading}} | ||
|
|
||
| Page<User> findAll(Pageable pageable); // OK | ||
|
|
||
| @Query(value = "SELECT * FROM users", countQuery = "SELECT count(*) FROM users", nativeQuery = true) | ||
| Page<User> findAllNative(Pageable pageable); // OK | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import java.util.List; | ||
| import org.springframework.stereotype.Repository; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.data.domain.Page; | ||
|
|
||
| public interface UserRepository { | ||
|
|
||
| List<User> findAll(); | ||
|
|
||
| @Query(value = "SELECT * FROM users", nativeQuery = true) | ||
| List<User> loadAll(); | ||
|
|
||
| @Query("SELECT u FROM User u") | ||
| List<User> getUsers(); | ||
|
|
||
| Page<User> findAll(Pageable pageable); // OK | ||
|
|
||
| @Query(value = "SELECT * FROM users", countQuery = "SELECT count(*) FROM users", nativeQuery = true) | ||
| Page<User> findAllNative(Pageable pageable); // OK | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * 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 <http://www.gnu.org/licenses/>. | ||
| */ | ||
| package org.greencodeinitiative.creedengo.java.checks; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.sonar.java.checks.verifier.CheckVerifier; | ||
|
|
||
| class DataInHibernateMustBePaginatedTest { | ||
| @Test | ||
| void test() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile("src/test/files/DataInHibernateMustBePaginated.java") | ||
| .withCheck(new DataInHibernateMustBePaginated()) | ||
| .verifyIssues(); | ||
| } | ||
|
|
||
| @Test | ||
| void testNoIssue() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile("src/test/files/DataInHibernateMustBePaginatedNoIssue.java") | ||
| .withCheck(new DataInHibernateMustBePaginated()) | ||
| .verifyNoIssues(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Duplicate import of
Collectionsdetected; remove redundant imports (CollectionsandList) to clean up the imports section.