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

Add failing test case for UWF_UNWRITTEN_FIELD false negative #1042

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

granthenke
Copy link

Adds a test case to show a false negative for UWF_UNWRITTEN_FIELD
when assert is used or when Slf4j’s LoggerFactory.getLogger(Foo.class)
is used.


Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

Adds a test case to show a false negative for UWF_UNWRITTEN_FIELD
when `assert` is used or when Slf4j’s `LoggerFactory.getLogger(Foo.class)`
is used.
@granthenke
Copy link
Author

This isn't intended to be committed, but just to illustrate the issue.

@granthenke
Copy link
Author

Digging in a little, adding assert prevents unwrittenField from being added to declaredFields here because xFactory.isReflectiveClass(classDescriptor) is true.

The same thing happens when LoggerFactory.getLogger(UnreportedUnwrittenField.class); is used.

@granthenke
Copy link
Author

It looks like it is marked as a reflective class here because of an LDC (18) op code in the case of assert and LoggerFactory.getLogger(UnreportedUnwrittenField.class);.

FWIW the test passes after removing the !xFactory.isReflectiveClass(classDescriptor) check here.

@granthenke
Copy link
Author

In case it's helpful this behavior was introduced in this commit: 9ae2687#diff-bfcd2c82604fc6cb72facf4b143d8394

@wreulicke
Copy link
Member

@granthenke Thank you for your PR.
I seem that CI is failing by formatting problem.
Could you apply ./gradlew spotlessApply in your terminal for code format and commit the changes.

@github-actions github-actions bot added the Stale label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants