Merge pull request #103 from max-208/69-java

[Challenge-25][GCI69][S.T.E.P] Enumeration false positive

Author: dedece35

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 ### Changed
+
+- [#103](https://github.com/green-code-initiative/creedengo-java/pull/103) GCI69 Java : calls to hasMoreElements() and nextElement() methods from java.util.Enumeration interface aren't flagged anymore when called in a for loop
 - [#110](https://github.com/green-code-initiative/creedengo-java/pull/110) GCI82 - remove false positives with reassignment using this and with passing a variable to a function it can be reassigned in
 - compatibility updates for SonarQube 25.5.0
 - upgrade libraries versions

src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java

@@ -451,8 +451,8 @@ void testGCI69() {
         String filePath = "src/main/java/org/greencodeinitiative/creedengo/java/checks/NoFunctionCallWhenDeclaringForLoop.java";
         String ruleId = "creedengo-java:GCI69";
         String ruleMsg = "Do not call a function when declaring a for-type loop";
-        int[] startLines = new int[]{58, 66, 74, 101};
-        int[] endLines = new int[]{58, 66, 74, 101};
+        int[] startLines = new int[]{62, 70, 78, 106, 127};
+        int[] endLines = new int[]{62, 70, 78, 106, 127};
 
         checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines);
     }

src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/NoFunctionCallWhenDeclaringForLoop.java

@@ -19,6 +19,10 @@
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Arrays;
+import java.util.Enumeration;
+import java.util.Collections;
+
+
 class NoFunctionCallWhenDeclaringForLoop {
 
     public int getMyValue() {
@@ -94,26 +98,56 @@ public void test7() {
         }
 
         // iterator called in an indirect way is allowed
-        for (final OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine.iterator()); otherClass.iterator.hasNext(); jour = otherClass.iterator.next()) {
+        for (final OtherClassWrapper otherClass = new OtherClassWrapper(joursSemaine.iterator()); otherClass.iterator.hasNext(); jour = otherClass.iterator.next()) {
             System.out.println(jour);
         }
+
         // but using a method that returns an iterator causes an issue
-        for (final OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine.iterator()); otherClass.getIterator().hasNext(); jour = otherClass.getIterator().next()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
+        for (final OtherClassWrapper otherClass = new OtherClassWrapper(joursSemaine.iterator()); otherClass.getIterator().hasNext(); jour = otherClass.getIterator().next()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
+            System.out.println(jour);
+        }
+
+    }
+
+    // compliant, enumeration is allowed
+    public void test8() {
+        final List<String> joursSemaine = Arrays.asList("Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche");
+
+        String jour = null;
+        for (final Enumeration<String> enumeration = Collections.enumeration(joursSemaine); enumeration.hasMoreElements(); jour = enumeration.nextElement()) {
+            System.out.println(jour);
+        }
+
+        // enumeration called in an indirect way is allowed
+        for(final OtherClassWrapper otherClass = new OtherClassWrapper(Collections.enumeration(joursSemaine)); otherClass.enumeration.hasMoreElements(); jour = otherClass.enumeration.nextElement()) {
+            System.out.println(jour);
+        }
+
+        // but using a method that returns an enumeration causes an issue
+        for(final OtherClassWrapper otherClass = new OtherClassWrapper(Collections.enumeration(joursSemaine)); otherClass.getEnumeration().hasMoreElements(); jour = otherClass.getEnumeration().nextElement()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
             System.out.println(jour);
         }
 
     }
 
 }
 
-class OtherClassWithIterator {
-    public final Iterator<String> iterator;
+class OtherClassWrapper {
+    public Iterator<String> iterator = null;
+    public Enumeration<String> enumeration = null;
 
-    public OtherClassWithIterator(Iterator<String> iterator){
+    public OtherClassWrapper(Iterator<String> iterator){
         this.iterator = iterator;
     }
+    public OtherClassWrapper(Enumeration<String> enumeration){
+        this.enumeration = enumeration;
+    }
 
     public Iterator<String> getIterator(){
         return iterator;
     }
+
+    public Enumeration<String> getEnumeration(){
+        return enumeration;
+    }
 }

src/main/java/org/greencodeinitiative/creedengo/java/checks/NoFunctionCallWhenDeclaringForLoop.java

@@ -26,6 +26,7 @@
 
 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.BaseTreeVisitor;
 import org.sonar.plugins.java.api.tree.ClassTree;
 import org.sonar.plugins.java.api.tree.CompilationUnitTree;
@@ -44,6 +45,20 @@ public class NoFunctionCallWhenDeclaringForLoop extends IssuableSubscriptionVisi
 
     protected static final String MESSAGERULE = "Do not call a function when declaring a for-type loop";
 
+    private static final String ITERATOR = "java.util.Iterator";
+    private static final MethodMatchers ITERATOR_METHODS = MethodMatchers.create()
+            .ofSubTypes(ITERATOR)
+            .names("hasNext", "next")
+            .withAnyParameters()
+            .build();
+    private static final String ENUMERATION = "java.util.Enumeration";
+    private static final MethodMatchers ENUMERATION_METHODS = MethodMatchers.create()
+            .ofSubTypes(ENUMERATION)
+            .names("hasMoreElements", "nextElement")
+            .withAnyParameters()
+            .build();
+
+    
     private static final Map<String, Collection<Integer>> linesWithIssuesByClass = new HashMap<>();
 
     @Override
@@ -68,19 +83,15 @@ private class MethodInvocationInForStatementVisitor extends BaseTreeVisitor {
 
         @Override
         public void visitMethodInvocation(MethodInvocationTree tree) {
-            if (!lineAlreadyHasThisIssue(tree) && !isIteratorMethod(tree)) {
+            if (!lineAlreadyHasThisIssue(tree) && !isMethodAllowed(tree)) {
                 report(tree);
                 return;
             }
             super.visitMethodInvocation(tree);
         }
 
-        private boolean isIteratorMethod(MethodInvocationTree tree) {
-            boolean isIterator = tree.methodSymbol().owner().type().isSubtypeOf("java.util.Iterator");
-            String methodName = tree.methodSelect().lastToken().text();
-            boolean isMethodNext = methodName.equals("next");
-            boolean isMethodHasNext = methodName.equals("hasNext");
-            return isIterator && (isMethodNext || isMethodHasNext);
+        private boolean isMethodAllowed(MethodInvocationTree tree) {
+            return ITERATOR_METHODS.matches(tree) || ENUMERATION_METHODS.matches(tree);
         }
 
         private boolean lineAlreadyHasThisIssue(Tree tree) {

src/test/files/NoFunctionCallWhenDeclaringForLoop.java

@@ -19,6 +19,10 @@
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Arrays;
+import java.util.Enumeration;
+import java.util.Collections;
+
+
 class NoFunctionCallWhenDeclaringForLoop {
 
     public int getMyValue() {
@@ -94,26 +98,56 @@ public void test7() {
         }
 
         // iterator called in an indirect way is allowed
-        for (final OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine.iterator()); otherClass.iterator.hasNext(); jour = otherClass.iterator.next()) {
+        for (final OtherClassWrapper otherClass = new OtherClassWrapper(joursSemaine.iterator()); otherClass.iterator.hasNext(); jour = otherClass.iterator.next()) {
             System.out.println(jour);
         }
+
         // but using a method that returns an iterator causes an issue
-        for (final OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine.iterator()); otherClass.getIterator().hasNext(); jour = otherClass.getIterator().next()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
+        for (final OtherClassWrapper otherClass = new OtherClassWrapper(joursSemaine.iterator()); otherClass.getIterator().hasNext(); jour = otherClass.getIterator().next()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
+            System.out.println(jour);
+        }
+
+    }
+
+    // compliant, enumeration is allowed
+    public void test8() {
+        final List<String> joursSemaine = Arrays.asList("Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche");
+
+        String jour = null;
+        for (final Enumeration<String> enumeration = Collections.enumeration(joursSemaine); enumeration.hasMoreElements(); jour = enumeration.nextElement()) {
+            System.out.println(jour);
+        }
+
+        // enumeration called in an indirect way is allowed
+        for(final OtherClassWrapper otherClass = new OtherClassWrapper(Collections.enumeration(joursSemaine)); otherClass.enumeration.hasMoreElements(); jour = otherClass.enumeration.nextElement()) {
+            System.out.println(jour);
+        }
+
+        // but using a method that returns an enumeration causes an issue
+        for(final OtherClassWrapper otherClass = new OtherClassWrapper(Collections.enumeration(joursSemaine)); otherClass.getEnumeration().hasMoreElements(); jour = otherClass.getEnumeration().nextElement()) {  // Noncompliant {{Do not call a function when declaring a for-type loop}}
             System.out.println(jour);
         }
 
     }
 
 }
 
-class OtherClassWithIterator {
+class OtherClassWrapper {
     public final Iterator<String> iterator;
+    public final Enumeration<String> enumeration;
 
-    public OtherClassWithIterator(Iterator<String> iterator){
+    public OtherClassWrapper(Iterator<String> iterator){
         this.iterator = iterator;
     }
+    public OtherClassWrapper(Enumeration<String> enumeration){
+        this.enumeration = enumeration;
+    }
 
     public Iterator<String> getIterator(){
         return iterator;
     }
+
+    public Enumeration<String> getEnumeration(){
+        return enumeration;
+    }
 }