S1172: Trivial implementation inconsistent and SuppressWarnings("unused") on parameter ignored

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);
  }
}
1 Like

Hello,

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.

Is that correct?

Hi Alexandre, thanks for your reply.

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:

  1. `SuppressWarnings(“unused”) is ignored when applied directly to a parameter

  2. 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

Hello.

Here is another typical Android use case. Once you assigned a listener to a view you need to provide a certain method signature in the corresponding method:

sendButton.setOnClickListener(::handleClick)

private fun handleClick(@Suppress("UNUSED_PARAMETER") view: View) {
   // do something
}

Lint understands the suppression but SonarLint ignores it and raises kotlin:S1172 as a code smell.
Let me know if you need more information regarding this issue.

Tobias