Java Deprecated Rules

Rules and Languages
,
1

Qube version: Community Build v25.7.0.110598

There are two rules for the Java Deprecated annotation, that kind of work together:

S6355: Complains about a naked @Deprecated annotation without any arguments.
S1133: Reminds me to remove code marked with @Deprecated.

If I have both activated in my profile and always follow the @Deprecated rules, I will have no naked @Deprecated annotations anywhere. Yes, 6355 does not necessarily force me to add ‘forRemoval’. Without the attribute it defaults to forRemoval=false, which is kind of weird.

So, if I have annotated a method with @Deprecated(forRemoval=true), 1133 lists it to remind me to remove it. That’s fine.

BUT, if I have annotated code with @Deprecated(forRemoval=false), still 1133 lists it to remind me to remove it. This doesn’t look right as I have explicitly marked it, that I do NOT intend to remove it. Yes, I do have reasons to mark it deprecated, but don’t want to remove it (in the near future).

I think, 6355 is almost fine. It just does not make much sense to accept @Deprecated with only one of ‘since’ and ‘forRemoval’. It should force ‘forRemoval’ to be used. ‘since’ is indeed optional. Yes, ‘forRemoval’ has a default. Though this default doesn’t really reflect what the annotation said before there were any attributes. And what is this rule good for then?

1133 is perfectly fine, if code is explicitly marked forRemoval=true. A reminder to remove the code is nice.

But 1133 also looks outdated maybe. It doesn’t reflect the new attribute ‘forRemoval’. I’m not sure, if it should rely on the default forRemoval=false, because I don’t think, if code is explicitly marked not to be removed, it is the same as if I haven’t thought about it (no attribute).
I think, 1133 should only list a finding, if there is no ‘forRemoval’ attribute or if it is true. Or, if you consider the current implementation fine, I suggest three new rules: One, that lists code to be removed, if ‘forRemoval’ is missing, one for forRemoval=false and one, that lists it, if forRemoval=true. Another solution could be to make 1133 configurable, so that it only lists findings if {forRemoval is missing; forRemoval=false or missing; forRemoval=false/true or missing}.

What do you think?

2

Unfortunately I got confused about the S5738 rule. It works the other way round and is not related here. I will adjust the initial topic.

5

Hi Marvin, I can see that at least S1133 should not raise an issue when the code is specifically marked as forRemoval=false.
I created a ticket to fix that.

I am not sure I got your points about the other rules. You suggest the forRemoval should be mandatory?

6

Yes, exactly.

Both since and forRemoval have default values. So, if I don’t add them (like it is done for non updated old @Deprecated annotations), the default will be used.

So, what is this rule about? Is it meant to make me modernize the annotation by using attributes? I think so. But if it is, why should I just use one of these attributes, while both have a default? Hence the rule should not just ask me to randomly add one of these attributes just to satisfy the rule. But it should ask me to use these attributes to improve the meaning of the annotation. And forRemoval does that. since also adds improvements, but is not always necessary or even known, if code is deprecated since a long time.

Additionally the default of forRemoval does not make much sense. When there was no such attribute code got deprecated to mark it

  1. not be be used for new code
  2. remind me of updating older code
  3. tell me, that this code will be removed sooner or later

This meaning exactly matches forRemoval=true, not false. And this is one more reason to use this rule to remind me to explicitly set the forRemoval attribute, which doesn’t have a good default value.

Thanks a lot for creating that ticket.

1 Like
7

I see you point, indeed the default being false makes it more relevant to specify the value when marking code that way.
I created a ticket to reflect on updating this rule.

Thanks a lot for the insights!

9

Great! Thanks a lot. :slight_smile:

1 Like