SonarQube marks function refered by MethodSource as Unused "private" method per rule 1144

  • UsingSonarQube Enterprise Edition Version 8.9.7 (build 52159)
  • I am trying to use the new MethodSource functionality from Junit 5, but SonarQube marks the referenced function as an unused private method.

As far as I can see, SonarQube should support Junit 5, so I don’t know why it flags these functions as unused.

Hi,

Welcome to the community!

I’m a bit confused by your question. Are you saying that S1144 is raising issues on test files? From what I can tell that rule is for source files only.

 
Ann

Hi,

Thank you!

Yes, the rule is raised in test files.

From what I read on the rule: [RSPEC-1144] - Jira it states in the Analysis Scope that Test Sources are part of it.

And I would very much think that it’s just as important to avoid unused code in test code as in production code.

Hi,

That Jira repo is vestigial; the rules got moved into Git a while ago. But even when it was in use, that scope field was largely aspirational. It got honored for C# but nothing else. Since then some work has been done around what rules apply to tests, but the scope of that was never clear to me.

Can you provide a reproducer snippet?

 
Ann

The function provideFToA is marked as an unused private method by SonarQube:

  @ParameterizedTest
  @MethodSource("provideFToA")
  void getAFromF_fIsDefined_aIsCorrect(String f, A eA) {
    A a = AM.getAFromF(f);

    assertThat(a).isEqualTo(eA);
  }

  private static Stream<Arguments> provideFToA() {
    return Stream.of(
      Arguments.of("ABC", A.ABC),
      Arguments.of("DEF", A.DEF),
      Arguments.of("invalid", A.INVALID),
      Arguments.of(null, A.NULL)
    );
  }

I have obfuscated the code a bit.

Some other examples can also be found here: junit5_workshop/ParameterizedTests.java at 917b4d59b1820c474a7e1ab6e18c5ee210914a15 · kousen/junit5_workshop · GitHub

1 Like

Hi,

Thanks for the code snippet.

I see work on this rule since 8.9, but nothing that seems relevant (to me) to your case, so I’m going to flag this for developer attention.

 
Ann

1 Like

hey @Molzen,

I have been trying to reproduce the issue using the Junit workshop project you mentioned but unfortunately, I cannot see the issue.

In order to confirm the issue, I will need to know about:

  • The version of Java used to build and analyze
  • The version of the scanner used
  • Any relevant analysis parameter that may cause the test code to be confused with the main code

Cheers,

Dorian

1 Like

Java: zulu8.60.0.21-ca-jdk8.0.322-win_x64
Scanner: org.sonarsource.scanner.cli:sonar-scanner-cli:4.7.0.2747

I would think it is correct that it also scans test code for unused methods. Having unused code in test files are just as much dead code as in regular classes.

Is anyone still looking into this issue?

As Ann has already mentioned, the rule is currently not designed to run on test code. In principle, I completely agree with you, that this rule could also make sense applied to test code. However, as your example shows very well, we can’t simply activate rules that work well on main code for test code and expect everything to work out of the box. Problems like this FP arise, as the rule does not understand that the annotation provided in the context of the parametrized test actually means that the method is used.

One day, I hope we can improve our support for test code, as there are certainly many rules in the Java analyzer, which currently only run on main code and would also bring value for test code. Unfortunately, I cannot give you an ETA for this, though.

Since you are using the CLI sonar scanner, you will manually need to provide the directories that contain test code vs main code (using the property sonar.tests - see the docs here). Alternatively, if you can, I would recommend that you use the SonarScanner for Gradle or for Maven instead, which will usually be able to set these (and more!) properties for you.

If you still want to run the rules designed for main code on your test code, you will unfortunately have to expect odd behavior for the time being.

Hi,

We noticed a sudden rise in the number of code smells in our projects (without code changes), and upon analysis, we noticed that all of them are S1144 in test files - all are on private methods used with @MethodSource. We also noticed that the “analysis scope” of S1144 is “Main and Test sources” in our SonarCloud instance. Did this scope change recently?

I also saw that there was recent work in this regard in [SONARJAVA-4943] - Jira. Is this fix already available?

Hi @RPluizberto,

Recently, we worked on the effort to enable some rules on the test code. You can find more details here: SONARJAVA-4932.
The rule S1144 was part of this effort, and that is why you see a rise in the number of issues in your project.

For the other point, SONARJAVA-4943 is part of the 7.34 release, and the fix should be on SonarCloud.

Hope this helps!

All the best,

Irina

Hi @irina.batinic ,

Thanks for the update.

Out of curiosity, was this communicated in any changelog?

Also, is the analysis scope of a given rule configurable in SonarCloud?

Regarding @MethodSource, there’s a false positive when no value is passed to the annotation:

If no factory method names are declared, a method within the test class that has the same name as the test method will be used as the factory method by default.

Source: https://junit.org/junit5/docs/5.8.0/api/org.junit.jupiter.params/org/junit/jupiter/params/provider/MethodSource.html

As JUnit5 is a massively used library, is there any chance that this false positive could be fixed? Most of our uses of @MethodSource rely on this default behavior.

1 Like

Hi @RPluizberto,

Yes, it is written in our change logs. You can find it here: Release 7.34.0.35958 · SonarSource/sonar-java · GitHub.

For now, it is not configurable.

And regarding the False Positive, may I ask you to open a new thread?

Thanks a lot!

All the best,

Irina

I just saw this issue running SL locally in IntelliJ 10.9

Hi @boardtc,

You’ve resurrected a thread that’s nearly 6 months old. Per the FAQ, please don’t do that. Please create a new thread with all your details.

 
Thx,
Ann