matthiasblaesing commented on code in PR #5866:
URL: https://github.com/apache/netbeans/pull/5866#discussion_r1175699215
##########
enterprise/web.beans/src/org/netbeans/modules/web/beans/analysis/analyzer/AbstractDecoratorAnalyzer.java:
##########
@@ -168,7 +168,7 @@ protected boolean checkBuiltInBeans( VariableElement
element,
Map<String, ? extends AnnotationMirror> qualifiersFqns = helper.
getAnnotationsByType(qualifiers);
boolean hasOnlyDefault = false;
- if ( qualifiersFqns.containsKey(AnnotationUtil.DEFAULT_FQN)){
+ if ( qualifiersFqns.keySet().contains(AnnotationUtil.DEFAULT_FQN)){
Review Comment:
Unnecessary change.
##########
enterprise/web.beans/src/org/netbeans/modules/web/beans/analysis/analyzer/field/InjectionPointAnalyzer.java:
##########
@@ -182,7 +182,7 @@ private void checkInjectionPointMetadata( VariableElement
element,
Map<String, ? extends AnnotationMirror> qualifiersFqns = helper.
getAnnotationsByType(qualifiers);
boolean hasDefault = model.hasImplicitDefaultQualifier( element );
- if ( !hasDefault &&
qualifiersFqns.containsKey(AnnotationUtil.DEFAULT_FQN)){
+ if ( !hasDefault &&
qualifiersFqns.keySet().contains(AnnotationUtil.DEFAULT_FQN)){
Review Comment:
Unnecessary change.
##########
enterprise/web.beans/src/org/netbeans/modules/web/beans/analysis/analyzer/method/InjectionPointParameterAnalyzer.java:
##########
@@ -194,7 +194,7 @@ private void checkInjectionPointMetadata( VariableElement
var,
.getAnnotationsByType(qualifiers);
boolean hasDefault = model.hasImplicitDefaultQualifier(varElement);
if (!hasDefault
- && qualifiersFqns.containsKey(AnnotationUtil.DEFAULT_FQN))
+ &&
qualifiersFqns.keySet().contains(AnnotationUtil.DEFAULT_FQN))
Review Comment:
Unnecessary change.
##########
enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/el/WebBeansELVariableResolver.java:
##########
@@ -134,6 +138,33 @@ public List<WebBean> run(WebBeansModel metadata) throws
Exception {
return Collections.emptyList();
}
+ private static List<WebBean>
getJakartaNamedBeans(MetadataModel<org.netbeans.modules.jakarta.web.beans.api.model.WebBeansModel>
webBeansModel) {
+ try {
+ return webBeansModel.runReadAction(new
MetadataModelAction<org.netbeans.modules.jakarta.web.beans.api.model.WebBeansModel,
List<WebBean>>() {
Review Comment:
I think this and the `javax` variant would IMHO both benefit from being
turned into a lambda. The complete variable declaration is only two lines above
the second declaration so adds little to readablity and the size makes it
harder to spot what is really going on.
##########
enterprise/web.jsf/src/org/netbeans/modules/web/jsf/wizards/PersistenceClientIterator.java:
##########
@@ -632,8 +632,18 @@ private static boolean showImportStatement(String
packageName, String fqn) {
}
private static boolean isCdiEnabled(Project project) {
- CdiUtil cdiUtil = project.getLookup().lookup(CdiUtil.class);
- return (cdiUtil == null) ? false : cdiUtil.isCdiEnabled();
+ WebModule wm = WebModule.getWebModule(project.getProjectDirectory());
Review Comment:
Can really be both `CdiUtils` in the lookup? If not, I would ditch the check
here and just try to lookup both sequentially and the first that is found, is
used.
##########
java/java.lsp.server/nbcode/nbproject/platform.properties:
##########
@@ -344,6 +343,7 @@ disabled.modules=\
org.netbeans.modules.j2ee.sun.appsrv,\
org.netbeans.modules.j2ee.sun.dd,\
org.netbeans.modules.j2ee.sun.ddui,\
+ org.netbeans.modules.jakartaee.web.beans,\
Review Comment:
This looks strange (both changes). Unintentional?
##########
ide/languages.hcl/nbproject/project.properties:
##########
@@ -14,5 +14,5 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-javac.source=1.8
+javac.source=11
Review Comment:
This looks strange and wrong.
##########
enterprise/web.jsf/src/org/netbeans/modules/web/jsf/hints/rules/FlowScopedBeanWithoutCdi.java:
##########
@@ -115,7 +127,7 @@ public String getText() {
@Override
public ChangeInfo implement() throws Exception {
- CdiUtil cdiUtil = project.getLookup().lookup(CdiUtil.class);
+ org.netbeans.modules.web.beans.CdiUtil cdiUtil =
project.getLookup().lookup(org.netbeans.modules.web.beans.CdiUtil.class);
Review Comment:
Looking at the additions from line 79+, I think here the case needs to be
handled, where `org.netbeans.modules.jakarta.web.beans.CdiUtil` is present
instead of `org.netbeans.modules.web.beans.CdiUtil`.
##########
enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/JsfSupportImpl.java:
##########
@@ -172,12 +171,22 @@ public void propertyChange(PropertyChangeEvent evt) {
}
});
- webBeansModel = new MetaModelSupport(project).getMetaModel();
-
+ if(isJsf30Plus()){
+ System.out.println("WE have JSF4+");
Review Comment:
Stray `System.out.println` should be removed. Same two rows down.
##########
enterprise/web.beans/src/org/netbeans/modules/web/beans/impl/model/EnableBeansFilter.java:
##########
@@ -471,7 +471,7 @@ else if ( type instanceof WildcardType ){
private boolean hasModifier ( Element element , Modifier mod){
Set<Modifier> modifiers = element.getModifiers();
for (Modifier modifier : modifiers) {
- if (modifier == mod) {
+ if ( modifier.equals( mod )){
Review Comment:
`Modifier` is an `enum`, thus identity comparison is valid here. If I'm not
mistaken, all changes to `web.beans` can be rolled back. If there are equal
constructs in `jakarta.web.beans`, it should be modified there also.
##########
enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/JsfSupportImpl.java:
##########
@@ -252,9 +261,14 @@ public FaceletsLibrarySupport getFaceletsLibrarySupport() {
return faceletsLibrarySupport;
}
- public synchronized MetadataModel<WebBeansModel> getWebBeansModel() {
+ public synchronized
MetadataModel<org.netbeans.modules.web.beans.api.model.WebBeansModel>
getWebBeansModel() {
return webBeansModel;
}
+
+ public synchronized
MetadataModel<org.netbeans.modules.jakarta.web.beans.api.model.WebBeansModel>
getJakartaWebBeansModel() {
+ System.out.println("getting getJakartaWebBeansModel");
Review Comment:
Another `System.out.println`.
##########
enterprise/web.jsf/src/org/netbeans/modules/web/jsf/JSFUtils.java:
##########
@@ -325,6 +325,23 @@ public static boolean isJavaEE5(TemplateWizard wizard) {
}
return false;
}
+
+ public static boolean isJakartaEE9Plus(TemplateWizard wizard) {
+ Project project = Templates.getProject(wizard);
+ WebModule wm = WebModule.getWebModule(project.getProjectDirectory());
+ if (wm != null) {
+ Profile profile = wm.getJ2eeProfile();
+ if (profile == Profile.JAKARTA_EE_9_WEB ||
+ profile == Profile.JAKARTA_EE_9_FULL ||
+ profile == Profile.JAKARTA_EE_9_1_WEB ||
+ profile == Profile.JAKARTA_EE_9_1_FULL ||
+ profile == Profile.JAKARTA_EE_10_WEB ||
+ profile == Profile.JAKARTA_EE_10_FULL) {
+ return true;
Review Comment:
Would it make sense to simplify this to:
```java
return profile.isAtLeast(Profile.JAKARTA_EE_9_WEB);
```
I have some hope that either there is not another name transfer necessary or
that the Eclipse Foundation has more common sense, than Oracle regarding
package and namespace naming.
##########
enterprise/web.jsf/src/org/netbeans/modules/web/jsf/wizards/ManagedBeanPanelVisual.java:
##########
@@ -93,8 +95,13 @@ public ManagedBeanPanelVisual(Project proj) {
}
}
Object[] scopes;
- CdiUtil cdiUtil = proj.getLookup().lookup(CdiUtil.class);
- isCDIEnabled = cdiUtil != null && cdiUtil.isCdiEnabled();
+ if(profile != null && (profile.isAtLeast(Profile.JAKARTA_EE_9_WEB) ||
profile.isAtLeast(Profile.JAKARTA_EE_9_FULL))){
Review Comment:
The check for the full profile should not be necessary.
- 9 is it in both cases
- `WEB` is lower than `FULL`
```java
if(profile != null && profile.isAtLeast(Profile.JAKARTA_EE_9_WEB)){
```
##########
enterprise/web.jsf/src/org/netbeans/modules/web/jsf/palette/items/ManagedBeanCustomizer.java:
##########
@@ -113,7 +113,12 @@ public void contentsChanged(ListDataEvent e) {
}
});
this.project = project;
- this.metaModelSupport = new MetaModelSupport(project);
+ if(JSFVersion.forProject(project) == JSFVersion.JSF_4_0){
Review Comment:
To prepare for the future I suggest (same in 375):
```java
JSFVersion projectJsfVersion = JSFVersion.forProject(project);
if(projectJsfVersion != null &&
projectJsfVersion.isAtLeast(JSFVersion.JSF_4_0)){
```
##########
nbbuild/cluster.properties:
##########
@@ -763,6 +762,7 @@ nb.cluster.enterprise=\
j2ee.sun.ddui,\
j2eeapis,\
j2eeserver,\
+ jakarta.web.beans,\
Review Comment:
Why did you remove `languages.hcl`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
