From a5b7b6a0e55b9eb5383fb3a7caf46798552710ad Mon Sep 17 00:00:00 2001 From: David DE CARVALHO Date: Fri, 3 Jan 2025 15:59:01 +0100 Subject: [PATCH 1/4] rafacto IT files + small UT corrections + add Java rule implem for GCI82 --- CHANGELOG.md | 3 +- pom.xml | 3 +- .../java/integration/tests/BaseIT.java | 28 ---- ...ldProject.java => BuildProjectEngine.java} | 43 +----- .../java/integration/tests/GCI69IT.java | 39 ------ .../java/integration/tests/GCI94IT.java | 39 ------ .../java/integration/tests/GCIRulesIT.java | 93 +++++++++++++ ...GettingSizeCollectionInForLoopIgnored.java | 9 +- .../MakeNonReassignedVariablesConstants.java | 69 ++++++++++ .../creedengo/java/JavaCheckRegistrar.java | 3 +- .../MakeNonReassignedVariablesConstants.java | 129 ++++++++++++++++++ .../creedengo/java/creedengo_way_profile.json | 1 + ...GettingSizeCollectionInForLoopIgnored.java | 9 +- .../MakeNonReassignedVariablesConstants.java | 69 ++++++++++ ...keNonReassignedVariablesConstantsTest.java | 33 +++++ 15 files changed, 413 insertions(+), 157 deletions(-) delete mode 100644 src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BaseIT.java rename src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/{LaunchSonarqubeAndBuildProject.java => BuildProjectEngine.java} (88%) delete mode 100644 src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI69IT.java delete mode 100644 src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI94IT.java create mode 100644 src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java create mode 100644 src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java create mode 100644 src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java create mode 100644 src/test/files/MakeNonReassignedVariablesConstants.java create mode 100644 src/test/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstantsTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b948872..d46e871 100644 --- a/CHANGELOG.md +++ b/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 +- [#88](https://github.com/green-code-initiative/creedengo-java/pull/88) Add new Java rule GCI82 - Make non reassigned variables constants ### Changed diff --git a/pom.xml b/pom.xml index 63e0f17..8940b3f 100644 --- a/pom.xml +++ b/pom.xml @@ -72,7 +72,8 @@ 1.7 - 2.0.0 + main-SNAPSHOT + https://repo1.maven.org/maven2 diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BaseIT.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BaseIT.java deleted file mode 100644 index 0c7e880..0000000 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BaseIT.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.greencodeinitiative.creedengo.java.integration.tests; - -import org.junit.jupiter.api.Test; -import org.sonarqube.ws.Issues; -import org.sonarqube.ws.Measures; - -import java.util.*; - -import static java.util.Optional.ofNullable; -import static org.assertj.core.api.Assertions.assertThat; - -class BaseIT extends LaunchSonarqubeAndBuildProject { - - @Test - void testMeasuresAndIssues() { - String projectKey = analyzedProjects.get(0).getProjectKey(); - - Map measures = getMeasures(projectKey); - - assertThat(ofNullable(measures.get("code_smells")).map(Measures.Measure::getValue).map(Integer::parseInt).orElse(0)) - .isGreaterThan(1); - - List projectIssues = issuesForComponent(projectKey); - assertThat(projectIssues).isNotEmpty(); - - } - -} diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/LaunchSonarqubeAndBuildProject.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BuildProjectEngine.java similarity index 88% rename from src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/LaunchSonarqubeAndBuildProject.java rename to src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/BuildProjectEngine.java index 58ed495..e85b3f0 100644 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/LaunchSonarqubeAndBuildProject.java +++ b/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 analyzedProjects; @@ -204,10 +199,10 @@ private static Stream splitAndTrim(String value, String regexSeparator) private static Set additionalPluginsToInstall() { Set 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 } } - @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()); - } - } diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI69IT.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI69IT.java deleted file mode 100644 index 0784a85..0000000 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI69IT.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.greencodeinitiative.creedengo.java.integration.tests; - -import org.junit.jupiter.api.Test; -import org.sonarqube.ws.Issues; - -import java.util.List; - -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 GCI69IT extends LaunchSonarqubeAndBuildProject { - - @Test - void testGCI69() { - String projectKey = analyzedProjects.get(0).getProjectKey(); - - List issuesForArrayCopyCheck = issuesForFile(projectKey, "src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java"); - - assertThat(issuesForArrayCopyCheck) - .hasSize(1) - .first().satisfies(issue -> verifyIssue(issue, IssueDetails.builder() - .rule("creedengo-java:GCI69") - .message("Do not call a function when declaring a for-type loop") - .line(18) - .startLine(18) - .endLine(18) - .startOffset(15) - .endOffset(27) - .severity(MINOR) - .type(CODE_SMELL) - .debt("5min") - .effort("5min") - .build()) - ); - - } - -} diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI94IT.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI94IT.java deleted file mode 100644 index 6121115..0000000 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCI94IT.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.greencodeinitiative.creedengo.java.integration.tests; - -import org.junit.jupiter.api.Test; -import org.sonarqube.ws.Issues; - -import java.util.List; - -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 GCI94IT extends LaunchSonarqubeAndBuildProject { - - @Test - void testGCI94() { - String projectKey = analyzedProjects.get(0).getProjectKey(); - - List issuesForArrayCopyCheck = issuesForFile(projectKey, "src/main/java/org/greencodeinitiative/creedengo/java/checks/UseOptionalOrElseGetVsOrElse.java"); - - assertThat(issuesForArrayCopyCheck) - .hasSize(1) - .first().satisfies(issue -> verifyIssue(issue, IssueDetails.builder() - .rule("creedengo-java:GCI94") - .message("Use optional orElseGet instead of orElse.") - .line(25) - .startLine(25) - .endLine(25) - .startOffset(38) - .endOffset(69) - .severity(MINOR) - .type(CODE_SMELL) - .debt("1min") - .effort("1min") - .build()) - ); - - } - -} diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java new file mode 100644 index 0000000..8b68d55 --- /dev/null +++ b/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 measures = getMeasures(projectKey); + + assertThat(ofNullable(measures.get("code_smells")).map(Measures.Measure::getValue).map(Integer::parseInt).orElse(0)) + .isGreaterThan(1); + + List projectIssues = issuesForComponent(projectKey); + assertThat(projectIssues).isNotEmpty(); + + } + + @Test + void testGCI69() { + String projectKey = analyzedProjects.get(0).getProjectKey(); + + List 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 = 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 = 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") + ); + + } + +} diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java index 91775b0..3ef6359 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidGettingSizeCollectionInForLoopIgnored.java +++ b/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 numberList = new ArrayList(); + final List numberList = new ArrayList(); numberList.add(10); numberList.add(20); - Iterator it = numberList.iterator(); + final Iterator it = numberList.iterator(); for (; it.hasNext(); ) { // Ignored => compliant - it.next(); - System.out.println("numberList.size()"); + System.out.println(it.next()); } } + + } diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java new file mode 100644 index 0000000..bef640d --- /dev/null +++ b/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()); + } + +} \ No newline at end of file diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java b/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java index a4037e0..791f0ce 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java +++ b/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 ); /** diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java new file mode 100644 index 0000000..6323cc0 --- /dev/null +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java @@ -0,0 +1,129 @@ +package org.greencodeinitiative.creedengo.java.checks; + +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; +import org.sonar.check.Rule; +import org.sonar.java.model.ModifiersUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.*; +import org.sonar.plugins.java.api.tree.Tree.Kind; + +import javax.annotation.Nonnull; +import java.util.List; + +@Rule(key = "GCI82") +public class MakeNonReassignedVariablesConstants extends IssuableSubscriptionVisitor { + + protected static final String MESSAGE_RULE = "The variable is never reassigned and can be 'final'"; + + private static final Logger LOGGER = Loggers.get(MakeNonReassignedVariablesConstants.class); + + @Override + public List nodesToVisit() { + return List.of(Kind.VARIABLE); + } + + @Override + public void visitNode(@Nonnull Tree tree) { + VariableTree variableTree = (VariableTree) tree; + LOGGER.debug("Variable > " + getVariableNameForLogger(variableTree)); + LOGGER.debug(" => isNotFinalAndNotStatic(variableTree) = " + isNotFinalAndNotStatic(variableTree)); + LOGGER.debug(" => usages = " + variableTree.symbol().usages().size()); + LOGGER.debug(" => isNotReassigned = " + isNotReassigned(variableTree)); + + if (isNotFinalAndNotStatic(variableTree) && isNotReassigned(variableTree)) { + reportIssue(tree, MESSAGE_RULE); + } else { + super.visitNode(tree); + } + } + + private static boolean isNotReassigned(VariableTree variableTree) { + return variableTree.symbol() + .usages() + .stream() + .noneMatch(MakeNonReassignedVariablesConstants::parentIsAssignment); + } + + private static boolean parentIsAssignment(Tree tree) { + return parentIsKind(tree, + Kind.ASSIGNMENT, + Kind.MULTIPLY_ASSIGNMENT, + Kind.DIVIDE_ASSIGNMENT, + Kind.REMAINDER_ASSIGNMENT, + Kind.PLUS_ASSIGNMENT, + Kind.MINUS_ASSIGNMENT, + Kind.LEFT_SHIFT_ASSIGNMENT, + Kind.RIGHT_SHIFT_ASSIGNMENT, + Kind.UNSIGNED_RIGHT_SHIFT_ASSIGNMENT, + Kind.AND_ASSIGNMENT, + Kind.XOR_ASSIGNMENT, + Kind.OR_ASSIGNMENT, + Kind.POSTFIX_INCREMENT, + Kind.POSTFIX_DECREMENT, + Kind.PREFIX_INCREMENT, + Kind.PREFIX_DECREMENT + ); + } + + private static boolean parentIsKind(Tree tree, Kind... orKind) { + Tree parent = tree.parent(); + if (parent == null) return false; + + for (Kind k : orKind) { + if (parent.is(k)) return true; + } + + return false; + } + + private static boolean isNotFinalAndNotStatic(VariableTree variableTree) { +// return ModifiersUtils.hasNoneOf(variableTree.modifiers(), Modifier.FINAL, Modifier.STATIC); + return hasNoneOf(variableTree.modifiers(), Modifier.FINAL, Modifier.STATIC); + } + + private static boolean hasNoneOf(ModifiersTree modifiersTree, Modifier... unexpectedModifiers) { + return !hasAnyOf(modifiersTree, unexpectedModifiers); + } + + private static boolean hasAnyOf(ModifiersTree modifiersTree, Modifier... expectedModifiers) { + for(Modifier expectedModifier : expectedModifiers) { + if (hasModifier(modifiersTree, expectedModifier)) { + return true; + } + } + return false; + } + + public static boolean hasModifier(ModifiersTree modifiersTree, Modifier expectedModifier) { + for(ModifierKeywordTree modifierKeywordTree : modifiersTree.modifiers()) { + if (modifierKeywordTree.modifier() == expectedModifier) { + return true; + } + } + + return false; + } + + private String getVariableNameForLogger(VariableTree variableTree) { + String name = variableTree.simpleName().name(); + + if (variableTree.parent() != null) return name; + + if (variableTree.parent().is(Kind.CLASS)) { + ClassTree cTree = (ClassTree) variableTree.parent(); + name += " --- from CLASS '" + cTree.simpleName() + "'"; + } + if (variableTree.parent().is(Kind.BLOCK)) { + BlockTree bTree = (BlockTree) variableTree.parent(); + if (bTree.parent() != null && bTree.parent().is(Kind.METHOD)) { + MethodTree mTree = (MethodTree) bTree.parent(); + name += " --- from METHOD '" + mTree.simpleName() + "'"; + } + } + + return name; + + } + +} diff --git a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json index b5a4b85..8c61366 100644 --- a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json +++ b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json @@ -17,6 +17,7 @@ "GCI77", "GCI78", "GCI79", + "GCI82", "GCI94" ] } diff --git a/src/test/files/AvoidGettingSizeCollectionInForLoopIgnored.java b/src/test/files/AvoidGettingSizeCollectionInForLoopIgnored.java index ced5bf0..c1fa56c 100644 --- a/src/test/files/AvoidGettingSizeCollectionInForLoopIgnored.java +++ b/src/test/files/AvoidGettingSizeCollectionInForLoopIgnored.java @@ -21,20 +21,19 @@ import java.util.ArrayList; import java.util.List; -class AvoidGettingSizeCollectionInForLoopBad { +class GCI69AvoidGettingSizeCollectionInForLoopBad { AvoidGettingSizeCollectionInForLoopBad() { } public void badForLoop() { - List numberList = new ArrayList(); + final List numberList = new ArrayList(); numberList.add(10); numberList.add(20); - Iterator it = numberList.iterator(); + final Iterator it = numberList.iterator(); for (; it.hasNext(); ) { // Ignored => compliant - it.next(); - System.out.println("numberList.size()"); + System.out.println(it.next()); } } } diff --git a/src/test/files/MakeNonReassignedVariablesConstants.java b/src/test/files/MakeNonReassignedVariablesConstants.java new file mode 100644 index 0000000..a8e4039 --- /dev/null +++ b/src/test/files/MakeNonReassignedVariablesConstants.java @@ -0,0 +1,69 @@ +import java.util.logging.Logger; + +public class GCI82MakeNonReassignedVariablesConstants { + + 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 GCI82MakeNonReassignedVariablesConstants() { + 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()); + } + +} \ No newline at end of file diff --git a/src/test/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstantsTest.java b/src/test/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstantsTest.java new file mode 100644 index 0000000..837a632 --- /dev/null +++ b/src/test/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstantsTest.java @@ -0,0 +1,33 @@ +/* + * 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 . + */ +package org.greencodeinitiative.creedengo.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +class MakeNonReassignedVariablesConstantsTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile("src/test/files/MakeNonReassignedVariablesConstants.java") + .withCheck(new MakeNonReassignedVariablesConstants()) + .verifyIssues(); + } + +} From adc70f9a2dc15926f17b9fef909f261e2ead8849 Mon Sep 17 00:00:00 2001 From: David DE CARVALHO Date: Fri, 3 Jan 2025 16:41:02 +0100 Subject: [PATCH 2/4] update creedengo-rules-spec --- pom.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 8940b3f..54cfa95 100644 --- a/pom.xml +++ b/pom.xml @@ -72,8 +72,7 @@ 1.7 - main-SNAPSHOT - + 2.1.0 https://repo1.maven.org/maven2 From ee5d5f64389c6ca3080d1ac2c6c6e34cfbd8cd59 Mon Sep 17 00:00:00 2001 From: David DE CARVALHO Date: Fri, 3 Jan 2025 17:00:11 +0100 Subject: [PATCH 3/4] clean code --- .../java/checks/MakeNonReassignedVariablesConstants.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java index 6323cc0..6533256 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java @@ -3,7 +3,6 @@ import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.check.Rule; -import org.sonar.java.model.ModifiersUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.tree.*; import org.sonar.plugins.java.api.tree.Tree.Kind; @@ -78,7 +77,6 @@ private static boolean parentIsKind(Tree tree, Kind... orKind) { } private static boolean isNotFinalAndNotStatic(VariableTree variableTree) { -// return ModifiersUtils.hasNoneOf(variableTree.modifiers(), Modifier.FINAL, Modifier.STATIC); return hasNoneOf(variableTree.modifiers(), Modifier.FINAL, Modifier.STATIC); } From 38952d789e7d19c3c27d5c799b2dcabf4b50f6a8 Mon Sep 17 00:00:00 2001 From: David DE CARVALHO Date: Fri, 3 Jan 2025 17:19:47 +0100 Subject: [PATCH 4/4] minor corrections --- CHANGELOG.md | 2 +- .../creedengo/java/creedengo_way_profile.json | 2 +- src/test/files/MakeNonReassignedVariablesConstants.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d46e871..277f0dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ 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 GCI82 - Make non reassigned variables constants +- [#89](https://github.com/green-code-initiative/creedengo-java/pull/89) Add new Java rule GCI82 - Make non reassigned variables constants ### Changed diff --git a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json index 8c61366..059bf0f 100644 --- a/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json +++ b/src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json @@ -17,7 +17,7 @@ "GCI77", "GCI78", "GCI79", - "GCI82", + "GCI82", "GCI94" ] } diff --git a/src/test/files/MakeNonReassignedVariablesConstants.java b/src/test/files/MakeNonReassignedVariablesConstants.java index a8e4039..bef640d 100644 --- a/src/test/files/MakeNonReassignedVariablesConstants.java +++ b/src/test/files/MakeNonReassignedVariablesConstants.java @@ -1,6 +1,6 @@ import java.util.logging.Logger; -public class GCI82MakeNonReassignedVariablesConstants { +public class MakeNonReassignedVariablesConstants { private final Logger logger = Logger.getLogger(""); // Compliant @@ -14,7 +14,7 @@ public class GCI82MakeNonReassignedVariablesConstants { private String varDefinedInClassReassigned = "0"; // Compliant private String varDefinedInConstructorReassigned = "1"; // Compliant - public GCI82MakeNonReassignedVariablesConstants() { + public MakeNonReassignedVariablesConstants() { varDefinedInConstructorReassigned = "3"; logger.info(varDefinedInConstructorReassigned); }