Skip to content

Commit

Permalink
improve GCI89 - get code from PR 45
Browse files Browse the repository at this point in the history
  • Loading branch information
dedece35 committed Jan 5, 2025
1 parent f42d73e commit 670e15b
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 67 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- upgrade some libraries versions
- improve Integration Tests system to be more flexible (add new IT for each rule)
- [#21](https://github.com/green-code-initiative/ecoCode-java/issues/21) Improvement: some method calls are legitimate in a for loop expression

### Deleted

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,46 @@ void testMeasuresAndIssues() {

}

@Test
void testGCI3() {
String projectKey = analyzedProjects.get(0).getProjectKey();

List<Issues.Issue> issues = issuesForFile(projectKey,
"src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopBad.java");

assertThat(issues)
.hasSize(2)
.extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
"textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
.containsExactly(
Tuple.tuple("creedengo-java:GCI3", "Avoid getting the size of the collection in the loop",
13, 13, 13, 28, 45, MINOR, CODE_SMELL, "5min", "5min"),
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
13, 13, 13, 28, 45, MINOR, CODE_SMELL, "5min", "5min")
);

}

@Test
void testGCI69() {
String projectKey = analyzedProjects.get(0).getProjectKey();

List<Issues.Issue> issues = issuesForFile(projectKey,
"src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java");
"src/main/java/org/greencodeinitiative/creedengo/java/checks/NoFunctionCallWhenDeclaringForLoop.java");

assertThat(issues)
.hasSize(1)
.hasSize(4)
.extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
"textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
.containsExactly(
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
18, 18, 18, 15, 27, MINOR, CODE_SMELL, "5min", "5min")
58, 58, 58, 28, 40, MINOR, CODE_SMELL, "5min", "5min"),
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
66, 66, 66, 34, 46, MINOR, CODE_SMELL, "5min", "5min"),
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
74, 74, 74, 39, 51, MINOR, CODE_SMELL, "5min", "5min"),
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
101, 101, 101, 108, 132, MINOR, CODE_SMELL, "5min", "5min")
);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
import java.util.List;

class AvoidGettingSizeCollectionInForLoopBad {
AvoidGettingSizeCollectionInForLoopBad() {

}

public void badForLoop() {
List<Integer> numberList = new ArrayList<Integer>();
final List<Integer> numberList = new ArrayList<Integer>();
numberList.add(10);
numberList.add(20);

for (int i = 0; i < numberList.size(); i++) { // Noncompliant {{Avoid getting the size of the collection in the loop}}
for (int i = 0; i < numberList.size(); ++i) { // Noncompliant
System.out.println("numberList.size()");
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,119 @@
package org.greencodeinitiative.creedengo.java.checks;

package org.greencodeinitiative.creedengo.java.integration.tests;/*
* creedengo - Java language - Provides rules to reduce the environmental footprint of your Java programs
* Copyright © 2024 Green Code Initiative (https://green-code-initiative.org/)
*
* 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/>.
*/
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Arrays;
class NoFunctionCallWhenDeclaringForLoop {
NoFunctionCallWhenDeclaringForLoop(NoFunctionCallWhenDeclaringForLoop mc) {
}

public int getMyValue() {
return 6;
}

public int incrementeMyValue(int i) {
public int incrementeMyValue(final int i) {
return i + 100;
}

public void test1() {
for (int i = 0; i < 20; i++) {
for (int i = 0; i < 20; ++i) {
System.out.println(i);
boolean b = getMyValue() > 6;
final boolean b = getMyValue() > 6;
System.out.println(b);
}
}

public void test2() {
String[] cars = {"Volvo", "BMW", "Ford", "Mazda"};
for (String i : cars) {
final String[] cars = {"Volvo", "BMW", "Ford", "Mazda"};
for (final String i : cars) {
System.out.println(i);
}

}

// 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;
final boolean b = getMyValue() > 6;
System.out.println(b);
}
}

public void test4() {
for (int i = 0; i < getMyValue(); i++) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
for (int i = 0; i < getMyValue(); ++i) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
System.out.println(i);
boolean b = getMyValue() > 6;
final boolean b = getMyValue() > 6;
System.out.println(b);
}
}

public void test5() {
for (int i = 0; i < getMyValue(); incrementeMyValue(i)) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
for (final int i = 0; i < getMyValue(); incrementeMyValue(i)) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
System.out.println(i);
boolean b = getMyValue() > 6;
final boolean b = getMyValue() > 6;
System.out.println(b);
}
}

public void test6() {
for (int i = getMyValue(); i < getMyValue(); i++) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
for (int i = getMyValue(); i < getMyValue(); ++i) { // Noncompliant {{Do not call a function when declaring a for-type loop}}
System.out.println(i);
boolean b = getMyValue() > 6;
final boolean b = getMyValue() > 6;
System.out.println(b);
}
}

// compliant, iterators are allowed to be called in a for loop
public void test7() {
final List<String> joursSemaine = Arrays.asList("Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche");

String jour = null;
// iterator is allowed
for (final Iterator<String> iterator = joursSemaine.iterator(); iterator.hasNext(); jour = iterator.next()) {
System.out.println(jour);
}

// subclass of iterator is allowed
for (final ListIterator<String> iterator = joursSemaine.listIterator(); iterator.hasNext(); jour = iterator.next()) {
System.out.println(jour);
}

// iterator called in an indirect way is allowed
for (final OtherClassWithIterator otherClass = new OtherClassWithIterator(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}}
System.out.println(jour);
}

}

}

class OtherClassWithIterator {
public final Iterator<String> iterator;

public OtherClassWithIterator(Iterator<String> iterator){
this.iterator = iterator;
}

public Iterator<String> getIterator(){
return iterator;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,30 @@ public void visitNode(Tree tree) {
if (null != condition) {
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);
Expand Down
25 changes: 2 additions & 23 deletions src/test/files/AvoidGettingSizeCollectionInForLoopBad.java
Original file line number Diff line number Diff line change
@@ -1,37 +1,16 @@
/*
* creedengo - Java language - Provides rules to reduce the environmental footprint of your Java programs
* Copyright © 2024 Green Code Initiative (https://green-code-initiative.org/)
*
* 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 org.greencodeinitiative.creedengo.java.checks;

import java.util.Collection;
import java.util.ArrayList;
import java.util.List;

class AvoidGettingSizeCollectionInForLoopBad {
AvoidGettingSizeCollectionInForLoopBad() {

}

public void badForLoop() {
List<Integer> numberList = new ArrayList<Integer>();
final List<Integer> numberList = new ArrayList<Integer>();
numberList.add(10);
numberList.add(20);

for (int i = 0; i < numberList.size(); i++) { // Noncompliant {{Avoid getting the size of the collection in the loop}}
for (int i = 0; i < numberList.size(); ++i) { // Noncompliant {{Avoid getting the size of the collection in the loop}}
System.out.println("numberList.size()");
}
}
Expand Down
Loading

0 comments on commit 670e15b

Please sign in to comment.