[ISSUE 112] [CRJVM208] get stale code of PRs 151 and 157 to merge them into this…

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#128](https://github.com/green-code-initiative/ecoCode/pull/128) Adding EC35 rule for Python and PHP : EC35 rule replaces EC34 with a specific use case ("file not found" sepcific)
 - [#132](https://github.com/green-code-initiative/ecoCode/issues/132) Upgrade RULES.md: set proposed Python rule "Use numpy array instead of standard list" as refused with link to the justification
 - [#103](https://github.com/green-code-initiative/ecoCode/issues/103) Upgrade RULES.md: set proposed HTML rule "HTML page must contain a doctype tag" as refused with link to the justification
+- [#112](https://github.com/green-code-initiative/ecoCode/issues/112) Updating EC1 rule to add controls on streams
 
 ### Deleted
 

RULES.md

@@ -38,7 +38,7 @@ Some are applicable for different technologies.
 | EC69     | Calling a function in the declaration of a for loop                                                        | Avoid calling the function each time the loop is iterated.                                                                                                                                                                                                                                                                                                                                                                   | [cnumr best practices (3rd edition) BP_069 (no longer exists in edition 4)](https://www.greenit.fr/2019/05/07/ecoconception-web-les-115-bonnes-pratiques-3eme-edition/) | ✅    | ✅   | 🚀 | ✅      | 🚀   |
 | EC72     | Perform an SQL query inside a loop                                                                         | Servers are optimized to process multiple selections, insertions, or changes in a single query or transaction. consume CPU cycles, RAM, and bandwidth unnecessarily.                                                                                                                                                                                                                                                         | [cnumr best practices (3rd edition) BP_072](https://github.com/cnumr/best-practices/blob/main/chapters/BP_072_fr.md)                                                    | ✅    | ✅   | 🚀 | ✅      | 🚀   |
 | EC74     | Write SELECT * FROM                                                                                        | The database server must resolve the fields based on the schema.  If you are familiar with the diagram, it is strongly recommended to name the fields.                                                                                                                                                                                                                                                                       | [cnumr best practices (3rd edition) BP_074 (no longer exists in edition 4)](https://www.greenit.fr/2019/05/07/ecoconception-web-les-115-bonnes-pratiques-3eme-edition/) | ✅    | ✅   | 🚀 | ✅      | 🚀   |
-| EC1      | Calling a Spring repository inside a loop                                                                  | The use of Spring repository in a loop induces unnecessary calculations by the CPU and therefore superfluous energy consumption.                                                                                                                                                                                                                                                                                             |                                                                                                                                                                         | ✅    | 🚫  | 🚫 | 🚫     | 🚫   |
+| EC1      | Calling a Spring repository inside a loop or a stream                                                      | The use of Spring repository in a loop or a stream induces unnecessary calculations by the CPU and therefore superfluous energy consumption.                                                                                                                                                                                                                                                                                 |                                                                                                                                                                         | ✅    | 🚫  | 🚫 | 🚫     | 🚫   |
 | EC3      | Getting the size of the collection in the loop                                                             | When iterating over any collection, fetch the size of the collection in advance to avoid fetching it on each iteration, this saves CPU cycles, and therefore consumes less power.                                                                                                                                                                                                                                            |                                                                                                                                                                         | ✅    | ✅   | 🚀 | 🚫     | 🚀   |
 | EC2      | Multiple if-else statement                                                                                 | Using too many conditional if-else statements will impact performance since JVM will have to compare the conditions. Prefer using a switch statement instead of multiple if-else if possible, or refactor your code to reduce conditonnal statements on the same variable. Switch statement has a performance advantage over if – else.                                                                                      |                                                                                                                                                                         | ✅    | ✅   | 🚀 | 🚧     | 🚀   |
 | EC76     | Usage of static collections                                                                                | Avoid usage of static collections. If you want to use static collections make them final and create for example a singleton if needed containing the collections. The static fields are more complicated for the Garbage Collector to manage and can lead to memory leaks.                                                                                                                                                   |                                                                                                                                                                         | ✅    | 🚫  | 🚫 | 🚫     | 🚫   |

ecocode-rules-specifications/src/main/rules/EC1/EC1.json

@@ -1,5 +1,5 @@
 {
-  "title": "Avoid Spring repository call in loop",
+  "title": "Avoid Spring repository call in loop or stream operations",
   "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {

ecocode-rules-specifications/src/main/rules/EC1/java/EC1.asciidoc

@@ -1,4 +1,5 @@
 The use of Spring repository in a loop induces unnecessary calculations by the CPU and therefore superfluous energy consumption.
+Also, the use of Spring repository in a stream operation like "peek, forEach, forEachOrdered, map" induces unnecessary multiple access to the database instead of single batch call.
 
 ## Noncompliant Code Example
 
@@ -15,9 +16,25 @@ for (Integer id: ids) {
 }
 ```
 
+```java
+List<Employee> employees = new ArrayList<>();
+Stream stream = Stream.of(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+stream.forEach(id -> {
+    Optional<Employee> employee = employeeRepository.findById(id); // Noncompliant
+    if (employee.isPresent()) {
+        employees.add(employee.get());
+    }
+});
+```
+
 ## Compliant Solution
 
 ```java
 private final List<Integer> ids = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
 List<Employee> employees = employeeRepository.findAllById(ids);
 ```
+
+```java
+private final List<Integer> ids = Stream.of(1, 2, 3, 4, 5, 6, 7, 8, 9, 10).toList();
+List<Employee> employees = employeeRepository.findAllById(ids);
+```

java-plugin/src/main/java/fr/greencodeinitiative/java/JavaCheckRegistrar.java

@@ -28,7 +28,7 @@
 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.AvoidSpringRepositoryCallInLoopOrStreamCheck;
 import fr.greencodeinitiative.java.checks.AvoidStatementForDMLQueries;
 import fr.greencodeinitiative.java.checks.AvoidUsageOfStaticCollections;
 import fr.greencodeinitiative.java.checks.AvoidUsingGlobalVariablesCheck;
@@ -60,7 +60,7 @@ public class JavaCheckRegistrar implements CheckRegistrar {
             AvoidRegexPatternNotStatic.class,
             NoFunctionCallWhenDeclaringForLoop.class,
             AvoidStatementForDMLQueries.class,
-            AvoidSpringRepositoryCallInLoopCheck.class,
+            AvoidSpringRepositoryCallInLoopOrStreamCheck.class,
             AvoidSQLRequestInLoop.class,
             AvoidFullSQLRequest.class,
             UseCorrectForLoop.class,

java-plugin/src/main/java/fr/greencodeinitiative/java/checks/AvoidSpringRepositoryCallInLoopOrStreamCheck.java

@@ -0,0 +1,123 @@
+/*
+ * ecoCode - Java language - Provides rules to reduce the environmental footprint of your Java programs
+ * Copyright © 2023 Green Code Initiative (https://www.ecocode.io)
+ *
+ * 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 fr.greencodeinitiative.java.checks;
+
+import java.util.Arrays;
+import java.util.List;
+
+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;
+
+@Rule(key = "EC1")
+@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GRC1")
+public class AvoidSpringRepositoryCallInLoopOrStreamCheck extends IssuableSubscriptionVisitor {
+
+    protected static final String RULE_MESSAGE = "Avoid Spring repository call in loop or stream";
+
+    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 AvoidSpringRepositoryCallInLoopCheckVisitor visitorInFile = new AvoidSpringRepositoryCallInLoopCheckVisitor();
+    private final StreamVisitor streamVisitor = new StreamVisitor();
+
+    private final AncestorMethodVisitor ancestorMethodVisitor = new AncestorMethodVisitor();
+
+    @Override
+    public List<Tree.Kind> 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);
+                    }
+                }
+            }
+        }
+
+    }
+
+}

java-plugin/src/test/files/AvoidSpringRepositoryCallInLoopCheck.java

@@ -21,16 +21,15 @@
 import org.springframework.data.jpa.repository.JpaRepository;
 
 import java.util.*;
-import java.util.stream.Collectors;
 
-public class AvoidRepositoryCallInLoopCheck {
+public class AvoidSpringRepositoryCallInLoopCheck {
     @Autowired
     private EmployeeRepository employeeRepository;
 
     public List<Employee> smellGetAllEmployeesByIds(List<Integer> ids) {
         List<Employee> employees = new ArrayList<>();
         for (Integer id : ids) {
-            Optional<Employee> employee = employeeRepository.findById(id); // Noncompliant {{Avoid Spring repository call in loop}}
+            Optional<Employee> employee = employeeRepository.findById(id); // Noncompliant {{Avoid Spring repository call in loop or stream}}
             if (employee.isPresent()) {
                 employees.add(employee.get());
             }
@@ -39,10 +38,20 @@ public List<Employee> smellGetAllEmployeesByIds(List<Integer> ids) {
     }
 
     public class Employee {
+        private Integer id;
+        private String name;
+
+        public Employee(Integer id, String name) {
+            this.id = id;
+            this.name = name;
+        }
+
+        public Integer getId() { return id; }
+        public String getName() { return name; }
     }
 
     public interface EmployeeRepository extends JpaRepository<Employee, Integer> {
 
     }
-    
+
 }