You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "pedro-w (via GitHub)" <gi...@apache.org> on 2025/07/20 19:58:38 UTC
[PR] Issue 8560 [netbeans]
pedro-w opened a new pull request, #8662:
URL: https://github.com/apache/netbeans/pull/8662
When getting the module definition from a `module-info.java` file, use `getModule` rather than scanning through `getTypeDecls`.
In the compiler API, the latter used to work up until JDK16, then it was changed for subsequent releases, so that the module object is no longer considered a type declaration. See discussion in #8560.
Tests included (there were no tests for this previously). I have not squashed them so it's possible to see the tests run without the fix. Please advise.
Fixes #8560
Fixes #8614
---
<details open>
<summary>Click to collapse/expand PR instructions</summary>
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
- are all your own work, and you have the right to contribute them.
- are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
### PR approval and merge checklist:
1. [ ] Was this PR [correctly labeled](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240884239#PRsandYouAreviewerGuide-PRtriggeredCIJobs(conditionalCIpipeline)), did the right tests run? When did they run?
2. [ ] Is this PR [squashed](https://cwiki.apache.org/confluence/display/NETBEANS/git%3A+squash+and+merge)?
3. [ ] Are author name / email address correct? Are [co-authors](https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line) correctly listed? Do the commit messages need updates?
3. [ ] Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?
If this PR targets the delivery branch: [don't merge](https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery). ([full wiki article](https://cwiki.apache.org/confluence/display/NETBEANS/PRs+and+You+-+A+reviewer+Guide))
</details>
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2297404557
##########
java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImplTest.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.java.api.common.queries;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.netbeans.api.java.queries.AccessibilityQuery.Accessibility;
+import org.netbeans.api.project.Project;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.modules.java.api.common.TestProject;
+import org.netbeans.modules.java.api.common.ant.UpdateHelper;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation2.Result;
+import org.netbeans.spi.project.support.ant.AntProjectHelper;
+import org.netbeans.spi.project.support.ant.EditableProperties;
+import org.openide.filesystems.FileObject;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.test.MockLookup;
+
+/**
+ *
+ * @author peterhull
+ */
+public class ModuleInfoAccessibilityQueryImplTest extends NbTestCase {
+
+ private static final String MODULE_INFO_JAVA = "module-info.java"; //NOI18N
+
+ // Module definition which exports mymodule but not myinternal
+ private static final String MODULE_INFO = """
+ module mymodule {
+ exports mypackage;
+ }
+ """;
+
+ private FileObject src;
+ private FileObject test;
+ private FileObject workDir;
+ private TestProject testProject;
+ private FileObject mypackage;
+ private FileObject myinternal;
+
+ public ModuleInfoAccessibilityQueryImplTest(String name) {
+ super(name);
+ }
+
+ @Before
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ clearWorkDir();
+ MockLookup.setInstances(TestProject.createProjectType());
+ workDir = FileUtil.toFileObject(FileUtil.normalizeFile(getWorkDir()));
+ src = workDir.createFolder("src"); //NOI18N
+ test = workDir.createFolder("test"); //NOI18N
+ mypackage = src.createFolder("mypackage"); //NOI18N
+ myinternal = src.createFolder("myinternal"); //NOI18N
+ try (OutputStream os = src.createAndOpen(MODULE_INFO_JAVA)) {
+ os.write(MODULE_INFO.getBytes(StandardCharsets.UTF_8));
+ }
+ Project prj = TestProject.createProject(workDir, src, test);
+ testProject = prj.getLookup().lookup(TestProject.class);
+ // Using modules, so project has to be source level 9 or later.
+ UpdateHelper helper = testProject.getUpdateHelper();
+ EditableProperties properties = helper.getProperties(AntProjectHelper.PROJECT_PROPERTIES_PATH);
+ properties.putAll(Map.of("javac.source", "9", "javac.target", "9"));
+ helper.putProperties(AntProjectHelper.PROJECT_PROPERTIES_PATH, properties);
+ /* Modular project has the structure
+ ROOT
+ +-src
+ | +-mypackage
+ | | `-*.java
+ | +-myinternal
+ | + `-*.java
+ | `-module-info.java
+ `-test
+ `-mypackage
+ `-MyClassTest.java
+ */
+ }
+
+ /**
+ * Test of isPubliclyAccessible method, of class
+ * ModuleInfoAccessibilityQueryImpl. Tests if an exported package
+ * 'mypackage' is accessible.
+ *
+ * @throws java.io.IOException if test file creation failed.
+ */
+ @Test
+ public void testIsPubliclyAccessible1() throws IOException {
+ ModuleInfoAccessibilityQueryImpl instance = new ModuleInfoAccessibilityQueryImpl(null, testProject.getSourceRoots(), null, testProject.getTestRoots());
+ Result result = instance.isPubliclyAccessible(mypackage);
+ // Return value can be null, that's a fail
+ if (result == null) {
+ fail("AccessibilityQuery returned null"); //NOI18N
+ } else {
+ Accessibility accessibility = result.getAccessibility();
+ Accessibility expAccessibility = Accessibility.EXPORTED;
+ assertEquals(expAccessibility, accessibility);
+ }
+ }
+
+ /**
+ * Test of isPubliclyAccessible method, of class
+ * ModuleInfoAccessibilityQueryImpl. Tests if a not-exported package
+ * 'myinternal' is not accessible.
+ *
+ * @throws java.io.IOException if test file creation failed.
+ */
+ @Test
+ public void testIsPubliclyAccessible2() throws IOException {
+ ModuleInfoAccessibilityQueryImpl instance = new ModuleInfoAccessibilityQueryImpl(null, testProject.getSourceRoots(), null, testProject.getTestRoots());
+ Result result = instance.isPubliclyAccessible(myinternal);
+ // Return value can be null, that's a fail
+ if (result == null) {
+ fail("AccessibilityQuery returned null"); //NOI18N
+ } else {
+ Accessibility accessibility = result.getAccessibility();
+ Accessibility expAccessibility = Accessibility.PRIVATE;
+ assertEquals(expAccessibility, accessibility);
+ }
+ }
+
+ /**
+ * Test of propertyChange method, of class ModuleInfoAccessibilityQueryImpl.
+ */
+ @Test
+ public void testPropertyChange() {
+ // This doesn't need testing
+ }
Review Comment:
done
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#issuecomment-3219252476
I do apologize, something got lost somewhere along the way. Should now be fixed. I don't know why CI didn't run tests. All tests on that module are passing locally.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2302894631
##########
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:
> while Set.of() will throw IAE in that case.
That's something new I learned today!
I also think the Set is being used to de-duplicate. I will put that back to using a HashSet.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien merged PR #8662:
URL: https://github.com/apache/netbeans/pull/8662
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#issuecomment-3222432268
awesome, thanks!
could you:
- rebase to latest master (git pull --rebase),
- then remove all `@Ignore` annotations from `UnitTestsCompilerOptionsQueryImplTest` in `java/java.api.common`
- and squash everything to one commit and force push?
It should now be all green.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#issuecomment-3227216946
ok 🤞
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#issuecomment-3218601997
on second look: I don't think this will build. `extractExports()` is also not used atm so something is missing in this changeset.
Next sync should trigger CI.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2297398843
##########
java/java.api.common/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImpl.java:
##########
@@ -46,6 +49,7 @@
import org.netbeans.api.java.source.JavaSource;
import org.netbeans.modules.java.api.common.SourceRoots;
import org.netbeans.modules.java.api.common.impl.MultiModule;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation;
Review Comment:
Correct, it was mentioned in a Javadoc comment, that's why NB didn't flag it as unused.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2297404148
##########
java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImplTest.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.java.api.common.queries;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.netbeans.api.java.queries.AccessibilityQuery.Accessibility;
+import org.netbeans.api.project.Project;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.modules.java.api.common.TestProject;
+import org.netbeans.modules.java.api.common.ant.UpdateHelper;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation2.Result;
+import org.netbeans.spi.project.support.ant.AntProjectHelper;
+import org.netbeans.spi.project.support.ant.EditableProperties;
+import org.openide.filesystems.FileObject;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.test.MockLookup;
+
+/**
+ *
+ * @author peterhull
+ */
+public class ModuleInfoAccessibilityQueryImplTest extends NbTestCase {
+
+ private static final String MODULE_INFO_JAVA = "module-info.java"; //NOI18N
+
+ // Module definition which exports mymodule but not myinternal
+ private static final String MODULE_INFO = """
+ module mymodule {
+ exports mypackage;
+ }
+ """;
Review Comment:
done
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "pedro-w (via GitHub)" <gi...@apache.org>.
pedro-w commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2297403267
##########
java/java.api.common/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImpl.java:
##########
@@ -232,48 +236,49 @@ 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)
+ 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)
+ .map(mi -> {
+ Set<FileObject> rootsSet = new HashSet<>();
+ Collections.addAll(rootsSet, roots);
+ Set<FileObject> exportsSet = readExports(mi, rootsSet);
+ return Pair.of(rootsSet, exportsSet);
+ })
.findFirst();
Review Comment:
There isn't a constructor for HashSet` that takes an array (I don't think?) so I used `Set.of` instead - AFAICS the set is only ever read from so this is OK.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix getting package export status in a modular project [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8662:
URL: https://github.com/apache/netbeans/pull/8662#discussion_r2296884183
##########
java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImplTest.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.java.api.common.queries;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.netbeans.api.java.queries.AccessibilityQuery.Accessibility;
+import org.netbeans.api.project.Project;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.modules.java.api.common.TestProject;
+import org.netbeans.modules.java.api.common.ant.UpdateHelper;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation2.Result;
+import org.netbeans.spi.project.support.ant.AntProjectHelper;
+import org.netbeans.spi.project.support.ant.EditableProperties;
+import org.openide.filesystems.FileObject;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.test.MockLookup;
+
+/**
+ *
+ * @author peterhull
+ */
+public class ModuleInfoAccessibilityQueryImplTest extends NbTestCase {
+
+ private static final String MODULE_INFO_JAVA = "module-info.java"; //NOI18N
+
+ // Module definition which exports mymodule but not myinternal
+ private static final String MODULE_INFO = """
+ module mymodule {
+ exports mypackage;
+ }
+ """;
+
+ private FileObject src;
+ private FileObject test;
+ private FileObject workDir;
+ private TestProject testProject;
+ private FileObject mypackage;
+ private FileObject myinternal;
+
+ public ModuleInfoAccessibilityQueryImplTest(String name) {
+ super(name);
+ }
+
+ @Before
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ clearWorkDir();
+ MockLookup.setInstances(TestProject.createProjectType());
+ workDir = FileUtil.toFileObject(FileUtil.normalizeFile(getWorkDir()));
+ src = workDir.createFolder("src"); //NOI18N
+ test = workDir.createFolder("test"); //NOI18N
+ mypackage = src.createFolder("mypackage"); //NOI18N
+ myinternal = src.createFolder("myinternal"); //NOI18N
+ try (OutputStream os = src.createAndOpen(MODULE_INFO_JAVA)) {
+ os.write(MODULE_INFO.getBytes(StandardCharsets.UTF_8));
+ }
+ Project prj = TestProject.createProject(workDir, src, test);
+ testProject = prj.getLookup().lookup(TestProject.class);
+ // Using modules, so project has to be source level 9 or later.
+ UpdateHelper helper = testProject.getUpdateHelper();
+ EditableProperties properties = helper.getProperties(AntProjectHelper.PROJECT_PROPERTIES_PATH);
+ properties.putAll(Map.of("javac.source", "9", "javac.target", "9"));
+ helper.putProperties(AntProjectHelper.PROJECT_PROPERTIES_PATH, properties);
+ /* Modular project has the structure
+ ROOT
+ +-src
+ | +-mypackage
+ | | `-*.java
+ | +-myinternal
+ | + `-*.java
+ | `-module-info.java
+ `-test
+ `-mypackage
+ `-MyClassTest.java
+ */
+ }
+
+ /**
+ * Test of isPubliclyAccessible method, of class
+ * ModuleInfoAccessibilityQueryImpl. Tests if an exported package
+ * 'mypackage' is accessible.
+ *
+ * @throws java.io.IOException if test file creation failed.
+ */
+ @Test
+ public void testIsPubliclyAccessible1() throws IOException {
+ ModuleInfoAccessibilityQueryImpl instance = new ModuleInfoAccessibilityQueryImpl(null, testProject.getSourceRoots(), null, testProject.getTestRoots());
+ Result result = instance.isPubliclyAccessible(mypackage);
+ // Return value can be null, that's a fail
+ if (result == null) {
+ fail("AccessibilityQuery returned null"); //NOI18N
+ } else {
+ Accessibility accessibility = result.getAccessibility();
+ Accessibility expAccessibility = Accessibility.EXPORTED;
+ assertEquals(expAccessibility, accessibility);
+ }
+ }
+
+ /**
+ * Test of isPubliclyAccessible method, of class
+ * ModuleInfoAccessibilityQueryImpl. Tests if a not-exported package
+ * 'myinternal' is not accessible.
+ *
+ * @throws java.io.IOException if test file creation failed.
+ */
+ @Test
+ public void testIsPubliclyAccessible2() throws IOException {
+ ModuleInfoAccessibilityQueryImpl instance = new ModuleInfoAccessibilityQueryImpl(null, testProject.getSourceRoots(), null, testProject.getTestRoots());
+ Result result = instance.isPubliclyAccessible(myinternal);
+ // Return value can be null, that's a fail
+ if (result == null) {
+ fail("AccessibilityQuery returned null"); //NOI18N
+ } else {
+ Accessibility accessibility = result.getAccessibility();
+ Accessibility expAccessibility = Accessibility.PRIVATE;
+ assertEquals(expAccessibility, accessibility);
+ }
+ }
+
+ /**
+ * Test of propertyChange method, of class ModuleInfoAccessibilityQueryImpl.
+ */
+ @Test
+ public void testPropertyChange() {
+ // This doesn't need testing
+ }
Review Comment:
please remove the empty test methods
##########
java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImplTest.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.java.api.common.queries;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.netbeans.api.java.queries.AccessibilityQuery.Accessibility;
+import org.netbeans.api.project.Project;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.modules.java.api.common.TestProject;
+import org.netbeans.modules.java.api.common.ant.UpdateHelper;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation2.Result;
+import org.netbeans.spi.project.support.ant.AntProjectHelper;
+import org.netbeans.spi.project.support.ant.EditableProperties;
+import org.openide.filesystems.FileObject;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.test.MockLookup;
+
+/**
+ *
+ * @author peterhull
+ */
+public class ModuleInfoAccessibilityQueryImplTest extends NbTestCase {
+
+ private static final String MODULE_INFO_JAVA = "module-info.java"; //NOI18N
+
+ // Module definition which exports mymodule but not myinternal
+ private static final String MODULE_INFO = """
+ module mymodule {
+ exports mypackage;
+ }
+ """;
Review Comment:
nitpick: align `"""`
##########
java/java.api.common/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImpl.java:
##########
@@ -232,48 +236,49 @@ 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)
+ 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)
+ .map(mi -> {
+ Set<FileObject> rootsSet = new HashSet<>();
+ Collections.addAll(rootsSet, roots);
+ Set<FileObject> exportsSet = readExports(mi, rootsSet);
+ return Pair.of(rootsSet, exportsSet);
+ })
.findFirst();
Review Comment:
this is fine but a bit dangerous in case someone changes something in future (e.g adds sort or something like that).
moving the `map()` stage after `findFirst()` would be safer.
nitpick:
copy constructor candidate:
```java
Set<FileObject> rootsSet = new HashSet<>(roots);
```
##########
java/java.api.common/src/org/netbeans/modules/java/api/common/queries/ModuleInfoAccessibilityQueryImpl.java:
##########
@@ -46,6 +49,7 @@
import org.netbeans.api.java.source.JavaSource;
import org.netbeans.modules.java.api.common.SourceRoots;
import org.netbeans.modules.java.api.common.impl.MultiModule;
+import org.netbeans.spi.java.queries.AccessibilityQueryImplementation;
Review Comment:
import is probably not needed since `AccessibilityQueryImplementation2` is in use looks like
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists