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

false-positives for CDI targeted methods in UPM_UNCALLED_PRIVATE_METHOD #947

Open
Vampire opened this issue May 15, 2019 · 6 comments
Open

Comments

@Vampire
Copy link
Contributor

Vampire commented May 15, 2019

If you have methods for CDI (or also for Spring, I don't know whether there are false-postivies though as I didn't try it) they are often private so that only the DI framework handles them usually.
This for example includes methods that are annotated with @Inject, or @Produces or methods with a parameter that is annotated with @Disposes, @Observes or @ObservesAsync. Those methods are usually called by the CDI framework and thus should not be marked as unused just like methods annotated with @PostConstruct, or @PreDestroy.

@ThrawnCA
Copy link
Contributor

How is your CDI framework calling private methods? If it's using reflection to disable the access control checks, then that is not a good approach.

@Vampire
Copy link
Contributor Author

Vampire commented May 17, 2019

I don't know, it is the official reference implementation of the CDI standard WELD, so you can have a look.
As far as I remember Springs @Autowired also works fine on private members.
From a client code perspective it actually is a good approach as you can make sure onyl the framework or some desparate one can set it.

@Syquel
Copy link

Syquel commented May 20, 2019

IMO using private methods with CDI is a bad programming style and package-private / protected Methods like defined in the JPA specification should be preferred.
But: the CDI specification allows and explicitly states that private methods and fields are supported for nearly all use-cases.
Just CTRL+F for "public, protected or private": http://docs.jboss.org/cdi/spec/2.0/cdi-spec.html

Regarding how CDI implementations do that: it depends on the specific implementation but since Java 11 disallowed reflective modifications to the visibility of methods and fields, most are either switching over to MethodHandle / VarHandle with a private lookup or to generating helper methods in the generated proxy class.

I would suggest to maintain a list of standart JavaEE annotations for this rule like the ones defined in CDI 2.0 to prevent such false-positives.

@Vampire
Copy link
Contributor Author

Vampire commented May 20, 2019

IMO using private methods with CDI is a bad programming style and package-private / protected Methods like defined in the JPA specification should be preferred.

May I ask where this is defined?
I didn't find it from a quick search, only that private is supported, but not that package-private or protected is preferred.
Or do you refer to the used examples?

@Syquel
Copy link

Syquel commented May 20, 2019

That was just my personal opinion. Therefore the "IMO".
This opinion is based on how the Java Language and Bytecode Specification intend us to use visibilities and they even made it way harder to call / access private methods / fields with the changes since Java 9 regarding reflection.
As a framework developer it got a lot harder to circumvent these new restrictions on visibilities.
Workarounds like Bytecode modification via Java Agents and / or generating inner classes with helper methods to be able to access private fields / methods should be viewed as such: workarounds.
We never know when the Java Spec will close those loopholes and when they will cease to work.

For CDI there is no official recommendation regarding visibility, but other JavaEE standards like JPA do to make it easier on Framework developers and to be more Future-Proof.

danielkec added a commit to danielkec/helidon that referenced this issue Jun 24, 2022
* Workaround UPM_UNCALLED_PRIVATE_METHOD false positive - spotbugs/spotbugs#947
* Mutable vs. immutable socket config
* Cleanup + examples alignment
* Compression with upgrade handlers test
* Https and wss negotiation with alpn
* One codec for enc and decoding

Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
danielkec added a commit to helidon-io/helidon that referenced this issue Jun 28, 2022
* Http/2 and WebSockets as standalone modules

* Workaround UPM_UNCALLED_PRIVATE_METHOD false positive - spotbugs/spotbugs#947
* Mutable vs. immutable socket config
* Cleanup + examples alignment
* Compression with upgrade handlers test
* Https and wss negotiation with alpn
* One codec for enc and decoding

Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@jensli
Copy link

jensli commented Mar 29, 2023

This is a problem for JavaFX applications too. GUI callbacks in JavaFX is commonly implemented in private methods with an @FXML annotation.

It would be great if we could configure the check for UPM_UNCALLED_PRIVATE_METHOD with a list of annotations which would suppress it.

That would cater for both JavaFX, DI frameworks and other use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants