Skip to content

Commit 992acf1

Browse files
SONARJAVA-5980 Fix FN for S3749 with Lombok annotations (#5514)
Co-authored-by: lpilastri <115481625+leonardo-pilastri-sonarsource@users.noreply.github.com>
1 parent a32757a commit 992acf1

File tree

3 files changed

+66
-5
lines changed

3 files changed

+66
-5
lines changed

java-checks-test-sources/default/src/main/java/checks/S3749_SpringComponentWithNonAutowiredMembersCheck/S3749_DefaultAnnotations.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import javax.inject.Inject;
66
import javax.persistence.EntityManager;
77
import javax.persistence.PersistenceContext;
8+
import lombok.AllArgsConstructor;
9+
import lombok.RequiredArgsConstructor;
810
import org.slf4j.Logger;
911
import org.slf4j.LoggerFactory;
1012
import org.springframework.beans.factory.annotation.Autowired;
@@ -200,3 +202,51 @@ class JakartaRepo {
200202
@jakarta.annotation.Resource
201203
String email2 = null; // Compliant
202204
}
205+
206+
@Service
207+
@RequiredArgsConstructor
208+
class LombokInjected {
209+
210+
private final String injected; // Compliant
211+
private String notInjected; // Noncompliant {{Annotate this member with "@Autowired", "@Resource", "@Inject", or "@Value", or remove it.}}
212+
// ^^^^^^^^^^^
213+
214+
public void foo() {
215+
System.out.println(injected);
216+
System.out.println(notInjected);
217+
}
218+
219+
}
220+
221+
@Component
222+
@RequiredArgsConstructor
223+
class LombokInjected1 {
224+
225+
private final String injected; // Compliant
226+
@lombok.NonNull
227+
private String injectedNonNull; // Compliant
228+
private String notInjected; // Noncompliant {{Annotate this member with "@Autowired", "@Resource", "@Inject", or "@Value", or remove it.}}
229+
// ^^^^^^^^^^^
230+
231+
public void foo() {
232+
System.out.println(injected);
233+
System.out.println(injectedNonNull);
234+
System.out.println(notInjected);
235+
}
236+
237+
}
238+
239+
@Service
240+
@AllArgsConstructor
241+
class LombokInjected2 {
242+
243+
private final String injected; // Compliant
244+
private String notInjected; // Compliant
245+
246+
247+
public void foo() {
248+
System.out.println(injected);
249+
System.out.println(notInjected);
250+
}
251+
252+
}

java-checks/src/main/java/org/sonar/java/checks/spring/SpringComponentWithNonAutowiredMembersCheck.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,22 @@ public List<Tree.Kind> nodesToVisit() {
7070
@Override
7171
public void visitNode(Tree tree) {
7272
ClassTree clazzTree = (ClassTree) tree;
73-
Set<Symbol> symbolsUsedInConstructors = symbolsUsedInConstructors(clazzTree);
73+
SymbolMetadata classMetadata = clazzTree.symbol().metadata();
74+
75+
if (classMetadata.isAnnotatedWith("lombok.AllArgsConstructor")) {
76+
return;
77+
}
78+
boolean requiredArgsConstructorAnnotationPresent = classMetadata.isAnnotatedWith("lombok.RequiredArgsConstructor");
7479

75-
if (isSpringSingletonComponent(clazzTree.symbol().metadata())) {
80+
Set<Symbol> symbolsUsedInConstructors = symbolsUsedInConstructors(clazzTree);
81+
if (isSpringSingletonComponent(classMetadata)) {
7682
clazzTree.members().stream().filter(v -> v.is(Tree.Kind.VARIABLE))
7783
.map(VariableTree.class::cast)
7884
.filter(v -> !v.symbol().isStatic())
7985
.filter(v -> !isSpringInjectionAnnotated(v.symbol().metadata()))
8086
.filter(v -> !isCustomInjectionAnnotated(v.symbol().metadata()))
8187
.filter(v -> !symbolsUsedInConstructors.contains(v.symbol()))
88+
.filter(v -> !isInjectedByLombok(v, requiredArgsConstructorAnnotationPresent))
8289
.forEach(v -> reportIssue(v.simpleName(), "Annotate this member with \"@Autowired\", \"@Resource\", \"@Inject\", or \"@Value\", or remove it."));
8390
}
8491
}
@@ -103,6 +110,12 @@ private static boolean isUsingConfigurationProperties(SymbolMetadata classMeta)
103110
return classMeta.isAnnotatedWith("org.springframework.boot.context.properties.ConfigurationProperties");
104111
}
105112

113+
private static boolean isInjectedByLombok(VariableTree field, boolean requiredArgsConstructorAnnotationPresent) {
114+
return requiredArgsConstructorAnnotationPresent
115+
&& field.initializer() == null
116+
&& (field.symbol().isFinal() || field.symbol().metadata().isAnnotatedWith("lombok.NonNull"));
117+
}
118+
106119
private Set<Symbol> symbolsUsedInConstructors(ClassTree clazzTree) {
107120
List<Symbol.MethodSymbol> constructors = constructors(clazzTree);
108121
return constructors.stream()

java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.sonar.java.checks.UtilityClassWithPublicConstructorCheck;
3838
import org.sonar.java.checks.helpers.ExpressionsHelper;
3939
import org.sonar.java.checks.naming.BadFieldNameCheck;
40-
import org.sonar.java.checks.spring.SpringComponentWithNonAutowiredMembersCheck;
4140
import org.sonar.java.checks.tests.AssertionTypesCheck;
4241
import org.sonar.java.checks.unused.UnusedPrivateFieldCheck;
4342
import org.sonar.plugins.java.api.JavaCheck;
@@ -69,7 +68,6 @@ public class LombokFilter extends BaseTreeVisitorIssueFilter {
6968
/* S1450 */ PrivateFieldUsedLocallyCheck.class,
7069
/* S4248 */ RegexPatternsNeedlesslyCheck.class,
7170
/* S2159 */ SillyEqualsCheck.class,
72-
/* S3749 */ SpringComponentWithNonAutowiredMembersCheck.class,
7371
/* S2325 */ StaticMethodCheck.class,
7472
/* S1068 */ UnusedPrivateFieldCheck.class,
7573
/* S1128 */ UselessImportCheck.class,
@@ -130,7 +128,7 @@ public void visitClass(ClassTree tree) {
130128
boolean generatesEquals = usesAnnotation(tree, GENERATE_EQUALS);
131129

132130
excludeLinesIfTrue(generatesEquals || usesAnnotation(tree, GENERATE_UNUSED_FIELD_RELATED_METHODS), tree, UnusedPrivateFieldCheck.class, PrivateFieldUsedLocallyCheck.class);
133-
excludeLinesIfTrue(usesAnnotation(tree, GENERATE_CONSTRUCTOR), tree, AtLeastOneConstructorCheck.class, SpringComponentWithNonAutowiredMembersCheck.class);
131+
excludeLinesIfTrue(usesAnnotation(tree, GENERATE_CONSTRUCTOR), tree, AtLeastOneConstructorCheck.class);
134132
excludeLinesIfTrue(generatesEquals, tree, EqualsNotOverriddenInSubclassCheck.class, EqualsNotOverriddenWithCompareToCheck.class);
135133
excludeLinesIfTrue(generatesNonPublicConstructor(tree), tree, UtilityClassWithPublicConstructorCheck.class);
136134
boolean isUtilityClass = usesAnnotation(tree, UTILITY_CLASS);

0 commit comments

Comments
 (0)