Skip to content

[BUG] SecurityManager is deprecated for removal and spotbugs complains about it during build #7079

Open

Description

@Azhrei

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

Activity

  1. Azhrei commented on Apr 8, 2024

    @Azhrei
    ContributorAuthor

    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.

  2. Azhrei commented on Apr 13, 2024

    @Azhrei
    ContributorAuthor

    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.

  3. Azhrei commented on Apr 22, 2024

    @Azhrei
    ContributorAuthor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions