GCI82 - add rule + refacto ITs + small corrections UT

CHANGELOG.md

@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- [#88](https://github.com/green-code-initiative/creedengo-java/pull/88) Add new Java rule GCI94 (Use orElseGet instead of orElse)
+- [#88](https://github.com/green-code-initiative/creedengo-java/pull/88) Add new Java rule GCI94 - Use orElseGet instead of orElse
+- [#89](https://github.com/green-code-initiative/creedengo-java/pull/89) Add new Java rule GCI82 - Make non reassigned variables constants
 
 ### Changed
 

pom.xml

@@ -72,7 +72,7 @@
         <google.re2j>1.7</google.re2j>
 
         <!-- Version of creedengo rules specifications implemented by this plugin -->
-        <creedengo-rules-specifications.version>2.0.0</creedengo-rules-specifications.version>
+        <creedengo-rules-specifications.version>2.1.0</creedengo-rules-specifications.version>
 
         <!-- URL of the Maven repository where sonarqube will be downloaded -->
         <test-it.orchestrator.artifactory.url>https://repo1.maven.org/maven2</test-it.orchestrator.artifactory.url>

src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/LaunchSonarqubeAndBuildProject.java → src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BuildProjectEngine.java

@@ -23,13 +23,10 @@
 import com.sonar.orchestrator.locator.Location;
 import com.sonar.orchestrator.locator.MavenLocation;
 import com.sonar.orchestrator.locator.URLLocation;
-import lombok.Builder;
 import lombok.Getter;
 import org.greencodeinitiative.creedengo.java.integration.tests.profile.ProfileBackup;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
-import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Issues;
 import org.sonarqube.ws.Measures;
 import org.sonarqube.ws.client.HttpConnector;
@@ -44,12 +41,10 @@
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonarqube.ws.Common.RuleType.CODE_SMELL;
-import static org.sonarqube.ws.Common.Severity.MINOR;
 
-abstract class LaunchSonarqubeAndBuildProject {
+abstract class BuildProjectEngine {
 
-	private static final System.Logger LOGGER = System.getLogger(LaunchSonarqubeAndBuildProject.class.getName());
+	private static final System.Logger LOGGER = System.getLogger(BuildProjectEngine.class.getName());
 
 	protected static OrchestratorExtension orchestrator;
 	protected static List<ProjectToAnalyze> analyzedProjects;
@@ -204,10 +199,10 @@ private static Stream<String> splitAndTrim(String value, String regexSeparator)
 
 	private static Set<Location> additionalPluginsToInstall() {
 		Set<Location> plugins = commaSeparatedValues(systemProperty("test-it.plugins"))
-				.map(LaunchSonarqubeAndBuildProject::toPluginLocation)
+				.map(BuildProjectEngine::toPluginLocation)
 				.collect(Collectors.toSet());
 		commaSeparatedValues(System.getProperty("test-it.additional-plugins", ""))
-				.map(LaunchSonarqubeAndBuildProject::toPluginLocation)
+				.map(BuildProjectEngine::toPluginLocation)
 				.forEach(plugins::add);
 		return plugins;
 	}
@@ -419,34 +414,4 @@ private void associateProjectToQualityProfile(Server server, Map<String, String>
 		}
 	}
 
-	@Getter
-	@Builder
-	protected static class IssueDetails {
-		private String rule;
-		private String message;
-		private int line;
-		private int startLine;
-		private int endLine;
-		private int startOffset;
-		private int endOffset;
-		private Common.RuleType type;
-		private Common.Severity severity;
-		private String debt;
-		private String effort;
-	}
-
-	protected void verifyIssue(Issues.Issue issueToCheck, IssueDetails issueSource) {
-		assertThat(issueToCheck.getRule()).isEqualTo(issueSource.getRule());
-		assertThat(issueToCheck.getMessage()).isEqualTo(issueSource.getMessage());
-		assertThat(issueToCheck.getLine()).isEqualTo(issueSource.getLine());
-		assertThat(issueToCheck.getTextRange().getStartLine()).isEqualTo(issueSource.getStartLine());
-		assertThat(issueToCheck.getTextRange().getEndLine()).isEqualTo(issueSource.getEndLine());
-		assertThat(issueToCheck.getTextRange().getStartOffset()).isEqualTo(issueSource.getStartOffset());
-		assertThat(issueToCheck.getTextRange().getEndOffset()).isEqualTo(issueSource.getEndOffset());
-		assertThat(issueToCheck.getSeverity()).isEqualTo(issueSource.getSeverity());
-		assertThat(issueToCheck.getType()).isEqualTo(issueSource.getType());
-		assertThat(issueToCheck.getDebt()).isEqualTo(issueSource.getDebt());
-		assertThat(issueToCheck.getEffort()).isEqualTo(issueSource.getEffort());
-	}
-
 }

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

@@ -0,0 +1,93 @@
+package org.greencodeinitiative.creedengo.java.integration.tests;
+
+import org.assertj.core.groups.Tuple;
+import org.junit.jupiter.api.Test;
+import org.sonarqube.ws.Issues;
+import org.sonarqube.ws.Measures;
+
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Optional.ofNullable;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonarqube.ws.Common.RuleType.CODE_SMELL;
+import static org.sonarqube.ws.Common.Severity.MINOR;
+
+class GCIRulesIT extends BuildProjectEngine {
+
+    @Test
+    void testMeasuresAndIssues() {
+        String projectKey = analyzedProjects.get(0).getProjectKey();
+
+        Map<String, Measures.Measure> measures = getMeasures(projectKey);
+
+        assertThat(ofNullable(measures.get("code_smells")).map(Measures.Measure::getValue).map(Integer::parseInt).orElse(0))
+                .isGreaterThan(1);
+
+        List<Issues.Issue> projectIssues = issuesForComponent(projectKey);
+        assertThat(projectIssues).isNotEmpty();
+
+    }
+
+    @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");
+
+        assertThat(issues)
+            .hasSize(1)
+            .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")
+            );
+
+    }
+
+    @Test
+    void testGCI82() {
+        String projectKey = analyzedProjects.get(0).getProjectKey();
+
+        List<Issues.Issue> issues = issuesForFile(projectKey,
+                "src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java");
+
+        assertThat(issues)
+                .hasSize(4)
+                .extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
+                        "textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
+                .contains(
+                        Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
+                                7, 7, 7, 4, 67, MINOR, CODE_SMELL, "5min", "5min"),
+                        Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
+                                12, 12, 12, 4, 56, MINOR, CODE_SMELL, "5min", "5min"),
+                        Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
+                                13, 13, 13, 4, 50, MINOR, CODE_SMELL, "5min", "5min"),
+                        Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
+                                45, 45, 45, 8, 25, MINOR, CODE_SMELL, "5min", "5min")
+                );
+
+    }
+
+    @Test
+    void testGCI94() {
+        String projectKey = analyzedProjects.get(0).getProjectKey();
+
+        List<Issues.Issue> issues = issuesForFile(projectKey,
+                "src/main/java/org/greencodeinitiative/creedengo/java/checks/UseOptionalOrElseGetVsOrElse.java");
+
+        assertThat(issues)
+                .hasSize(1)
+                .extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
+                        "textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
+                .containsExactly(
+                        Tuple.tuple(
+                                "creedengo-java:GCI94", "Use optional orElseGet instead of orElse.",
+                                25, 25, 25, 38, 69, MINOR, CODE_SMELL, "1min", "1min")
+                );
+
+    }
+
+}

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

@@ -10,14 +10,15 @@ class AvoidGettingSizeCollectionInForLoopIgnored {
     }
 
     public void badForLoop() {
-        List<Integer> numberList = new ArrayList<Integer>();
+        final List<Integer> numberList = new ArrayList<Integer>();
         numberList.add(10);
         numberList.add(20);
 
-        Iterator it = numberList.iterator();
+        final Iterator<Integer> it = numberList.iterator();
         for (; it.hasNext(); ) { // Ignored => compliant
-            it.next();
-            System.out.println("numberList.size()");
+            System.out.println(it.next());
         }
     }
+
+
 }

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

@@ -0,0 +1,69 @@
+import java.util.logging.Logger;
+
+public class MakeNonReassignedVariablesConstants {
+
+    private final Logger logger = Logger.getLogger(""); // Compliant
+
+    private Object myNonFinalAndNotReassignedObject = new Object(); // Noncompliant {{The variable is never reassigned and can be 'final'}}
+    private Object myNonFinalAndReassignedObject = new Object(); // Compliant
+    private final Object myFinalAndNotReassignedObject = new Object(); // Compliant
+
+    private static final String CONSTANT = "toto";  // Compliant
+    private String varDefinedInClassNotReassigned = "0"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
+    private String varDefinedInClassNotUsed = "0"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
+    private String varDefinedInClassReassigned = "0"; // Compliant
+    private String varDefinedInConstructorReassigned = "1"; // Compliant
+
+    public MakeNonReassignedVariablesConstants() {
+        varDefinedInConstructorReassigned = "3";
+        logger.info(varDefinedInConstructorReassigned);
+    }
+
+    void localVariableReassigned() {
+        String y1 = "10"; // Compliant
+        final String PI = "3.14159"; // Compliant
+
+        y1 = "titi";
+
+        logger.info(y1);
+        logger.info(PI);
+    }
+
+    void localVariableIncrement() {
+        String y2 = "10"; // Compliant
+        y2 += "titi";
+        logger.info(y2);
+    }
+
+    void localIntVariableIncrement() {
+        int y3 = 10; // Compliant
+        ++y3;
+        logger.info(y3+"");
+    }
+
+    void localVariableNotReassigned() {
+        String y4 = "10"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
+        final String PI2 = "3.14159"; // Compliant
+
+        logger.info(y4);
+        logger.info(PI2);
+    }
+
+    void classVariableReassigned() {
+        varDefinedInClassReassigned = "1";
+
+        logger.info(varDefinedInClassReassigned);
+        logger.info(varDefinedInClassNotReassigned);
+        logger.info(CONSTANT);
+    }
+
+    void classVariableReassignedBis() {
+        varDefinedInClassReassigned = "2"; // method to avoid sonarqube error asking for moving class variable "varDefinedInClassReassigned" to local variable method
+        myNonFinalAndReassignedObject = new Object();
+
+        logger.info(varDefinedInClassReassigned);
+        logger.info(myNonFinalAndReassignedObject.toString());
+        logger.info(myFinalAndNotReassignedObject.toString());
+    }
+
+}

src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java

@@ -49,7 +49,8 @@ public class JavaCheckRegistrar implements CheckRegistrar {
             AvoidSetConstantInBatchUpdate.class,
             FreeResourcesOfAutoCloseableInterface.class,
             AvoidMultipleIfElseStatement.class,
-            UseOptionalOrElseGetVsOrElse.class
+            UseOptionalOrElseGetVsOrElse.class,
+            MakeNonReassignedVariablesConstants.class
     );
 
     /**