Skip to content

Commit 83a1630

Browse files
author
Morille, Amelie (ITDV XF) - AF
committed
fix(GCI82): update GCI82 rule to handle Lombok generated setters
1 parent 7c576c4 commit 83a1630

File tree

6 files changed

+164
-11
lines changed

6 files changed

+164
-11
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- upgrade libraries versions
1616
- correction of technical problem with Integration tests (because of Maven format in technical answer to "sonar-orchestrator-junit5" library)
1717
- upgrade JDK from 11 to 17
18+
- [#113](https://github.com/green-code-initiative/creedengo-java/pull/113) fix GCI82 rule to handle Lombok generated setters
1819

1920
### Deleted
2021

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,8 @@ void testGCI82() {
440440
String filePath = "src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java";
441441
String ruleId = "creedengo-java:GCI82";
442442
String ruleMsg = "The variable is never reassigned and can be 'final'";
443-
int[] startLines = new int[]{7, 12, 13, 45};
444-
int[] endLines = new int[]{7, 12, 13, 45};
443+
int[] startLines = new int[]{13, 18, 19, 51, 82, 91};
444+
int[] endLines = new int[]{13, 18, 19, 51, 83, 92};
445445

446446
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines);
447447
}

src/it/test-projects/creedengo-java-plugin-test-project/pom.xml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,31 @@
3232
<artifactId>spring-beans</artifactId>
3333
<version>5.3.25</version>
3434
</dependency>
35+
<dependency>
36+
<groupId>org.projectlombok</groupId>
37+
<artifactId>lombok</artifactId>
38+
<version>1.18.38</version>
39+
</dependency>
3540
</dependencies>
3641

37-
</project>
42+
<build>
43+
<plugins>
44+
<plugin>
45+
<groupId>org.apache.maven.plugins</groupId>
46+
<artifactId>maven-compiler-plugin</artifactId>
47+
<version>3.14.0</version>
48+
<configuration>
49+
<source>17</source>
50+
<target>17</target>
51+
<annotationProcessorPaths>
52+
<path>
53+
<groupId>org.projectlombok</groupId>
54+
<artifactId>lombok</artifactId>
55+
<version>1.18.38</version>
56+
</path>
57+
</annotationProcessorPaths>
58+
</configuration>
59+
</plugin>
60+
</plugins>
61+
</build>
62+
</project>

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
import java.util.logging.Logger;
2+
import lombok.Setter;
3+
import lombok.Data;
4+
import lombok.AccessLevel;
25

36
public class MakeNonReassignedVariablesConstants {
47

58
private final Logger logger = Logger.getLogger(""); // Compliant
69

10+
@Setter
11+
private String myLombokManagedString = "initialValue"; // Compliant
12+
713
private Object myNonFinalAndNotReassignedObject = new Object(); // Noncompliant {{The variable is never reassigned and can be 'final'}}
814
private Object myNonFinalAndReassignedObject = new Object(); // Compliant
915
private final Object myFinalAndNotReassignedObject = new Object(); // Compliant
@@ -66,4 +72,22 @@ void classVariableReassignedBis() {
6672
logger.info(myFinalAndNotReassignedObject.toString());
6773
}
6874

69-
}
75+
}
76+
77+
@Setter
78+
class myExtraClassWithLombokSetter {
79+
private String myExtraClassString = "initialValue"; // Compliant
80+
private final String myExtraClassFinalString = "initialValue"; // Compliant
81+
82+
@Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}}
83+
private String myExtraClassSetterNoneString = "initialValue";
84+
}
85+
86+
@Data
87+
class myExtraClassWithLombokData {
88+
private String myExtraClassString = "initialValue"; // Compliant
89+
private final String myExtraClassFinalString = "initialValue"; // Compliant
90+
91+
@Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}}
92+
private String myExtraClassSetterNoneString = "initialValue";
93+
}

src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,28 @@ public class MakeNonReassignedVariablesConstants extends IssuableSubscriptionVis
1717

1818
private static final Logger LOGGER = Loggers.get(MakeNonReassignedVariablesConstants.class);
1919

20+
private final String LOMBOK_SETTER = "Setter";
21+
private final String LOMBOK_DATA = "Data";
22+
23+
private boolean hasParsedImports = false;
24+
private boolean hasLombokSetterImport = false;
25+
private boolean hasLombokDataImport = false;
26+
27+
2028
@Override
2129
public List<Kind> nodesToVisit() {
2230
return List.of(Kind.VARIABLE);
2331
}
2432

2533
@Override
2634
public void visitNode(@Nonnull Tree tree) {
27-
VariableTree variableTree = (VariableTree) tree;
28-
LOGGER.debug("Variable > " + getVariableNameForLogger(variableTree));
29-
LOGGER.debug(" => isNotFinalAndNotStatic(variableTree) = " + isNotFinalAndNotStatic(variableTree));
30-
LOGGER.debug(" => usages = " + variableTree.symbol().usages().size());
31-
LOGGER.debug(" => isNotReassigned = " + isNotReassigned(variableTree));
3235

33-
if (isNotFinalAndNotStatic(variableTree) && isNotReassigned(variableTree)) {
36+
VariableTree variableTree = (VariableTree) tree;
37+
LOGGER.info("Variable > " + getVariableNameForLogger(variableTree));
38+
LOGGER.info(" => isNotFinalAndNotStatic(variableTree) = " + isNotFinalAndNotStatic(variableTree));
39+
LOGGER.info(" => usages = " + variableTree.symbol().usages().size());
40+
LOGGER.info(" => isNotReassigned = " + isNotReassigned(variableTree));
41+
if (hasNoLombokSetter(variableTree) && isNotFinalAndNotStatic(variableTree) && isNotReassigned(variableTree)) {
3442
reportIssue(tree, MESSAGE_RULE);
3543
} else {
3644
super.visitNode(tree);
@@ -103,6 +111,77 @@ public static boolean hasModifier(ModifiersTree modifiersTree, Modifier expected
103111
return false;
104112
}
105113

114+
private boolean hasNoLombokSetter(VariableTree variableTree) {
115+
// Check if the variable is annotated with @Setter
116+
117+
for (AnnotationTree annotation : variableTree.modifiers().annotations()) {
118+
if (annotation.annotationType().toString().equals(LOMBOK_SETTER)) {
119+
if (hasLombokImport(variableTree, LOMBOK_SETTER)) {
120+
121+
// Ignore if the annotation has AccessLevel.NONE
122+
if (!annotation.arguments().isEmpty()) {
123+
for (ExpressionTree argument : annotation.arguments()) {
124+
if (argument.is(Kind.MEMBER_SELECT)) {
125+
MemberSelectExpressionTree memberSelectExpressionTree = (MemberSelectExpressionTree) argument;
126+
if (memberSelectExpressionTree.expression().toString().equals("AccessLevel")
127+
&& memberSelectExpressionTree.identifier().name().equals("NONE")) {
128+
return true;
129+
}
130+
}
131+
}
132+
}
133+
return false;
134+
}
135+
}
136+
}
137+
// Check if the variable is in a class with @Setter or with @Data
138+
if( variableTree.parent() != null && !variableTree.parent().is(Kind.CLASS)){
139+
return true;
140+
}
141+
if (variableTree.parent() != null && variableTree.parent().is(Kind.CLASS)) {
142+
ClassTree classTree = (ClassTree) variableTree.parent();
143+
for (AnnotationTree annotation : classTree.modifiers().annotations()) {
144+
if (annotation.annotationType().toString().equals(LOMBOK_SETTER) && hasLombokImport(variableTree, LOMBOK_SETTER)) {
145+
return false;
146+
}
147+
if (annotation.annotationType().toString().equals(LOMBOK_DATA) && hasLombokImport(variableTree, LOMBOK_DATA)) {
148+
return false;
149+
}
150+
}
151+
}
152+
153+
return true;
154+
}
155+
156+
private boolean hasLombokImport(VariableTree variableTree, String lombokImport) {
157+
if (!hasParsedImports) {
158+
Tree currentTree = variableTree;
159+
while (currentTree.parent() != null && !currentTree.parent().is(Kind.COMPILATION_UNIT)) {
160+
currentTree = currentTree.parent();
161+
}
162+
if (currentTree != null) {
163+
CompilationUnitTree rootNode = (CompilationUnitTree) currentTree.parent();
164+
for (var importClauseTree : rootNode.imports()) {
165+
ImportTree importTree = (ImportTree) importClauseTree;
166+
MemberSelectExpressionTree identifier = (MemberSelectExpressionTree) importTree.qualifiedIdentifier();
167+
168+
if ("lombok".equals(identifier.expression().toString())) {
169+
if ("*".equals(identifier.identifier().name())) {
170+
hasLombokSetterImport = true;
171+
hasLombokDataImport = true;
172+
} else if (LOMBOK_SETTER.equals(identifier.identifier().name())) {
173+
hasLombokSetterImport = true;
174+
} else if (LOMBOK_DATA.equals(identifier.identifier().name())) {
175+
hasLombokDataImport = true;
176+
}
177+
}
178+
}
179+
}
180+
hasParsedImports = true;
181+
}
182+
return LOMBOK_SETTER.equals(lombokImport) ? hasLombokSetterImport : hasLombokDataImport;
183+
}
184+
106185
private String getVariableNameForLogger(VariableTree variableTree) {
107186
String name = variableTree.simpleName().name();
108187

src/test/files/MakeNonReassignedVariablesConstants.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
import java.util.logging.Logger;
2+
import lombok.Setter;
3+
import lombok.Data;
4+
import lombok.AccessLevel;
25

36
public class MakeNonReassignedVariablesConstants {
47

58
private final Logger logger = Logger.getLogger(""); // Compliant
69

10+
@Setter
11+
private String myLombokManagedString = "initialValue"; // Compliant
12+
713
private Object myNonFinalAndNotReassignedObject = new Object(); // Noncompliant {{The variable is never reassigned and can be 'final'}}
814
private Object myNonFinalAndReassignedObject = new Object(); // Compliant
915
private final Object myFinalAndNotReassignedObject = new Object(); // Compliant
@@ -66,4 +72,22 @@ void classVariableReassignedBis() {
6672
logger.info(myFinalAndNotReassignedObject.toString());
6773
}
6874

69-
}
75+
}
76+
77+
@Setter
78+
class myExtraClassWithLombokSetter {
79+
private String myExtraClassString = "initialValue"; // Compliant
80+
private final String myExtraClassFinalString = "initialValue"; // Compliant
81+
82+
@Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}}
83+
private String myExtraClassSetterNoneString = "initialValue";
84+
}
85+
86+
@Data
87+
class myExtraClassWithLombokData {
88+
private String myExtraClassString = "initialValue"; // Compliant
89+
private final String myExtraClassFinalString = "initialValue"; // Compliant
90+
91+
@Setter(AccessLevel.NONE) // Noncompliant {{The variable is never reassigned and can be 'final'}}
92+
private String myExtraClassSetterNoneString = "initialValue";
93+
}

0 commit comments

Comments
 (0)