False positive "The static method X from the type Y should be accessed directly"

[laeubi]

Assume the following code: test.access.staticprotected.zip

We now change the class like this and get a warning:

This is a false-positive because the recommended "direct" access is actually not accessible:

[stephan-herrmann]

While wondering, whether or not that warning should be ditched completely, I found that SonarLint has a similar warning, see, e.g., https://stackoverflow.com/questions/71754314/sonarlint-rule-javas3252-static-base-class-members-should-not-be-accessed-vi

Some reasons to keep this warning can be found in scenarios of code evolution, e.g., when adding a same-named static member to the derived type, which of course does not override the inherited member (there is no overriding for statics), but still the client will only be binary compatible, but not source compatible (recompiling will change the result).

Actually, in "normal" use, there is no benefit ever in using a subclass for referencing an inherited static member. So, I believe this pattern will always create some confusion to the reader. Furthermore I've seen cases, where such reference creates a completely unnecessary dependency on the sub class.

On the other hand I'm not sure it's worth adding more complexity to this analysis.

Maybe the following compromise would fit:

• jdt.core: rephrase the warning to: "Static method X from type Y is accessed indirectly via type Z" -- i.e., refrain from suggesting what "should" happen
• jdt.ui: perform a visibility check before proposing this particular quick fix, alternatively ditch that quick fix.

[laeubi]

@stephan-herrmann the problem is not the warning itself, it is just that in this case the user can't follow the advice without creating invalid code.

So the filter should happen before generating the warning, if the "direct" type is not visible (and with the second screenshot you see that JDT actually knows this in some way already) the warning should be skipped (and then the quick-fix problem is solved as well), but I think strange enough the Quickfix already knows because it only offers to remove the static, but it should actually recommend to use the subtype!

[stephan-herrmann]

@stephan-herrmann the problem is not the warning itself, it is just that in this case the user can't follow the advice without creating invalid code.

If the warning contains no advice, this wouldn't be a problem, would it? :)

the Quickfix already knows because it only offers to remove the static, but it should actually recommend to use the subtype!

So you recommend, the quick fix for the "is not visible" problem should do a workspace search for all subtypes of the protected type? Performing this kind of work is OK for applying a quick fix, but I don't think it would be acceptable to wait for a workspace search before actually proposing any quick fixes (and thus, before even showing the hover).

[laeubi]

If the warning contains no advice, this wouldn't be a problem, would it? :)

It depends on, a warning I cannot act upon will most likely be reported as a bug because I can never fix it :-D

So you recommend, the quick fix for the "is not visible" problem should do a workspace search for all subtypes of the

No the warning should not be displayed at all, so wherever this warning is produced, instead of create the warning:

Use X instead of Y, it should first check if X can actually be accessed ... if not, simply skip adding the warning marker, so the code must already know the type because how could it know otherwise I should change it to X?

[stephan-herrmann]

So you recommend, the quick fix for the "is not visible" problem should do a workspace search for all subtypes of the

No the warning should not be displayed at all,

I was careful to quote the problem (an error) I was referring to. You're answer refers to a different problem (warning).

[laeubi]

I see a warning (first screenshot ), that should not be displayed, because if I follow the advice from the warning the warning turns into an error (second screenshot) :-)

[stephan-herrmann]

So we can keep confusing each other by talking about different warnings, errors, quick fixes.

Essentially I was trying to respond to your request "it [a quick fix] should actually recommend to use the subtype", and apparently that request is not solved by not showing a problem. Are we on the same page now?

[laeubi]

Okay sorry for confusion then, for me there are two cases:

1. The SubType is not accessible -> no warning (and therefore no quickfix)
2. The SubType is accessible -> warning, but the Quickfix should suggest that I use the SubType because I don't understand why removing the static qualifier should be good for anything (what is currently suggested as a quickfix).