[CRJVM206] Avoid N+1 selects problem

[MohamedAliSidibe]

Description

This PR introduces a new rule to detect potential Hibernate N+1 query issues caused by lazy-loaded relationship access inside loops or stream operations.

The rule detects patterns such as:

for (Product product : products) {
    product.getOrders();
}

or

products.forEach(product -> product.getOrders().size());

These patterns may trigger one SQL query per iteration due to Hibernate lazy loading.

The rule analyzes:

• loops (for, foreach, while, do while)
• stream operations (forEach, map, peek, forEachOrdered)

and reports potential lazy relationship getter calls such as:

• getOrders()
• getRoles()
• getProducts()
• getAddresses()

This helps reduce unnecessary database access, CPU usage, and overall energy consumption caused by N+1 query patterns.

[MohamedAliSidibe]

Mahamadou Ali Sidibe / LesShreks des dépendances moisies / CGI

[MohamedAliSidibe]

Closes #36

[Copilot]

Pull request overview

This PR adds a new SonarJava rule (CRJVM206) intended to flag potential Hibernate N+1 query patterns by detecting plural-looking getter calls on objects accessed within loops or stream operations, and wires the rule into the built-in “creedengo way” profile alongside a new unit test + sample file.

Changes:

• Add AvoidHibernateLazyRelationAccessInLoopCheck rule implementation (CRJVM206).
• Add a CheckVerifier unit test and an IT sample file with compliant/noncompliant examples.
• Enable the rule in the built-in creedengo_way_profile.json and update the changelog.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidHibernateLazyRelationAccessInLoopCheck.java	New rule implementation to detect potential lazy relationship access inside loops/streams.
src/test/java/org/greencodeinitiative/creedengo/java/checks/CRJVM206/AvoidHibernateLazyRelationAccessInLoopCheckTest.java	Unit test verifying issues against a sample file.
src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/CRJVM206/AvoidHibernateLazyRelationAccessInLoopCheckSample.java	Sample code containing compliant/noncompliant patterns for verifier.
src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json	Adds CRJVM206 to the built-in profile’s ruleKeys.
CHANGELOG.md	Adds an entry describing the new rule (but currently duplicates “Unreleased” structure).

Comments suppressed due to low confidence (3)

src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidHibernateLazyRelationAccessInLoopCheck.java:61

• isStreamOperation() uses methodInvocationTree.symbol().name() without guarding against a null/unknown symbol and only checks the method name, so any non-stream method named map/peek/forEach* will be treated as a stream operation. Prefer a semantic matcher (e.g., MethodMatchers on java.util.stream.BaseStream like AvoidSpringRepositoryCallInLoopOrStreamCheck) and handle unresolved symbols safely.

    private boolean isStreamOperation(MethodInvocationTree methodInvocationTree) {
        String methodName = methodInvocationTree.symbol().name();
        return "forEach".equals(methodName)
                || "forEachOrdered".equals(methodName)
                || "map".equals(methodName)
                || "peek".equals(methodName);
    }

src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidHibernateLazyRelationAccessInLoopCheck.java:68

• isPotentialLazyRelationGetter() currently flags any method starting with "get" and ending with "s" (e.g., getStatus), which will create many false positives and still miss common relationship getters (e.g., getChildren/getPeople). Consider tightening the heuristic using semantic info (e.g., only methods returning Collection/Map/Iterable/array, and optionally only when the owning type is an @entity).

    private boolean isPotentialLazyRelationGetter(MethodInvocationTree methodInvocationTree) {
        String methodName = methodInvocationTree.symbol().name();
        return methodName.startsWith("get")
                && methodName.length() > 4
                && methodName.endsWith("s");
    }

src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidHibernateLazyRelationAccessInLoopCheck.java:41

• checkInsideTree() traverses the full subtree for every subscribed loop node, which can lead to duplicate issue reports when loops are nested (outer loop visit will report issues inside inner loops, and then inner loop visit will report them again). Consider skipping traversal into nested loops/stream operations when already in a loop context, or only analyzing the direct loop body/lambda of the matched node.

    @Override
    public void visitNode(Tree tree) {
        if (tree.is(Tree.Kind.METHOD_INVOCATION)) {
            MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree;

            if (isStreamOperation(methodInvocationTree)) {
                checkInsideTree(tree);
            }
        } else {
            checkInsideTree(tree);
        }
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rducasse]

Blockers:

• Build failure: try mvn verify locally and fix the errors accordingly
• Missing integration test in GCIRulesIT.java for creedengo-java:CRJVM206
• Missing package declaration in AvoidHibernateLazyRelationAccessInLoopCheckSample.java
• Too broad heuristic: isPotentialLazyRelationGetter flags any get*s() method — real code has getStatus(), getErrors(), getDetails() etc. that are NOT lazy relations. This will cause many false positives. Consider checking the return type (must extend Collection or Map) or document the limitation.
• Deprecated API: methodInvocationTree.symbol().name() is deprecated and marked for removal. Please use methodSelect() instead.
• Check upvoted Copilot messages.

Non-blocking:

• Add @ Override on nodesToVisit() and visitNode()
• Change MESSAGE from private to protected
• Add // Noncompliant {{exact message}} to all test markers
• Add test cases for while, do-while, forEachOrdered, peek