squid:S1172 false positive (unused parameter) in SonarLint for Java

[vlot...@evelopers.com]

Dear team,

I can not understand why if parameter not used in the ancestor but used in descendant it is marked as unused by Sonar? For example, in the following code snippet:

public class S1172Ancestor {

    protected void m(String a) {

        System.out.println("test");

    }

}

public class S1172DEscendant extends S1172Ancestor {

    @Override

    protected void m(String a) {

        System.out.println(a);

    }

}

SonarLint detects S1172 positive on the method 'm' in the class S1172Ancestor

Thanks in advance,

Vadim

[Michael Gumowski]

Hello Vadim,

The rule is ignoring unused parameters only in very particular cases:

• The method is overriding a method from a parent class (obviously, you may not need the parameter, but you can not get rid of it in the signature):
  
• The method is a method from the Serializable contract;
  
• The method has any kind of annotation;
  
• The method is identified as being "designed for extension".
  

You method does not satisfy the 3 first conditions. In order to consider a method as being "designed for extension", it needs to have an empty body (can contains comments), or its body should only throw an exception (such as NotImplementedException). In your case, we do not consider the method as being so, as you are obviously adding logic into its body. Remove the System.out.println method invocation and the issue will disappear. The same way, you can drop the Sysout in S1172DEscendant and the rule will not raise an issue, as you are overriding the method.

The rule does not check for subclasses overriding the method, because we only compute childClass -> parentClass(es) relations, on the fly.

Michael

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/a104b520-6d58-48d3-8030-994f93a3a287%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Michael Gumowski | SonarSource

Software Developer, Language Team
http://sonarsource.com

[vlot...@evelopers.com]

Michael,

Why just do not look for all sub-classes overriding the method? Is it technically difficult? Agree, this is definitely a false positive. There is nothing wrong with the method designed for extension but had some reasonable default implementation.

Regards,

Vadim

четверг, 24 августа 2017 г., 19:04:22 UTC+3 пользователь Michael Gumowski написал:

[Michael Gumowski]

Hello Vadim,

Unfortunately, being able to collect all overrides of a method is technically difficult, and won't be envisaged for the time being. Going the other way around is much more simpler (from subclass to parent classes). In you case, I agree that this can be considered as a false positive, depending of what is the implementation.

In order to limit such cases, another approach could be to be able to define what is "a reasonable default implementation", not using (some of the) parameters. Could you define what you are considering a valid one? Maybe we could extract a heuristic from it, and try limit the noise. Without a general approach, I'm afraid that for the time being you will have to consider these as FPs.

Cheers,

Michael

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/3d5dfc1e-6219-4015-876c-55d72b4c0d82%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Vadim Lotarev]

Michael,

      In addition to SuppressWarnings annotation wouldn't it be possible to implement an Eclipse way to suppress this warning: if this parameter is documented in JavaDoc than it is considered as "used"?
     
Regards,
Vadim

---

From: Michael Gumowski <michael....@sonarsource.com>
Sent: Tuesday, September 5, 2017, 12:17:22 PM
To: vlot...@evelopers.com, SonarQube <sona...@googlegroups.com>
CC:
Subject: squid:S1172 false positive (unused parameter) in SonarLint for Java

vlot...@evelopers.com> wrote:

[Michael Gumowski]

Hey Vadim,

Thanks for the idea, that's indeed a good option and a good compromise. I created the following ticket to improve the rule and not raise issues on unused parameter anymore, if they are properly documented in javadoc: SONARJAVA-2443

This will of course only be applied on overridable method. Unused parameters of static or final method will continue to raise issue, even if the parameter is documented.

Cheers,

Michael