Skip to content

Commit

Permalink
SONARJAVA-5099 Fix FP on S1144 if @MethodSource is used without argum…
Browse files Browse the repository at this point in the history
…ents (#4867)
  • Loading branch information
irina-batinic-sonarsource authored Sep 18, 2024
1 parent 03e34a9 commit 8501fe9
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public void javaCheckTestSources() throws Exception {
SoftAssertions softly = new SoftAssertions();
softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
softly.assertThat(newTotal).isEqualTo(knownTotal);
softly.assertThat(rulesCausingFPs).hasSize(9);
softly.assertThat(rulesCausingFPs).hasSize(10);
softly.assertThat(rulesNotReporting).hasSize(10);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"ruleKey": "S1144",
"hasTruePositives": true,
"falseNegatives": 1,
"falsePositives": 0
}
"falsePositives": 1
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package checks;

import org.junit.jupiter.params.provider.MethodSource;

import java.io.Serializable;
import java.math.BigDecimal;
import java.util.ArrayList;
Expand All @@ -11,12 +13,25 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;

import static java.util.Collections.emptySet;
@interface Observes {}

class UnusedPrivateMethodCheck {

@MethodSource
private void unusedMethodWithWrongAnnotation(){} // Compilant - FN

@MethodSource()
void testMessages(String message) {
// ...
}

private static Stream<String> testMessages() { // Compliant, used by @MethodSource
return Stream.of("Hello world!", "Carpe diem!");
}

private void init(@Observes Object object, String test) {} // Noncompliant
private void init(@javax.enterprise.event.Observes Object object) {} //Compliant, javax.enterprise.event.Observes is an exception to the rule
private void jakartaInit(@jakarta.enterprise.event.Observes Object object) {} //Compliant, jakarta.enterprise.event.Observes is an exception to the rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ private static Stream<MethodTree> findUnusedPrivateMethods(ClassTree tree) {
methodNames.removeAll(collector.unresolvedMethodNames);
var filter = new MethodsUsedInAnnotationsFilter(methodNames);
tree.accept(filter);
return unusedPrivateMethods.stream().filter(it -> filter.filteredNames.contains(it.simpleName().name()));
var methodTreeStream = unusedPrivateMethods.stream().filter(it -> filter.filteredNames.contains(it.simpleName().name()));
var methodSourceAnnotatedMethods = getMethodSourcesNames(tree);
return methodTreeStream.filter(it -> !methodSourceAnnotatedMethods.contains(it.simpleName().name()));
}

private void reportUnusedPrivateMethods(Stream<MethodTree> methods) {
Expand All @@ -101,6 +103,19 @@ private void reportUnusedPrivateMethods(Stream<MethodTree> methods) {
});
}

private static List<String> getMethodSourcesNames(ClassTree tree) {
return tree.members().stream()
.filter(it -> it instanceof MethodTree mt && isAnnotatedWithMethodSource(mt))
.map(MethodTree.class::cast)
.map(it -> it.simpleName().name())
.toList();
}

private static boolean isAnnotatedWithMethodSource(MethodTree methodTree) {
return methodTree.modifiers().annotations().stream()
.anyMatch(annotation -> annotation.annotationType().symbolType().is("org.junit.jupiter.params.provider.MethodSource"));
}

private static class UnusedMethodCollector extends BaseTreeVisitor {

public final List<MethodTree> unusedPrivateMethods = new ArrayList<>();
Expand Down

0 comments on commit 8501fe9

Please sign in to comment.