diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a688cf3..886e840d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - correction of technical problem with Integration tests (because of Maven format in technical answer to "sonar-orchestrator-junit5" library) - upgrade JDK from 11 to 17 - [#4](https://github.com/green-code-initiative/creedengo-java/issues/4) Improvement: "++i" statement is not so bad +- [#113](https://github.com/green-code-initiative/creedengo-java/pull/113) fix GCI82 rule to handle Lombok generated setters ### Deleted 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 index 3a51b723..93081b59 100644 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java +++ b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java @@ -440,8 +440,8 @@ void testGCI82() { String filePath = "src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java"; String ruleId = "creedengo-java:GCI82"; String ruleMsg = "The variable is never reassigned and can be 'final'"; - int[] startLines = new int[]{7, 12, 13, 45}; - int[] endLines = new int[]{7, 12, 13, 45}; + int[] startLines = new int[]{13, 18, 19, 51, 82, 91}; + int[] endLines = new int[]{13, 18, 19, 51, 83, 92}; checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines); } diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/pom.xml b/src/it/test-projects/creedengo-java-plugin-test-project/pom.xml index cfb7ba4f..540c63bb 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/pom.xml +++ b/src/it/test-projects/creedengo-java-plugin-test-project/pom.xml @@ -32,6 +32,31 @@ spring-beans 5.3.25 + + org.projectlombok + lombok + 1.18.38 + - \ No newline at end of file + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.14.0 + + 17 + 17 + + + org.projectlombok + lombok + 1.18.38 + + + + + + + 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 index bef640d4..41d84e4d 100644 --- 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 @@ -1,9 +1,15 @@ import java.util.logging.Logger; +import lombok.Setter; +import lombok.Data; +import lombok.AccessLevel; public class MakeNonReassignedVariablesConstants { private final Logger logger = Logger.getLogger(""); // Compliant + @Setter + private String myLombokManagedString = "initialValue"; // 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 @@ -66,4 +72,22 @@ void classVariableReassignedBis() { logger.info(myFinalAndNotReassignedObject.toString()); } -} \ No newline at end of file +} + +@Setter +class myExtraClassWithLombokSetter { + private String myExtraClassString = "initialValue"; // Compliant + private final String myExtraClassFinalString = "initialValue"; // Compliant + + @Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}} + private String myExtraClassSetterNoneString = "initialValue"; +} + +@Data +class myExtraClassWithLombokData { + private String myExtraClassString = "initialValue"; // Compliant + private final String myExtraClassFinalString = "initialValue"; // Compliant + + @Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}} + private String myExtraClassSetterNoneString = "initialValue"; +} 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 6533256d..8d4caef9 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java @@ -17,6 +17,14 @@ public class MakeNonReassignedVariablesConstants extends IssuableSubscriptionVis private static final Logger LOGGER = Loggers.get(MakeNonReassignedVariablesConstants.class); + private final String LOMBOK_SETTER = "Setter"; + private final String LOMBOK_DATA = "Data"; + + private boolean hasParsedImports = false; + private boolean hasLombokSetterImport = false; + private boolean hasLombokDataImport = false; + + @Override public List nodesToVisit() { return List.of(Kind.VARIABLE); @@ -24,13 +32,13 @@ public List nodesToVisit() { @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)) { + VariableTree variableTree = (VariableTree) tree; + LOGGER.info("Variable > " + getVariableNameForLogger(variableTree)); + LOGGER.info(" => isNotFinalAndNotStatic(variableTree) = " + isNotFinalAndNotStatic(variableTree)); + LOGGER.info(" => usages = " + variableTree.symbol().usages().size()); + LOGGER.info(" => isNotReassigned = " + isNotReassigned(variableTree)); + if (hasNoLombokSetter(variableTree) && isNotFinalAndNotStatic(variableTree) && isNotReassigned(variableTree)) { reportIssue(tree, MESSAGE_RULE); } else { super.visitNode(tree); @@ -103,6 +111,77 @@ public static boolean hasModifier(ModifiersTree modifiersTree, Modifier expected return false; } + private boolean hasNoLombokSetter(VariableTree variableTree) { + // Check if the variable is annotated with @Setter + + for (AnnotationTree annotation : variableTree.modifiers().annotations()) { + if (annotation.annotationType().toString().equals(LOMBOK_SETTER)) { + if (hasLombokImport(variableTree, LOMBOK_SETTER)) { + + // Ignore if the annotation has AccessLevel.NONE + if (!annotation.arguments().isEmpty()) { + for (ExpressionTree argument : annotation.arguments()) { + if (argument.is(Kind.MEMBER_SELECT)) { + MemberSelectExpressionTree memberSelectExpressionTree = (MemberSelectExpressionTree) argument; + if (memberSelectExpressionTree.expression().toString().equals("AccessLevel") + && memberSelectExpressionTree.identifier().name().equals("NONE")) { + return true; + } + } + } + } + return false; + } + } + } + // Check if the variable is in a class with @Setter or with @Data + if( variableTree.parent() != null && !variableTree.parent().is(Kind.CLASS)){ + return true; + } + if (variableTree.parent() != null && variableTree.parent().is(Kind.CLASS)) { + ClassTree classTree = (ClassTree) variableTree.parent(); + for (AnnotationTree annotation : classTree.modifiers().annotations()) { + if (annotation.annotationType().toString().equals(LOMBOK_SETTER) && hasLombokImport(variableTree, LOMBOK_SETTER)) { + return false; + } + if (annotation.annotationType().toString().equals(LOMBOK_DATA) && hasLombokImport(variableTree, LOMBOK_DATA)) { + return false; + } + } + } + + return true; + } + + private boolean hasLombokImport(VariableTree variableTree, String lombokImport) { + if (!hasParsedImports) { + Tree currentTree = variableTree; + while (currentTree.parent() != null && !currentTree.parent().is(Kind.COMPILATION_UNIT)) { + currentTree = currentTree.parent(); + } + if (currentTree != null) { + CompilationUnitTree rootNode = (CompilationUnitTree) currentTree.parent(); + for (var importClauseTree : rootNode.imports()) { + ImportTree importTree = (ImportTree) importClauseTree; + MemberSelectExpressionTree identifier = (MemberSelectExpressionTree) importTree.qualifiedIdentifier(); + + if ("lombok".equals(identifier.expression().toString())) { + if ("*".equals(identifier.identifier().name())) { + hasLombokSetterImport = true; + hasLombokDataImport = true; + } else if (LOMBOK_SETTER.equals(identifier.identifier().name())) { + hasLombokSetterImport = true; + } else if (LOMBOK_DATA.equals(identifier.identifier().name())) { + hasLombokDataImport = true; + } + } + } + } + hasParsedImports = true; + } + return LOMBOK_SETTER.equals(lombokImport) ? hasLombokSetterImport : hasLombokDataImport; + } + private String getVariableNameForLogger(VariableTree variableTree) { String name = variableTree.simpleName().name(); diff --git a/src/test/files/MakeNonReassignedVariablesConstants.java b/src/test/files/MakeNonReassignedVariablesConstants.java index bef640d4..41d84e4d 100644 --- a/src/test/files/MakeNonReassignedVariablesConstants.java +++ b/src/test/files/MakeNonReassignedVariablesConstants.java @@ -1,9 +1,15 @@ import java.util.logging.Logger; +import lombok.Setter; +import lombok.Data; +import lombok.AccessLevel; public class MakeNonReassignedVariablesConstants { private final Logger logger = Logger.getLogger(""); // Compliant + @Setter + private String myLombokManagedString = "initialValue"; // 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 @@ -66,4 +72,22 @@ void classVariableReassignedBis() { logger.info(myFinalAndNotReassignedObject.toString()); } -} \ No newline at end of file +} + +@Setter +class myExtraClassWithLombokSetter { + private String myExtraClassString = "initialValue"; // Compliant + private final String myExtraClassFinalString = "initialValue"; // Compliant + + @Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}} + private String myExtraClassSetterNoneString = "initialValue"; +} + +@Data +class myExtraClassWithLombokData { + private String myExtraClassString = "initialValue"; // Compliant + private final String myExtraClassFinalString = "initialValue"; // Compliant + + @Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}} + private String myExtraClassSetterNoneString = "initialValue"; +}