mbien commented on code in PR #6160: URL: https://github.com/apache/netbeans/pull/6160#discussion_r1257254787
########## enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/JarFileFaceletLibrary.java: ########## Review Comment: class unused? ########## enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/MavenJsfReferenceImplementationProvider.java: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.maven.j2ee; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.EnumMap; +import java.util.Map; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.resolver.ArtifactNotFoundException; +import org.apache.maven.artifact.resolver.ArtifactResolutionException; +import static org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_ID; +import static org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_URL; +import org.netbeans.modules.maven.embedder.EmbedderFactory; +import org.netbeans.modules.maven.embedder.MavenEmbedder; +import org.netbeans.modules.web.jsfapi.api.JsfVersion; +import org.netbeans.modules.web.jsfapi.spi.JsfReferenceImplementationProvider; +import org.openide.util.Exceptions; +import org.openide.util.lookup.ServiceProvider; + +/** + * + * @author Benjamin Asbach + */ +@ServiceProvider(service = JsfReferenceImplementationProvider.class) +public class MavenJsfReferenceImplementationProvider implements JsfReferenceImplementationProvider { + + static final Map<JsfVersion, String> JSF_VERSION_MAVEN_COORDINATES_MAPPING; + static { + EnumMap<JsfVersion, String> map = new EnumMap<>(JsfVersion.class); + map.put(JsfVersion.JSF_1_0, "javax.faces:jsf-impl:1.1_02"); //seems to be not available in maven central + map.put(JsfVersion.JSF_1_1, "javax.faces:jsf-impl:1.1_02"); + map.put(JsfVersion.JSF_1_2, "javax.faces:jsf-impl:1.2"); + map.put(JsfVersion.JSF_2_0, "com.sun.faces:jsf-impl:2.0.11"); + map.put(JsfVersion.JSF_2_1, "com.sun.faces:jsf-impl:2.1.29"); + map.put(JsfVersion.JSF_2_2, "com.sun.faces:jsf-impl:2.2.20"); + map.put(JsfVersion.JSF_2_3, "org.glassfish:jakarta.faces:2.3.19"); + map.put(JsfVersion.JSF_3_0, "org.glassfish:jakarta.faces:3.0.4"); + map.put(JsfVersion.JSF_4_0, "org.glassfish:jakarta.faces:4.0.2"); + JSF_VERSION_MAVEN_COORDINATES_MAPPING = Collections.unmodifiableMap(map); + } + + @Override + public Path of(JsfVersion jsfVersion) { + String mavenCoordinates[] = JSF_VERSION_MAVEN_COORDINATES_MAPPING.get(jsfVersion).split(":"); + if (mavenCoordinates.length != 3) { + return null; + } + String groupId = mavenCoordinates[0]; + String artifactId = mavenCoordinates[1]; + String version = mavenCoordinates[2]; + + MavenEmbedder mavenEmbedder = EmbedderFactory.getOnlineEmbedder(); + + ArtifactRepository localRepository = mavenEmbedder.getLocalRepository(); + ArtifactRepository remoteRepository = mavenEmbedder.createRemoteRepository(DEFAULT_REMOTE_REPO_URL, DEFAULT_REMOTE_REPO_ID); + Artifact jsfRIArtifact = mavenEmbedder.createArtifact(groupId, artifactId, version, "jar"); + + try { + mavenEmbedder.resolve(jsfRIArtifact, Collections.singletonList(remoteRepository), localRepository); Review Comment: note to self: this is going to be fairly expensive esp if the artifact isn't in the local repo yet. We should check that this is not called from EDT. Might be a candidate for a cache. ########## enterprise/web.jsf/src/org/netbeans/modules/web/jsf/api/facesmodel/JsfVersionUtils.java: ########## @@ -58,48 +59,31 @@ * * @author Petr Pisl, ads, Martin Fousek */ -public enum JSFVersion { - JSF_1_0("JSF 1.0"), - JSF_1_1("JSF 1.1"), - JSF_1_2("JSF 1.2"), - JSF_2_0("JSF 2.0"), - JSF_2_1("JSF 2.1"), - JSF_2_2("JSF 2.2"), - JSF_2_3("JSF 2.3"), - JSF_3_0("JSF 3.0"), - JSF_4_0("JSF 4.0"); +public final class JsfVersionUtils { - private static final LinkedHashMap<JSFVersion, String> SPECIFIC_CLASS_NAMES = new LinkedHashMap<>(); + private static final LinkedHashMap<JsfVersion, String> SPECIFIC_CLASS_NAMES = new LinkedHashMap<>(); static { - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_4_0, JSFUtils.JSF_4_0__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_3_0, JSFUtils.JSF_3_0__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_2_3, JSFUtils.JSF_2_3__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_2_2, JSFUtils.JSF_2_2__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_2_1, JSFUtils.JSF_2_1__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_2_0, JSFUtils.JSF_2_0__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_1_2, JSFUtils.JSF_1_2__API_SPECIFIC_CLASS); - SPECIFIC_CLASS_NAMES.put(JSFVersion.JSF_1_1, JSFUtils.FACES_EXCEPTION); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_4_0, JSFUtils.JSF_4_0__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_3_0, JSFUtils.JSF_3_0__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_2_3, JSFUtils.JSF_2_3__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_2_2, JSFUtils.JSF_2_2__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_2_1, JSFUtils.JSF_2_1__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_2_0, JSFUtils.JSF_2_0__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_1_2, JSFUtils.JSF_1_2__API_SPECIFIC_CLASS); + SPECIFIC_CLASS_NAMES.put(JsfVersion.JSF_1_1, JSFUtils.FACES_EXCEPTION); } - private final String shortName; - - - private JSFVersion(String shortName) { - this.shortName = shortName; - } - - public String getShortName() { - return shortName; - } - - private static final RequestProcessor RP = new RequestProcessor(JSFVersion.class); - private static final Logger LOG = Logger.getLogger(JSFVersion.class.getName()); + private static final RequestProcessor RP = new RequestProcessor(JsfVersion.class); Review Comment: RP doesn't seem to be used anywhere and can be removed ########## enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/FaceletsLibrarySupport.java: ########## @@ -358,38 +361,66 @@ public Collection<URI> getResources(ServletContext sc) { } }); - //3. last add a provider for default jsf libs - // - //Add a facelet taglib provider which provides the libraries from - //netbeans jsf2.0 library - // - //This is needed for the standart JSF 2.0 libraries since it may - //happen that there is no javax-faces.jar with the .taglib.xml files - //on the compile classpath and we still want the features like code - //completion work. This happens for example in Maven web projects. - // - //The provider is last in the list so the provided libraries will - //be overridden if the descriptors are found in any of the jars - //on compile classpath. - Collection<FileObject> libraryDescriptorFiles = DefaultFaceletLibraries.getInstance().getLibrariesDescriptorsFiles(); - final Collection<URI> libraryURIs = new ArrayList<>(); - for(FileObject fo : libraryDescriptorFiles) { - try { - libraryURIs.add(fo.toURL().toURI()); - } catch (URISyntaxException ex) { - LOGGER.log(Level.INFO, null, ex); + + // try to find reference implementation based on jsf version + JsfVersion jsfVersion = getJsfSupport().getJsfVersion(); + JsfReferenceImplementationProvider jsfRIProvider = Lookup.getDefault().lookup(JsfReferenceImplementationProvider.class); + Path jsfReferenceImplementation = jsfRIProvider.of(jsfVersion); + + List<URL> jsfRIJars = new ArrayList<>(); + try { + if (jsfReferenceImplementation != null) { + DefaultFaceletLibraries jsfRIFaceletLibraries = new DefaultFaceletLibraries(jsfReferenceImplementation.toFile()); + List<URI> jsfRIDescriptors = jsfRIFaceletLibraries.getLibrariesDescriptorsFiles().stream() + .map(FileObject::toURI) + .collect(Collectors.toList()); + faceletTaglibProviders.add(new ConfigurationResourceProvider() { + @Override + public Collection<URI> getResources(ServletContext sc) { + return jsfRIDescriptors; + } + }); Review Comment: lambda candidate: ` faceletTaglibProviders.add((sc) -> jsfRIDescriptors);` Same in L356 and L406 ########## enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/spi/JsfReferenceImplementationProvider.java: ########## @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.web.jsfapi.spi; + +import java.nio.file.Path; +import org.netbeans.modules.web.jsfapi.api.JsfVersion; + +/** + * + * @author Benjamin Asbach + */ +public interface JsfReferenceImplementationProvider { + + Path of(JsfVersion jsfVersion); Review Comment: the method is a bit mysterious :) I think it deserves a longer name. `artifactPathFor`, `implPathFor` or even `findImpl` potentially? ########## enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/hints/LibraryDeclarationChecker.java: ########## @@ -238,43 +246,45 @@ public void visit(Element node) { final boolean[] passthroughUsage = new boolean[1]; final Collection<OffsetRange> ranges = new ArrayList<>(); for (Library lib : declaredLibraries) { - Node rootNode = result.root(lib.getNamespace()); - if (lib.getLegacyNamespace() != null && (rootNode == null || rootNode.children().isEmpty())) { - rootNode = result.root(lib.getLegacyNamespace()); - } - if (rootNode == null) { + List<Node> nodes = lib.getValidNamespaces().stream() + .map(result::root) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + if (nodes.isEmpty()) { continue; //no parse result for this namespace, the namespace is not declared } final int[] usages = new int[1]; - ElementUtils.visitChildren(rootNode, new ElementVisitor() { - @Override - public void visit(Element node) { - usages[0]++; - if (declaredPassthroughOrJsf) { - OpenTag ot = (OpenTag) node; - for (Attribute attribute : ot.attributes(new AttributeFilter() { - @Override - public boolean accepts(Attribute attribute) { - return attribute.namespacePrefix() != null; - } - })) { - if (passthroughNsPrefix != null && LexerUtils.equals(passthroughNsPrefix, attribute.namespacePrefix(), true, true)) { - // http://java.sun.com/jsf/passthrough or http://xmlns.jcp.org/jsf/passthrough used - passthroughUsage[0] = true; - } else if (jsfNsPrefix != null && ot.namespacePrefix() != null - && LexerUtils.equals(jsfNsPrefix, attribute.namespacePrefix(), true, true)) { - // http://java.sun.com/jsf used at JSF-aware tag - wrongJsfNsUsages.add(attribute); + for (Node rootNode : nodes) { + ElementUtils.visitChildren(rootNode, new ElementVisitor() { + @Override + public void visit(Element node) { + usages[0]++; + if (declaredPassthroughOrJsf) { + OpenTag ot = (OpenTag) node; + for (Attribute attribute : ot.attributes(new AttributeFilter() { + @Override + public boolean accepts(Attribute attribute) { Review Comment: two lambda candidates here if you want ########## enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/MavenJsfReferenceImplementationProvider.java: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.maven.j2ee; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.EnumMap; +import java.util.Map; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.resolver.ArtifactNotFoundException; +import org.apache.maven.artifact.resolver.ArtifactResolutionException; +import static org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_ID; +import static org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_URL; +import org.netbeans.modules.maven.embedder.EmbedderFactory; +import org.netbeans.modules.maven.embedder.MavenEmbedder; +import org.netbeans.modules.web.jsfapi.api.JsfVersion; +import org.netbeans.modules.web.jsfapi.spi.JsfReferenceImplementationProvider; +import org.openide.util.Exceptions; +import org.openide.util.lookup.ServiceProvider; + +/** + * + * @author Benjamin Asbach + */ +@ServiceProvider(service = JsfReferenceImplementationProvider.class) +public class MavenJsfReferenceImplementationProvider implements JsfReferenceImplementationProvider { + + static final Map<JsfVersion, String> JSF_VERSION_MAVEN_COORDINATES_MAPPING; + static { + EnumMap<JsfVersion, String> map = new EnumMap<>(JsfVersion.class); + map.put(JsfVersion.JSF_1_0, "javax.faces:jsf-impl:1.1_02"); //seems to be not available in maven central + map.put(JsfVersion.JSF_1_1, "javax.faces:jsf-impl:1.1_02"); + map.put(JsfVersion.JSF_1_2, "javax.faces:jsf-impl:1.2"); + map.put(JsfVersion.JSF_2_0, "com.sun.faces:jsf-impl:2.0.11"); + map.put(JsfVersion.JSF_2_1, "com.sun.faces:jsf-impl:2.1.29"); + map.put(JsfVersion.JSF_2_2, "com.sun.faces:jsf-impl:2.2.20"); + map.put(JsfVersion.JSF_2_3, "org.glassfish:jakarta.faces:2.3.19"); + map.put(JsfVersion.JSF_3_0, "org.glassfish:jakarta.faces:3.0.4"); + map.put(JsfVersion.JSF_4_0, "org.glassfish:jakarta.faces:4.0.2"); + JSF_VERSION_MAVEN_COORDINATES_MAPPING = Collections.unmodifiableMap(map); + } + + @Override + public Path of(JsfVersion jsfVersion) { + String mavenCoordinates[] = JSF_VERSION_MAVEN_COORDINATES_MAPPING.get(jsfVersion).split(":"); Review Comment: `[]` to the type please. ########## enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/api/JsfVersion.java: ########## @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.web.jsfapi.api; + +import org.netbeans.api.annotations.common.NonNull; + +/** + * + * @author Benjamin Asbach + */ +public enum JsfVersion { Review Comment: what happens if you rename the class back to JSFVersion? Looks like this would create a smaller patch. JsfVersionUtils is in a package with 5 classes starting with capital JSF* https://github.com/apache/netbeans/blob/f2528fc85c22c919d245a96427b2803d98ca73d4/enterprise/web.jsf/src/org/netbeans/modules/web/jsf/api/facesmodel/JsfVersionUtils.java Maybe continuing with capital JSF is a better choice here? -- 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
