mbien commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2302307701


##########
java/java.api.common/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImpl.java:
##########
@@ -232,48 +236,48 @@ private Collection<FileObject[]> collectModuleRoots(
         final MultiModule model = MultiModule.getOrCreate(mods, src);
         for (String modName : model.getModuleNames()) {
             final ClassPath cp = model.getModuleSources(modName);
-            res.add(cp.getRoots());
+            if (cp != null) {
+                res.add(cp.getRoots());
+            }
         }
         return res;
     }
 
     @NonNull
-    private static Optional<Pair<Set<FileObject>,Set<FileObject>>> readExports(
-            @NonNull final FileObject[] roots,
-            @NonNull final Set<? super FileObject> rootsCollector) {
-        Collections.addAll(rootsCollector, roots);
-        final Optional<FileObject> moduleInfo = Arrays.stream(roots)
-                .map((root) -> root.getFileObject(MODULE_INFO_JAVA))
-                .filter((mi) -> mi != null)
-                .findFirst();
-        if (!moduleInfo.isPresent()) {
-            return Optional.empty();
-        }
-        final Set<FileObject> rootsSet = new HashSet<>();
-        Collections.addAll(rootsSet, roots);
-        final Set<FileObject> exportsSet = readExports(moduleInfo.get(), 
rootsSet);
-        return Optional.of(Pair.of(rootsSet, exportsSet));
+    private static Optional<Pair<Set<FileObject>, Set<FileObject>>> 
extractExports(
+            @NonNull final FileObject[] roots) {
+        return Arrays.stream(roots)
+                .map(root -> root.getFileObject(MODULE_INFO_JAVA))
+                .filter(Objects::nonNull)
+                .findFirst()
+                .map(mi -> {
+                    Set<FileObject> rootsSet = Set.of(roots);

Review Comment:
   I am not 100% certain if roots are allowed to have duplicated elements. The 
original code used `HashSet`. 
   
   Sometimes sets are used to deduplicate elements, while `Set.of()` will throw 
IAE in that case. It would probably be better to switch back to `HashSet` here 
- sorry that I didn't see that earlier.
   
   (the usages only iterate over the set without using  `contains` etc which is 
extra suspicious since this could have been a list or array if there was no 
intention to deduplicate anything)



-- 
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

Reply via email to