Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Close #91 (UselessCode FP) #116

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KengoTODA
Copy link
Member

This PR contains two changes:

  • reproduced problem by JUnit
  • solve problem by changing ASSUME_ASSERTIONS_ENABLED to false

The 2nd change would affect other detectors, I need to check carefully.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 56.697% when pulling b5b03c1 on KengoTODA:issue-91 into b3f3117 on spotbugs:master.

@KengoTODA
Copy link
Member Author

As 362c9fb (not in this branch) I created a sub-class of CFG to let specific detector ignores assert, but it failed.
It seems that the problem exists in product architecture: analysis cache doesn't suppose the possibility to have multiple kinds of CFG. I think it's really hard to introduce this way which can keep backward compatibility of other detectors.

So we should choose other methodology, for instance:

  • Use SystemProperties.ASSERTIONS_ENABLED instead of ASSUME_ASSERTIONS_ENABLED
  • Create an option (system properties?) to switch the value of ASSUME_ASSERTIONS_ENABLED

Note that both of above affects all detectors which depends on CFG.
Any other ideas are welcome :)

@hazendaz hazendaz marked this pull request as draft March 4, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants