diff --git a/CHANGELOG.md b/CHANGELOG.md index 467e711..a750593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- [#21](https://github.com/green-code-initiative/ecoCode-java/issues/21) Improvement: some method calls are legitimate in a for loop expression. - [#49](https://github.com/green-code-initiative/ecoCode-java/pull/49) Add test to ensure all Rules are registered - [#336](https://github.com/green-code-initiative/ecoCode/issues/336) [Adds Maven Wrapper](https://github.com/green-code-initiative/ecoCode-java/pull/67) diff --git a/src/main/java/fr/greencodeinitiative/java/checks/NoFunctionCallWhenDeclaringForLoop.java b/src/main/java/fr/greencodeinitiative/java/checks/NoFunctionCallWhenDeclaringForLoop.java index d4088bd..188e803 100644 --- a/src/main/java/fr/greencodeinitiative/java/checks/NoFunctionCallWhenDeclaringForLoop.java +++ b/src/main/java/fr/greencodeinitiative/java/checks/NoFunctionCallWhenDeclaringForLoop.java @@ -59,22 +59,28 @@ public void visitNode(Tree tree) { method.condition().accept(invocationMethodVisitor); } // update - // initaliser method.update().accept(invocationMethodVisitor); - method.initializer().accept(invocationMethodVisitor); } private class MethodInvocationInForStatementVisitor extends BaseTreeVisitor { @Override public void visitMethodInvocation(MethodInvocationTree tree) { - if (!lineAlreadyHasThisIssue(tree)) { + if (!lineAlreadyHasThisIssue(tree) && !isIteratorMethod(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 lineAlreadyHasThisIssue(Tree tree) { if (tree.firstToken() != null) { final String classname = getFullyQualifiedNameOfClassOf(tree); diff --git a/src/test/files/NoFunctionCallWhenDeclaringForLoop.java b/src/test/files/NoFunctionCallWhenDeclaringForLoop.java index da72a3e..344c42c 100644 --- a/src/test/files/NoFunctionCallWhenDeclaringForLoop.java +++ b/src/test/files/NoFunctionCallWhenDeclaringForLoop.java @@ -15,6 +15,10 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.Arrays; class NoFunctionCallWhenDeclaringForLoop { NoFunctionCallWhenDeclaringForLoop(NoFunctionCallWhenDeclaringForLoop mc) { } @@ -30,7 +34,7 @@ public int incrementeMyValue(int i) { public void test1() { for (int i = 0; i < 20; i++) { System.out.println(i); - boolean b = getMyValue() > 6; + boolean b = this.getMyValue() > 6; } } @@ -42,8 +46,9 @@ public void test2() { } + // compliant, the function is called only once in the initialization so it's not a performance issue public void test3() { - for (int i = getMyValue(); i < 20; i++) { // Noncompliant {{Do not call a function when declaring a for-type loop}} + for (int i = getMyValue(); i < 20; i++) { System.out.println(i); boolean b = getMyValue() > 6; } @@ -70,4 +75,42 @@ public void test6() { } } + // compliant, iterators are allowed to be called in a for loop + public void test7() { + List joursSemaine = Arrays.asList("Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche"); + + String jour; + // iterator is allowed + for (Iterator iterator = joursSemaine.iterator(); iterator.hasNext(); jour = iterator.next()) { + System.out.println(jour); + } + + // subclass of iterator is allowed + for (ListIterator iterator = joursSemaine.listIterator(); iterator.hasNext(); jour = iterator.next()) { + System.out.println(jour); + } + + // iterator called in an indirect way is allowed + for (OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine); otherClass.iterator.hasNext(); jour = otherClass.iterator.next()) { + System.out.println(jour); + } + // but using a method that returns an iterator causes an issue + for (OtherClassWithIterator otherClass = new OtherClassWithIterator(joursSemaine); otherClass.getIterator().hasNext(); jour = otherClass.getIterator().next()) { // Noncompliant {{Do not call a function when declaring a for-type loop}} + System.out.println(jour); + } + + } + +} + +class OtherClassWithIterator { + public Iterator iterator; + + public OtherClassWithIterator(Iterator iterator){ + this.iterator = iterator; + } + + public Iterator getIterator(){ + return iterator; + } } \ No newline at end of file