Skip to content

Unused field not reported due to assumed reflection#2326

Merged
iloveeclipse merged 1 commit intospotbugs:masterfrom
trancexpress:sb2325
Jan 23, 2023
Merged

Unused field not reported due to assumed reflection#2326
iloveeclipse merged 1 commit intospotbugs:masterfrom
trancexpress:sb2325

Conversation

@trancexpress
Copy link
Copy Markdown
Contributor

@trancexpress trancexpress commented Jan 23, 2023

This change ensures an unused field is reported despite assumptions on reflection use for the defining class.

If reflection is assumed, the priority of the created spotbugs problem is lowered.

Fixes: #2325

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

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

@trancexpress
Copy link
Copy Markdown
Contributor Author

trancexpress commented Jan 23, 2023 •
edited
Loading

@iloveeclipse I don't see how to lower the confidence of the bug. Just the priority (++ is lowering it).

I don't see the bug reported in Eclipse, when the code believes there can be reflection. I assume due to the lowered priority.

iloveeclipse
iloveeclipse requested changes Jan 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@iloveeclipse iloveeclipse left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do two things:

  • add a trivial test to spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect
  • add an entry to the CHANGELOG.md file

Beside this, I think we should be fine.

Dirty code should be properly reported, and by filtering on prio one can define what one will see what not.
Not reporting such obvious thing makes people assume that SpotBugs is stupid, which isn't the case in general :-)

@trancexpress
Unused field not reported in reflective class
cb5647a 
This change ensures an unused field is reported despite assumptions on
reflection use for the defining class.

In reflective classes, the priority of the created problem is lowered.

Fixes: spotbugs#2325
iloveeclipse
iloveeclipse approved these changes Jan 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@iloveeclipse iloveeclipse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New with "reflective" class usage we report

Rank: Of Concern (20), confidence: Low
Pattern: UUF_UNUSED_FIELD

Same but without "reflective" class usage

Rank: Of Concern (18), confidence: Normal
Pattern: UUF_UNUSED_FIELD

So one should be able to sort that out.
Thanks, LGTM

@iloveeclipse iloveeclipse merged commit a8cd4d6 into spotbugs:master Jan 23, 2023
@iloveeclipse iloveeclipse mentioned this pull request Jan 23, 2023
Closed
@rovarga rovarga mentioned this pull request Dec 6, 2023
Open
@hazendaz hazendaz added this to the SpotBugs 4.8.0 milestone Dec 18, 2023
@vectro
Copy link
Copy Markdown

vectro commented Oct 24, 2024

With this change, we now generate SIC_INNER_SHOULD_BE_STATIC_ANON for these common patterns for getting a Method for the current method:

        class L {
        };
        Method currentlyExecutingMethod = L.class.getEnclosingMethod();

        Method currentlyExecutingMethod = new Object(){}.getClass().getEnclosingMethod();

The latter also generates DM_NEW_FOR_GETCLASS.

Is it worth opening a new issue for that?

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

Reviewers

@iloveeclipse iloveeclipse iloveeclipse approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

SpotBugs 4.8.0

Development

Successfully merging this pull request may close these issues.

Unused field not reported due to assumed reflection

4 participants

@trancexpress @vectro @iloveeclipse @hazendaz