Skip to content

Commit f19e85f

Browse files
dedece35dedece35
authored
Merge pull request #89 from green-code-initiative/GCI82
GCI82 - add rule + refacto ITs + small corrections UT
2 parents 7c977d3 + 38952d7 commit f19e85f

File tree

15 files changed

+410
-157
lines changed

15 files changed

+410
-157
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12-
- [#88](https://github.com/green-code-initiative/creedengo-java/pull/88) Add new Java rule GCI94 (Use orElseGet instead of orElse)
12+
- [#88](https://github.com/green-code-initiative/creedengo-java/pull/88) Add new Java rule GCI94 - Use orElseGet instead of orElse
13+
- [#89](https://github.com/green-code-initiative/creedengo-java/pull/89) Add new Java rule GCI82 - Make non reassigned variables constants
1314

1415
### Changed
1516

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
<google.re2j>1.7</google.re2j>
7373

7474
<!-- Version of creedengo rules specifications implemented by this plugin -->
75-
<creedengo-rules-specifications.version>2.0.0</creedengo-rules-specifications.version>
75+
<creedengo-rules-specifications.version>2.1.0</creedengo-rules-specifications.version>
7676

7777
<!-- URL of the Maven repository where sonarqube will be downloaded -->
7878
<test-it.orchestrator.artifactory.url>https://repo1.maven.org/maven2</test-it.orchestrator.artifactory.url>

‎src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BaseIT.java‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/LaunchSonarqubeAndBuildProject.java‎ renamed to ‎src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BuildProjectEngine.java‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,10 @@
2323
import com.sonar.orchestrator.locator.Location;
2424
import com.sonar.orchestrator.locator.MavenLocation;
2525
import com.sonar.orchestrator.locator.URLLocation;
26-
import lombok.Builder;
2726
import lombok.Getter;
2827
import org.greencodeinitiative.creedengo.java.integration.tests.profile.ProfileBackup;
2928
import org.junit.jupiter.api.AfterAll;
3029
import org.junit.jupiter.api.BeforeAll;
31-
import org.junit.jupiter.api.Test;
32-
import org.sonarqube.ws.Common;
3330
import org.sonarqube.ws.Issues;
3431
import org.sonarqube.ws.Measures;
3532
import org.sonarqube.ws.client.HttpConnector;
@@ -44,12 +41,10 @@
4441
import static java.util.stream.Collectors.toList;
4542
import static java.util.stream.Collectors.toMap;
4643
import static org.assertj.core.api.Assertions.assertThat;
47-
import static org.sonarqube.ws.Common.RuleType.CODE_SMELL;
48-
import static org.sonarqube.ws.Common.Severity.MINOR;
4944

50-
abstract class LaunchSonarqubeAndBuildProject {
45+
abstract class BuildProjectEngine {
5146

52-
private static final System.Logger LOGGER = System.getLogger(LaunchSonarqubeAndBuildProject.class.getName());
47+
private static final System.Logger LOGGER = System.getLogger(BuildProjectEngine.class.getName());
5348

5449
protected static OrchestratorExtension orchestrator;
5550
protected static List<ProjectToAnalyze> analyzedProjects;
@@ -204,10 +199,10 @@ private static Stream<String> splitAndTrim(String value, String regexSeparator)
204199

205200
private static Set<Location> additionalPluginsToInstall() {
206201
Set<Location> plugins = commaSeparatedValues(systemProperty("test-it.plugins"))
207-
.map(LaunchSonarqubeAndBuildProject::toPluginLocation)
202+
.map(BuildProjectEngine::toPluginLocation)
208203
.collect(Collectors.toSet());
209204
commaSeparatedValues(System.getProperty("test-it.additional-plugins", ""))
210-
.map(LaunchSonarqubeAndBuildProject::toPluginLocation)
205+
.map(BuildProjectEngine::toPluginLocation)
211206
.forEach(plugins::add);
212207
return plugins;
213208
}
@@ -419,34 +414,4 @@ private void associateProjectToQualityProfile(Server server, Map<String, String>
419414
}
420415
}
421416

422-
@Getter
423-
@Builder
424-
protected static class IssueDetails {
425-
private String rule;
426-
private String message;
427-
private int line;
428-
private int startLine;
429-
private int endLine;
430-
private int startOffset;
431-
private int endOffset;
432-
private Common.RuleType type;
433-
private Common.Severity severity;
434-
private String debt;
435-
private String effort;
436-
}
437-
438-
protected void verifyIssue(Issues.Issue issueToCheck, IssueDetails issueSource) {
439-
assertThat(issueToCheck.getRule()).isEqualTo(issueSource.getRule());
440-
assertThat(issueToCheck.getMessage()).isEqualTo(issueSource.getMessage());
441-
assertThat(issueToCheck.getLine()).isEqualTo(issueSource.getLine());
442-
assertThat(issueToCheck.getTextRange().getStartLine()).isEqualTo(issueSource.getStartLine());
443-
assertThat(issueToCheck.getTextRange().getEndLine()).isEqualTo(issueSource.getEndLine());
444-
assertThat(issueToCheck.getTextRange().getStartOffset()).isEqualTo(issueSource.getStartOffset());
445-
assertThat(issueToCheck.getTextRange().getEndOffset()).isEqualTo(issueSource.getEndOffset());
446-
assertThat(issueToCheck.getSeverity()).isEqualTo(issueSource.getSeverity());
447-
assertThat(issueToCheck.getType()).isEqualTo(issueSource.getType());
448-
assertThat(issueToCheck.getDebt()).isEqualTo(issueSource.getDebt());
449-
assertThat(issueToCheck.getEffort()).isEqualTo(issueSource.getEffort());
450-
}
451-
452417
}

‎src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI69IT.java‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

‎src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI94IT.java‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package org.greencodeinitiative.creedengo.java.integration.tests;
2+
3+
import org.assertj.core.groups.Tuple;
4+
import org.junit.jupiter.api.Test;
5+
import org.sonarqube.ws.Issues;
6+
import org.sonarqube.ws.Measures;
7+
8+
import java.util.List;
9+
import java.util.Map;
10+
11+
import static java.util.Optional.ofNullable;
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
import static org.sonarqube.ws.Common.RuleType.CODE_SMELL;
14+
import static org.sonarqube.ws.Common.Severity.MINOR;
15+
16+
class GCIRulesIT extends BuildProjectEngine {
17+
18+
@Test
19+
void testMeasuresAndIssues() {
20+
String projectKey = analyzedProjects.get(0).getProjectKey();
21+
22+
Map<String, Measures.Measure> measures = getMeasures(projectKey);
23+
24+
assertThat(ofNullable(measures.get("code_smells")).map(Measures.Measure::getValue).map(Integer::parseInt).orElse(0))
25+
.isGreaterThan(1);
26+
27+
List<Issues.Issue> projectIssues = issuesForComponent(projectKey);
28+
assertThat(projectIssues).isNotEmpty();
29+
30+
}
31+
32+
@Test
33+
void testGCI69() {
34+
String projectKey = analyzedProjects.get(0).getProjectKey();
35+
36+
List<Issues.Issue> issues = issuesForFile(projectKey,
37+
"src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java");
38+
39+
assertThat(issues)
40+
.hasSize(1)
41+
.extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
42+
"textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
43+
.containsExactly(
44+
Tuple.tuple("creedengo-java:GCI69", "Do not call a function when declaring a for-type loop",
45+
18, 18, 18, 15, 27, MINOR, CODE_SMELL, "5min", "5min")
46+
);
47+
48+
}
49+
50+
@Test
51+
void testGCI82() {
52+
String projectKey = analyzedProjects.get(0).getProjectKey();
53+
54+
List<Issues.Issue> issues = issuesForFile(projectKey,
55+
"src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java");
56+
57+
assertThat(issues)
58+
.hasSize(4)
59+
.extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
60+
"textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
61+
.contains(
62+
Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
63+
7, 7, 7, 4, 67, MINOR, CODE_SMELL, "5min", "5min"),
64+
Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
65+
12, 12, 12, 4, 56, MINOR, CODE_SMELL, "5min", "5min"),
66+
Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
67+
13, 13, 13, 4, 50, MINOR, CODE_SMELL, "5min", "5min"),
68+
Tuple.tuple("creedengo-java:GCI82", "The variable is never reassigned and can be 'final'",
69+
45, 45, 45, 8, 25, MINOR, CODE_SMELL, "5min", "5min")
70+
);
71+
72+
}
73+
74+
@Test
75+
void testGCI94() {
76+
String projectKey = analyzedProjects.get(0).getProjectKey();
77+
78+
List<Issues.Issue> issues = issuesForFile(projectKey,
79+
"src/main/java/org/greencodeinitiative/creedengo/java/checks/UseOptionalOrElseGetVsOrElse.java");
80+
81+
assertThat(issues)
82+
.hasSize(1)
83+
.extracting("rule", "message", "line", "textRange.startLine", "textRange.endLine",
84+
"textRange.startOffset", "textRange.endOffset", "severity", "type", "debt", "effort")
85+
.containsExactly(
86+
Tuple.tuple(
87+
"creedengo-java:GCI94", "Use optional orElseGet instead of orElse.",
88+
25, 25, 25, 38, 69, MINOR, CODE_SMELL, "1min", "1min")
89+
);
90+
91+
}
92+
93+
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@ class AvoidGettingSizeCollectionInForLoopIgnored {
1010
}
1111

1212
public void badForLoop() {
13-
List<Integer> numberList = new ArrayList<Integer>();
13+
final List<Integer> numberList = new ArrayList<Integer>();
1414
numberList.add(10);
1515
numberList.add(20);
1616

17-
Iterator it = numberList.iterator();
17+
final Iterator<Integer> it = numberList.iterator();
1818
for (; it.hasNext(); ) { // Ignored => compliant
19-
it.next();
20-
System.out.println("numberList.size()");
19+
System.out.println(it.next());
2120
}
2221
}
22+
23+
2324
}

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import java.util.logging.Logger;
2+
3+
public class MakeNonReassignedVariablesConstants {
4+
5+
private final Logger logger = Logger.getLogger(""); // Compliant
6+
7+
private Object myNonFinalAndNotReassignedObject = new Object(); // Noncompliant {{The variable is never reassigned and can be 'final'}}
8+
private Object myNonFinalAndReassignedObject = new Object(); // Compliant
9+
private final Object myFinalAndNotReassignedObject = new Object(); // Compliant
10+
11+
private static final String CONSTANT = "toto"; // Compliant
12+
private String varDefinedInClassNotReassigned = "0"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
13+
private String varDefinedInClassNotUsed = "0"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
14+
private String varDefinedInClassReassigned = "0"; // Compliant
15+
private String varDefinedInConstructorReassigned = "1"; // Compliant
16+
17+
public MakeNonReassignedVariablesConstants() {
18+
varDefinedInConstructorReassigned = "3";
19+
logger.info(varDefinedInConstructorReassigned);
20+
}
21+
22+
void localVariableReassigned() {
23+
String y1 = "10"; // Compliant
24+
final String PI = "3.14159"; // Compliant
25+
26+
y1 = "titi";
27+
28+
logger.info(y1);
29+
logger.info(PI);
30+
}
31+
32+
void localVariableIncrement() {
33+
String y2 = "10"; // Compliant
34+
y2 += "titi";
35+
logger.info(y2);
36+
}
37+
38+
void localIntVariableIncrement() {
39+
int y3 = 10; // Compliant
40+
++y3;
41+
logger.info(y3+"");
42+
}
43+
44+
void localVariableNotReassigned() {
45+
String y4 = "10"; // Noncompliant {{The variable is never reassigned and can be 'final'}}
46+
final String PI2 = "3.14159"; // Compliant
47+
48+
logger.info(y4);
49+
logger.info(PI2);
50+
}
51+
52+
void classVariableReassigned() {
53+
varDefinedInClassReassigned = "1";
54+
55+
logger.info(varDefinedInClassReassigned);
56+
logger.info(varDefinedInClassNotReassigned);
57+
logger.info(CONSTANT);
58+
}
59+
60+
void classVariableReassignedBis() {
61+
varDefinedInClassReassigned = "2"; // method to avoid sonarqube error asking for moving class variable "varDefinedInClassReassigned" to local variable method
62+
myNonFinalAndReassignedObject = new Object();
63+
64+
logger.info(varDefinedInClassReassigned);
65+
logger.info(myNonFinalAndReassignedObject.toString());
66+
logger.info(myFinalAndNotReassignedObject.toString());
67+
}
68+
69+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public class JavaCheckRegistrar implements CheckRegistrar {
4949
AvoidSetConstantInBatchUpdate.class,
5050
FreeResourcesOfAutoCloseableInterface.class,
5151
AvoidMultipleIfElseStatement.class,
52-
UseOptionalOrElseGetVsOrElse.class
52+
UseOptionalOrElseGetVsOrElse.class,
53+
MakeNonReassignedVariablesConstants.class
5354
);
5455

5556
/**

0 commit comments

Comments
 (0)