You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[BUG] SecurityManager is deprecated for removal and spotbugs complains about it during build #7079
Describe the bug
There are ~36 situations where a SecurityManager is being constructed in the current code base. This class is deprecated and will likely go away very soon (it was deprecated prior to Java 17, then warnings started being issued, and it's likely to go away no later than 24).
To Reproduce
Attempt to run ./gradlew spotbugs and watch the error messages scroll. 🙂
Expected behavior
The build should finish without errors.
Desktop (please complete the following information):
OS: Any
Version Any
Additional context
All references to the SecurityManager are from the SystemExitInterloper (or is it Intercepter?) class. This class is a kludge to allow a test to call argparse4j.parseArgsOrFail() and for the error to be caught.
However, the latest argparse4j has a method named parseArgs() that throws an IllegalArgumentException instead. This is how the tests should be written.
(I have made proof of concept changes to Main, CommandLine, and CommandLineTest, but I have not pushed them to my repo yet and won't be able to do so for another 24 hours or so. It's at https://github.com/Azhrei/pcgen under the sm-fix2 branch.)
I can't seem to assign this to myself (GitHub isn't providing the panel on the right that usually allows labels, assignations, projects, and so forth). But I should have time this coming week to work on it. I'll do a few and try to give an ETA on completion.
Status update: I found two NPEs during slowtest that were the result of my change to the base code. Both will be included in this eventual PR as they're part of this same problem.
Another update. I'm correcting the SecurityManager stuff, but as I touch a given file, I'm trying to cleanup any warnings that IntelliJ is giving me. Some of these source files have a huge number of warnings to go with them.
For example, some of the tests have an assertTrue() or assertFalse() that is testing to ensure a reference is non-null, then the reference is used later in the code — but IntelliJ complains about a possible null because it doesn't know that the assert doesn't return under some circumstances. For those issues, the recommendation in the IDE is to add a Java language assert. That's what I've been doing, on the assumption that the tests aren't production code and we don't care if an extra assert has to run once in awhile.
Activity
Azhrei commentedon Apr 8, 2024
I can't seem to assign this to myself (GitHub isn't providing the panel on the right that usually allows labels, assignations, projects, and so forth). But I should have time this coming week to work on it. I'll do a few and try to give an ETA on completion.
Azhrei commentedon Apr 13, 2024
Status update: I found two NPEs during
slowtestthat were the result of my change to the base code. Both will be included in this eventual PR as they're part of this same problem.Azhrei commentedon Apr 22, 2024
Another update. I'm correcting the SecurityManager stuff, but as I touch a given file, I'm trying to cleanup any warnings that IntelliJ is giving me. Some of these source files have a huge number of warnings to go with them.
For example, some of the tests have an assertTrue() or assertFalse() that is testing to ensure a reference is non-
null, then the reference is used later in the code — but IntelliJ complains about a possiblenullbecause it doesn't know that the assert doesn't return under some circumstances. For those issues, the recommendation in the IDE is to add a Java languageassert. That's what I've been doing, on the assumption that the tests aren't production code and we don't care if an extra assert has to run once in awhile.