For a method with a return type the trivial implementation is to return null; for a method returning void it is the empty body. However, for trivial implementations with return type the rule raises an issue because the parameters are not used; see function1 and function2. In both cases the method might implement a default do nothing operation. Compare that to methods throwing an exception instead of returning null, where no issue is raised.
An issue is raised even if an unused method parameter is annotated with SuppressWarnings("unused"), see function3(String, String) and function4(String, String): Here no issue is raised for function3, but an issue is raised for function4. Intuitively, I would expect, as do my colleagues, that suppressing the warning on a specific parameter would not generate an issue for this parameter. Compare that to function5 where only one parameter is used: The method is overridable, but the implementation of the base class uses only the parameter str2, an issue is raised for str1. Subclasses are free to use both parameters, but the default behavior is to ignore str1. Here I’d like to ignore the warning on str1 but not on str2.
public class UnusedParameter {
public String function1(String str1, String str2) {
return null;
}
public void function2(String str1, String str2) {
// do nothing
}
@SuppressWarnings("unused")
public String function3(String str1, String str2) {
return null;
}
public String function4(@SuppressWarnings("unused") String str1, String str2) {
return null;
}
public void function5(String str1, String str2) {
System.out.println(str2);
}
}
If I understand correctly your feedback, you are saying that when a method is having the annotation @Override, then it’s acceptable to have some parameters not used because these parameters are requested by the interface and you have no way to remove them.
It means, S1172 should not raise an issue if a parameter defined by the parent class / interface is not used.
The rule already ignores methods with unused parameters but annotated with @Override. So it’ not me who is saying this is acceptable.
Example:
public static class OverridenMethods extends UnusedParameter {
@Override
public String function1(String str1, String str2) {
return null;
}
@Override
public void function2(String str1, String str2) {
}
@Override
public String function3(String str1, String str2) {
return null;
}
@Override
public String function4(String str1, String str2) {
return null;
}
@Override
public void function5(String str1, String str2) {
System.out.println(str1);
}
}
For non of those methods an issue for the rule S1172 is raised, although the implementation is identical.
Actually, I have two topics:
`SuppressWarnings(“unused”) is ignored when applied directly to a parameter
Rule exceptions are inconsistent between methods with and without return value
Compare String UnusedParameter#function1(String, String) and void UnusedParameter#function2(String, String): Both are overridable and have no meaningful implementation. It is up to the subclasser to provide behavior. But because function1 returns a String it must have a return value. S1172 raises an issue because it does not assess the single line return null statement as necessary default value. Thus, what I’d like is methods function1 and function2 being treated the same by the rule. The list of rule exceptions should then read
The rule will not raise issues for unused parameters:
in non-private methods that only throw, have empty bodies or simply return null
I’m not sure if this ticket is relevant. The issue is filtered if the annotation is applied to the method but not if applied to the parameter.
Would the ticket implement an additional step after ticket creation, where issues are filtered matching certain criteria or would each individual rule have to check for annotations?
Does the current implementation of S1172 ignore issue if a @SuppressWarnings("unsed") is present on the method?
Indeed the case at parameter level is not covered. I don’t think we really want to go to that level of details regarding suppress warnings.
I don’t really understand this question.
This is not defined at rule level but there is a filter applied after rule has raised issue. And answer is : no, “unused” is not a supported syntax today, even if the annotation is present at method level. And that should be covered by SONARJAVA-2410