Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package checks.tests;

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

import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.fail;

class UnusedPrivateMethodSampleTest {
private static Stream<Integer> provideData() {
return Stream.of(1, 2, 3);
}

private static Stream<Integer> provideDataValue() {
return Stream.of(1, 2, 3);
}

// False positive: JUnit 5 supports fully qualified method names, but we do not.
private static Stream<Integer> provideDataFQ() { // Noncompliant
return Stream.of(1, 2, 3);
}

private static Stream<Integer> provideDataArray1() {
return Stream.of(1, 2, 3);
}

private static Stream<Integer> provideDataArray2() {
return Stream.of(4, 5, 6);
}

private static Stream<Integer> provideDataArray1Value() {
return Stream.of(1, 2, 3);
}

private static Stream<Integer> provideDataArray2Value() {
return Stream.of(4, 5, 6);
}

private static Stream<Integer> notUsed() { // Noncompliant
return Stream.of(7, 8, 9);
}

@ParameterizedTest
@MethodSource("provideData")
void test(int num) {
fail();
}

@ParameterizedTest
@MethodSource(value = "provideDataValue")
void testValue(int num) {
fail();
}

@ParameterizedTest
@MethodSource("checks.tests.UnusedPrivateMethodSampleTest#provideDataFQ")
void testFQ(int num) {
fail();
}

@ParameterizedTest
@MethodSource({"provideDataArray1", "provideDataArray2"})
void testArray(int num) {
fail();
}

@ParameterizedTest
@MethodSource(value = {"provideDataArray1Value", "provideDataArray2Value"})
void testArrayValue(int num) {
fail();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
import org.sonar.plugins.java.api.tree.ClassTree;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.LiteralTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodReferenceTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.NewArrayTree;
import org.sonar.plugins.java.api.tree.NewClassTree;
import org.sonar.plugins.java.api.tree.ParameterizedTypeTree;
import org.sonar.plugins.java.api.tree.Tree;
Expand Down Expand Up @@ -119,6 +121,14 @@ public List<MethodTree> getUnusedResolvedPrivateMethods() {
return unusedPrivateMethods.stream().filter(it -> !unresolvedMethodNames.contains(it.simpleName().name())).toList();
}

/**
* JUnit's {@code @MethodSource} annotation is attached to a test and can be used in two ways:
* <ul>
* <li>Without arguments, it indicates that the method with the same name as the test provides arguments.</li>
* <li>If one or more arguments are provided, then they explicitly indicate which methods will be called.</li>
* </ul>
* Here we handle the first case. The second case is handled by the more general {@link MethodsUsedInAnnotationsFilter}.
*/
private static List<String> getMethodSourcesNames(ClassTree tree) {
return tree.members().stream()
.filter(it -> it instanceof MethodTree mt && isAnnotatedWithMethodSource(mt))
Expand Down Expand Up @@ -273,26 +283,46 @@ private static boolean isNameIndicatingMethod(String name) {
return name.toLowerCase(Locale.getDefault()).contains("method");
}

private void removeMethodName(LiteralTree literal) {
filteredNames.remove(removeQuotes(literal.value()));
private void removeMethodName(String quotedName) {
filteredNames.remove(removeQuotes(quotedName));
}

private static String removeQuotes(String withQuotes) {
return withQuotes.substring(1, withQuotes.length() - 1);
}

private void removeMethodName(LiteralTree literal) {
removeMethodName(literal.value());
}

private void removeMethodNames(NewArrayTree newArrayTree) {
for (ExpressionTree initializer : newArrayTree.initializers()) {
if (initializer.is(Tree.Kind.STRING_LITERAL)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition should always be true (or the code doesn't compile)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initializer must be known at compile time, but does not have to be a literal. For example, the following is fine:

private static final String PROVIDE_DATA_ARRAY = "provideDataArray";

  @ParameterizedTest
  @MethodSource(value = {PROVIDE_DATA_ARRAY, PROVIDE_DATA_ARRAY + "2"})
  void testAddArray(int num) {}

This is, however, a corner case, and I don't think supporting it is a good use of time.

removeMethodName((LiteralTree) initializer);
}
}
}

private void removeMethodName(ExpressionTree arg) {
if (arg.is(Tree.Kind.STRING_LITERAL)) {
removeMethodName((LiteralTree) arg);
} else if(arg.is(Tree.Kind.NEW_ARRAY)) {
removeMethodNames((NewArrayTree) arg);
}
}

@Override
public void visitAnnotation(AnnotationTree annotationTree) {
var isMethodAnnotation = isNameIndicatingMethod(annotationTree.annotationType().symbolType().name());
for (var arg : annotationTree.arguments()) {
if (arg.is(Tree.Kind.STRING_LITERAL)) {
if (isMethodAnnotation) {
removeMethodName((LiteralTree) arg);
if (arg instanceof AssignmentExpressionTree asgn) {
// Handle syntax with assignment, for example, @MethodSource(value = "methodName").
if (isMethodAnnotation || isNameIndicatingMethod(((IdentifierTree) asgn.variable()).name())) {
removeMethodName(asgn.expression());
}
Comment thread
tomasz-tylenda-sonarsource marked this conversation as resolved.
} else if (arg instanceof AssignmentExpressionTree asgn && asgn.expression().is(Tree.Kind.STRING_LITERAL) && (
isMethodAnnotation || isNameIndicatingMethod(((IdentifierTree) asgn.variable()).name())
)) {
removeMethodName((LiteralTree) asgn.expression());
} else if (isMethodAnnotation) {
// No assignment, for example: @MethodSource("methodName").
removeMethodName(arg);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ void test() {
.verifyIssues();
}

@Test
void test_methodSource() {
CheckVerifier.newVerifier()
.onFile(TestUtils.testCodeSourcesPath("checks/tests/UnusedPrivateMethodSampleTest.java"))
.withCheck(new UnusedPrivateMethodCheck())
.verifyIssues();
}

@Test
void test_non_compiling() {
CheckVerifier.newVerifier()
Expand Down
Loading