From 68bf749e5add39615ab5b99c8b55da78703953b6 Mon Sep 17 00:00:00 2001 From: Denielig Date: Wed, 29 May 2024 12:58:44 +0200 Subject: [PATCH 1/2] fix: Improve EC69 chore: Add false positive test case fix: :bug: removed false positives fix: :bulb: fix comments and remove logs docs: :memo: remove comment and useless imports Fix #21 --- .../NoFunctionCallWhenDeclaringForLoop.java | 12 +++-- .../NoFunctionCallWhenDeclaringForLoop.java | 47 ++++++++++++++++++- 2 files changed, 54 insertions(+), 5 deletions(-) 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 From 025f800798ded64df2db202cbe1396aa33fc0b00 Mon Sep 17 00:00:00 2001 From: Luc Fouin Date: Thu, 30 May 2024 11:57:39 +0200 Subject: [PATCH 2/2] =?UTF-8?q?docs:=20=F0=9F=93=9D=20Updating=20CHANGELOG?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce62a8..d221c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ 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. + ### Deleted ## [1.6.1] - 2024-05-15