Skip to content

Commit fbba6c6

Browse files
Rejoice472Rejoice472
authored
Feature/managed bean diagnostics experience improved (#1414)
* Explore whether the user experience can be improved for diagnostics / quick fixes involving @dependent * updated test cases * added test resource to support the new test scenarios * format issue corrected * Update ManagedBeanTest.java * Update ManagedBeanTest.java * Update ManagedBeanTest.java * Update ManagedBeanTest.java
1 parent 25ae118 commit fbba6c6

File tree

4 files changed

+225
-27
lines changed

4 files changed

+225
-27
lines changed

‎src/main/java/io/openliberty/tools/intellij/lsp4jakarta/lsp4ij/cdi/ManagedBeanDiagnosticsCollector.java‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2021, 2024 IBM Corporation.
2+
* Copyright (c) 2021, 2025 IBM Corporation.
33
*
44
* This program and the accompanying materials are made available under the
55
* terms of the Eclipse Public License v. 2.0 which is available at
@@ -53,17 +53,12 @@ public void collectDiagnostics(PsiJavaFile unit, List<Diagnostic> diagnostics) {
5353
List<String> managedBeanAnnotations = getMatchedJavaElementNames(type, Stream.of(type.getAnnotations())
5454
.map(annotation -> annotation.getQualifiedName()).toArray(String[]::new),
5555
scopeFQNames);
56-
boolean isManagedBean = managedBeanAnnotations.size() > 0;
57-
58-
if (managedBeanAnnotations.size() > 1) {
59-
diagnostics.add(createDiagnostic(type, unit,
60-
Messages.getMessage("ScopeTypeAnnotationsManagedBean"),
61-
DIAGNOSTIC_CODE_SCOPEDECL, (JsonArray) (new Gson().toJsonTree(managedBeanAnnotations)),
62-
DiagnosticSeverity.Error));
63-
}
64-
56+
boolean isManagedBean = !managedBeanAnnotations.isEmpty();
57+
boolean isDependent = managedBeanAnnotations.stream().anyMatch(DEPENDENT_FQ_NAME::equals);
58+
boolean hasMultipleScopes = managedBeanAnnotations.size() > 1;
6559
String[] injectAnnotations = { PRODUCES_FQ_NAME, INJECT_FQ_NAME };
6660
PsiField fields[] = type.getFields();
61+
boolean nonStaticPublicFieldPresent = false;
6762
for (PsiField field : fields) {
6863
String[] annotationNames = Stream.of(field.getAnnotations())
6964
.map(annotation -> annotation.getQualifiedName()).toArray(String[]::new);
@@ -77,10 +72,8 @@ public void collectDiagnostics(PsiJavaFile unit, List<Diagnostic> diagnostics) {
7772
*
7873
* https://jakarta.ee/specifications/cdi/3.0/jakarta-cdi-spec-3.0.html#managed_beans
7974
*/
80-
if (isManagedBean
81-
&& field.hasModifierProperty(PsiModifier.PUBLIC)
82-
&& !field.hasModifierProperty(PsiModifier.STATIC)
83-
&& !managedBeanAnnotations.contains(DEPENDENT_FQ_NAME)) {
75+
if (validateNonStaticPublicField(isManagedBean, isDependent, hasMultipleScopes, field)) {
76+
nonStaticPublicFieldPresent = true;
8477
diagnostics.add(createDiagnostic(field, unit,
8578
Messages.getMessage("ManagedBeanWithNonStaticPublicField"),
8679
DIAGNOSTIC_CODE, null,
@@ -233,12 +226,17 @@ else if (INJECT_FQ_NAME.equals(annotation))
233226
*/
234227
if (isManagedBean) {
235228
boolean isClassGeneric = type.getTypeParameters().length != 0;
236-
boolean isDependent = managedBeanAnnotations.stream()
237-
.anyMatch(annotation -> DEPENDENT_FQ_NAME.equals(annotation));
238-
239-
if (isClassGeneric && !isDependent) {
229+
if (isClassGeneric && (!isDependent || hasMultipleScopes)) {
240230
diagnostics.add(createDiagnostic(type, unit, Messages.getMessage("ManagedBeanGenericType"),
241231
DIAGNOSTIC_CODE, null, DiagnosticSeverity.Error));
232+
} else if (nonStaticPublicFieldPresent) {
233+
diagnostics.add(createDiagnostic(type, unit, Messages.getMessage("ManagedBeanWithNonStaticPublicField"),
234+
DIAGNOSTIC_CODE, null, DiagnosticSeverity.Error));
235+
} else if (hasMultipleScopes) {
236+
diagnostics.add(createDiagnostic(type, unit,
237+
Messages.getMessage("ScopeTypeAnnotationsManagedBean"),
238+
DIAGNOSTIC_CODE_SCOPEDECL, new Gson().toJsonTree(managedBeanAnnotations),
239+
DiagnosticSeverity.Error));
242240
}
243241
}
244242

@@ -358,4 +356,21 @@ private String createInvalidProducesLabel(Set<String> invalidAnnotations) {
358356
private String createInvalidDisposesLabel(Set<String> invalidAnnotations) {
359357
return Messages.getMessage("ManagedBeanInvalidDisposer", String.join(", ", invalidAnnotations)); // assuming comma delimited list is ok
360358
}
361-
}
359+
360+
/**
361+
* validateNonStaticPublicField
362+
* This is to verify whether the @Dependent annotation must be the only scope applied to a managed bean
363+
* that contains a non-static public field.
364+
*
365+
* @param isManagedBean
366+
* @param isDependent
367+
* @param hasMultipleScopes
368+
* @param field
369+
* @return
370+
*/
371+
private boolean validateNonStaticPublicField(boolean isManagedBean, boolean isDependent, boolean hasMultipleScopes,
372+
PsiField field) {
373+
return isManagedBean && field.hasModifierProperty(PsiModifier.PUBLIC) && !field.hasModifierProperty(PsiModifier.STATIC)
374+
&& (!isDependent || hasMultipleScopes);
375+
}
376+
}

‎src/main/resources/io/openliberty/tools/intellij/lsp4jakarta/messages/messages.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ ManagedBeanDisposeOneParameter = The @Disposes annotation must not be defined on
8484
ManagedBeanInvalidDisposer = A disposer method cannot have parameter(s) annotated with {0}.
8585
ManagedBeanInvalidProduces = A producer method cannot have parameter(s) annotated with {0}.
8686
ManagedBeanInvalidInject = A bean constructor or a method annotated with @Inject cannot have parameter(s) annotated with {0}.
87-
ManagedBeanGenericType = Managed bean class of generic type must have scope @Dependent.
87+
ManagedBeanGenericType = The @Dependent annotation must be the only scope defined by a Managed bean class of generic type.
8888
ManagedBeanProducesAndInject = The @Produces and @Inject annotations must not be used on the same field or property.
8989

9090
# ManagedBeanNoArgConstructorQuickFix

‎src/test/java/io/openliberty/tools/intellij/lsp4jakarta/it/cdi/ManagedBeanTest.java‎

Lines changed: 146 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2021, 2024 IBM Corporation and others.
2+
* Copyright (c) 2021, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials are made available under the
55
* terms of the Eclipse Public License v. 2.0 which is available at
@@ -58,7 +58,7 @@ public void managedBeanAnnotations() throws Exception {
5858
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
5959

6060
Diagnostic d2 = d(5, 13, 24,
61-
"Managed bean class of generic type must have scope @Dependent.",
61+
"The @Dependent annotation must be the only scope defined by a Managed bean class of generic type.",
6262
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
6363

6464
assertJavaDiagnostics(diagnosticsParams, utils, d1, d2);
@@ -84,7 +84,150 @@ public void managedBeanAnnotations() throws Exception {
8484
CodeAction ca2 = ca(uri, "Replace current scope with @Dependent", d2, te2);
8585
assertJavaCodeAction(codeActionParams2, utils, ca2);
8686
}
87-
87+
88+
@Test
89+
public void ManagedBeanWithDependent() throws Exception {
90+
Module module = createMavenModule(new File("src/test/resources/projects/maven/jakarta-sample"));
91+
IPsiUtils utils = PsiUtilsLSImpl.getInstance(getProject());
92+
93+
VirtualFile javaFile = LocalFileSystem.getInstance().refreshAndFindFileByPath(ModuleUtilCore.getModuleDirPath(module)
94+
+ "/src/main/java/io/openliberty/sample/jakarta/cdi/ManagedBeanWithDependent.java");
95+
String uri = VfsUtilCore.virtualToIoFile(javaFile).toURI().toString();
96+
97+
JakartaJavaDiagnosticsParams diagnosticsParams = new JakartaJavaDiagnosticsParams();
98+
diagnosticsParams.setUris(Arrays.asList(uri));
99+
100+
// test expected diagnostic
101+
Diagnostic d1 = d(6, 13, 37,
102+
"The @Dependent annotation must be the only scope defined by a Managed bean class of generic type.",
103+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
104+
105+
Diagnostic d2 = d(7, 15, 16,
106+
"The @Dependent annotation must be the only scope defined by a managed bean with a non-static public field.",
107+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
108+
109+
Diagnostic d3 = d(17, 6, 27,
110+
"The @Dependent annotation must be the only scope defined by a managed bean with a non-static public field.",
111+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
112+
113+
Diagnostic d4 = d(18, 15, 16,
114+
"The @Dependent annotation must be the only scope defined by a managed bean with a non-static public field.",
115+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
116+
117+
Diagnostic d5 = d(27, 6, 33,
118+
"The @Dependent annotation must be the only scope defined by a Managed bean class of generic type.",
119+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidManagedBeanAnnotation");
120+
121+
Diagnostic d6 = d(37, 6, 36,
122+
"Scope type annotations must be specified by a managed bean class at most once.",
123+
DiagnosticSeverity.Error, "jakarta-cdi", "InvalidScopeDecl");
124+
d6.setData(new Gson().toJsonTree(Arrays.asList("jakarta.enterprise.context.SessionScoped", "jakarta.enterprise.context.RequestScoped")));
125+
126+
assertJavaDiagnostics(diagnosticsParams, utils, d1, d2, d3, d4, d5, d6);
127+
128+
String newText1 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
129+
"@Dependent\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
130+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n" +
131+
"@Dependent\n@RequestScoped\n@SessionScoped\nclass NonGenericManagedBean {\n public int a;\n\n " +
132+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n@SessionScoped\n" +
133+
"class ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
134+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n@SessionScoped\n" +
135+
"class ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
136+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
137+
String newText2 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
138+
"@Dependent\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
139+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n" +
140+
"@Dependent\n@RequestScoped\n@SessionScoped\nclass NonGenericManagedBean {\n public int a;\n\n " +
141+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n@SessionScoped\n" +
142+
"class ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
143+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n@SessionScoped\n" +
144+
"class ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
145+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
146+
String newText3 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
147+
"@Dependent\n@RequestScoped\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
148+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n" +
149+
"@Dependent\nclass NonGenericManagedBean {\n public int a;\n\n " +
150+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
151+
"@SessionScoped\nclass ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
152+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
153+
"@SessionScoped\nclass ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
154+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
155+
String newText4 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
156+
"@Dependent\n@RequestScoped\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
157+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n@Dependent\n" +
158+
"class NonGenericManagedBean {\n public int a;\n\n " +
159+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n@SessionScoped\n" +
160+
"class ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
161+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
162+
"@SessionScoped\nclass ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
163+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
164+
String newText5 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
165+
"@Dependent\n@RequestScoped\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
166+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n@Dependent\n@RequestScoped\n" +
167+
"@SessionScoped\nclass NonGenericManagedBean {\n public int a;\n\n " +
168+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@Dependent\n" +
169+
"class ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
170+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
171+
"@SessionScoped\nclass ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
172+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
173+
String newText61 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
174+
"@Dependent\n@RequestScoped\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
175+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n@Dependent\n@RequestScoped\n" +
176+
"@SessionScoped\nclass NonGenericManagedBean {\n public int a;\n\n " +
177+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
178+
"@SessionScoped\nclass ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
179+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@SessionScoped\n" +
180+
"class ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
181+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
182+
String newText62 = "package io.openliberty.sample.jakarta.cdi;\n\nimport jakarta.enterprise.context.*;\n\n" +
183+
"@Dependent\n@RequestScoped\npublic class ManagedBeanWithDependent<T> {\n public int a;\n\n " +
184+
"public ManagedBeanWithDependent() {\n this.a = 10;\n }\n}\n\n@Dependent\n@RequestScoped\n" +
185+
"@SessionScoped\nclass NonGenericManagedBean {\n public int a;\n\n " +
186+
"public NonGenericManagedBean() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
187+
"@SessionScoped\nclass ManagedBeanWithoutDependent<T> {\n public static int a;\n\n " +
188+
"public ManagedBeanWithoutDependent() {\n this.a = 10;\n }\n}\n\n@RequestScoped\n" +
189+
"class ManagedBeanWithMultipleScopes2 {\n public static int a;\n\n " +
190+
"public ManagedBeanWithMultipleScopes2() {\n this.a = 10;\n }\n}";
191+
192+
// Assert for the diagnostic d1
193+
JakartaJavaCodeActionParams codeActionParams1 = createCodeActionParams(uri, d1);
194+
TextEdit te1 = te(0, 0, 43, 1, newText1);
195+
CodeAction ca1 = ca(uri, "Replace current scope with @Dependent", d1, te1);
196+
assertJavaCodeAction(codeActionParams1, utils, ca1);
197+
198+
// Assert for the diagnostic d2
199+
JakartaJavaCodeActionParams codeActionParams2 = createCodeActionParams(uri, d2);
200+
TextEdit te2 = te(0, 0, 43, 1, newText2);
201+
CodeAction ca2 = ca(uri, "Replace current scope with @Dependent", d2, te2);
202+
assertJavaCodeAction(codeActionParams2, utils, ca2);
203+
204+
// Assert for the diagnostic d3
205+
JakartaJavaCodeActionParams codeActionParams3 = createCodeActionParams(uri, d3);
206+
TextEdit te3 = te(0, 0, 43, 1, newText3);
207+
CodeAction ca3 = ca(uri, "Replace current scope with @Dependent", d3, te3);
208+
assertJavaCodeAction(codeActionParams3, utils, ca3);
209+
210+
// Assert for the diagnostic d4
211+
JakartaJavaCodeActionParams codeActionParams4 = createCodeActionParams(uri, d4);
212+
TextEdit te4 = te(0, 0, 43, 1, newText4);
213+
CodeAction ca4 = ca(uri, "Replace current scope with @Dependent", d4, te4);
214+
assertJavaCodeAction(codeActionParams4, utils, ca4);
215+
216+
// Assert for the diagnostic d5
217+
JakartaJavaCodeActionParams codeActionParams5 = createCodeActionParams(uri, d5);
218+
TextEdit te5 = te(0, 0, 43, 1, newText5);
219+
CodeAction ca5 = ca(uri, "Replace current scope with @Dependent", d5, te5);
220+
assertJavaCodeAction(codeActionParams5, utils, ca5);
221+
222+
// Assert for the diagnostic d6
223+
JakartaJavaCodeActionParams codeActionParams6 = createCodeActionParams(uri, d6);
224+
TextEdit te61 = te(0, 0, 43, 1, newText61);
225+
CodeAction ca61 = ca(uri, "Remove @RequestScoped", d6, te61);
226+
TextEdit te62 = te(0, 0, 43, 1, newText62);
227+
CodeAction ca62 = ca(uri, "Remove @SessionScoped", d6, te62);
228+
assertJavaCodeAction(codeActionParams6, utils, ca61, ca62);
229+
}
230+
88231
@Test
89232
public void scopeDeclaration() throws Exception {
90233
Module module = createMavenModule(new File("src/test/resources/projects/maven/jakarta-sample"));
@@ -1297,7 +1440,6 @@ public void producesAndDisposesObservesObservesAsync() throws Exception {
12971440
assertJavaCodeAction(codeActionParams8, utils, ca20, ca21);
12981441
}
12991442

1300-
13011443
@Test
13021444
public void multipleDisposes() throws Exception {
13031445
Module module = createMavenModule(new File("src/test/resources/projects/maven/jakarta-sample"));
@@ -1306,14 +1448,11 @@ public void multipleDisposes() throws Exception {
13061448
VirtualFile javaFile = LocalFileSystem.getInstance().refreshAndFindFileByPath(ModuleUtilCore.getModuleDirPath(module)
13071449
+ "/src/main/java/io/openliberty/sample/jakarta/cdi/MultipleDisposes.java");
13081450
String uri = VfsUtilCore.virtualToIoFile(javaFile).toURI().toString();
1309-
13101451
JakartaJavaDiagnosticsParams diagnosticsParams = new JakartaJavaDiagnosticsParams();
13111452
diagnosticsParams.setUris(Arrays.asList(uri));
1312-
13131453
Diagnostic d = d(9, 18, 23,
13141454
"The @Disposes annotation must not be defined on more than one parameter of a method.",
13151455
DiagnosticSeverity.Error, "jakarta-cdi", "RemoveExtraDisposes");
1316-
13171456
assertJavaDiagnostics(diagnosticsParams, utils, d);
13181457
}
13191458
}

0 commit comments

Comments
 (0)