Prevent non-test methods from being flagged as such

This commit is contained in:
Gijs de Jong
2023-03-13 09:55:19 +01:00
committed by Rick Ossendrijver
parent 9ab79cb69b
commit edf2b7ca79
5 changed files with 38 additions and 16 deletions

View File

@@ -13,7 +13,7 @@ import java.util.Optional;
* A {@link tech.picnic.errorprone.testngjunit.Migrator} that migrates the {@code enabled} argument.
*/
@Immutable
public class EnabledArgumentMigrator implements Migrator {
final class EnabledArgumentMigrator implements Migrator {
@Override
public Optional<SuggestedFix> createFix(
ClassTree classTree, MethodTree methodTree, ExpressionTree dataValue, VisitorState state) {

View File

@@ -125,8 +125,7 @@ public final class TestNGJUnitMigration extends BugChecker implements Compilatio
fixBuilder.merge(migrateAnnotation(annotation, tree));
state.reportMatch(
describeMatch(annotation.getAnnotationTree(), fixBuilder.build()));
state.reportMatch(describeMatch(tree, fixBuilder.build()));
});
return super.visitMethod(tree, metaData);
}

View File

@@ -1,11 +1,17 @@
package tech.picnic.errorprone.testngjunit;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.not;
import static tech.picnic.errorprone.testngjunit.TestNGMatchers.TESTNG_TEST_ANNOTATION;
import static tech.picnic.errorprone.testngjunit.TestNGMatchers.TESTNG_VALUE_FACTORY_METHOD;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.ClassTree;
@@ -15,6 +21,7 @@ import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.source.util.TreeScanner;
import java.util.Optional;
import javax.lang.model.element.Modifier;
import org.jspecify.annotations.Nullable;
/**
@@ -24,6 +31,10 @@ import org.jspecify.annotations.Nullable;
*/
final class TestNGScanner extends TreeScanner<@Nullable Void, TestNGMetadata.Builder> {
private final VisitorState state;
private static final Matcher<MethodTree> TESTNG_TEST_METHOD =
anyOf(
hasAnnotation("org.testng.annotations.Test"),
allOf(hasModifier(Modifier.PUBLIC), not(hasModifier(Modifier.STATIC))));
private final ImmutableMap.Builder<ClassTree, TestNGMetadata> metadataBuilder =
ImmutableMap.builder();
@@ -63,9 +74,11 @@ final class TestNGScanner extends TreeScanner<@Nullable Void, TestNGMetadata.Bui
return super.visitMethod(tree, builder);
}
getTestNGAnnotation(tree, state)
.or(builder::getClassLevelAnnotationMetadata)
.ifPresent(annotation -> builder.methodAnnotationsBuilder().put(tree, annotation));
if (TESTNG_TEST_METHOD.matches(tree, state)) {
getTestNGAnnotation(tree, state)
.or(builder::getClassLevelAnnotationMetadata)
.ifPresent(annotation -> builder.methodAnnotationsBuilder().put(tree, annotation));
}
return super.visitMethod(tree, builder);
}

View File

@@ -15,25 +15,27 @@ final class TestNGJUnitMigrationTest {
"import org.testng.annotations.DataProvider;",
"import org.testng.annotations.Test;",
"",
"// BUG: Diagnostic contains:",
"@Test",
"public class A {",
" // BUG: Diagnostic contains:",
" public void classLevelAnnotation() {}",
"",
" public static void staticNotATest() {}",
"",
" private void notATest() {}",
"",
" // BUG: Diagnostic contains:",
" @Test(description = \"bar\")",
" // BUG: Diagnostic contains:",
" public void methodAnnotation() {}",
"",
" // BUG: Diagnostic contains:",
" @Test",
" public static class B {",
" // BUG: Diagnostic contains:",
" public void nestedClass() {}",
" }",
"",
" // BUG: Diagnostic contains:",
" @Test(dataProvider = \"dataProviderTestCases\")",
" // BUG: Diagnostic contains:",
" public void dataProvider(int foo) {}",
"",
" @DataProvider",
@@ -75,9 +77,9 @@ final class TestNGJUnitMigrationTest {
" @Test(description = \"bar\")",
" public void methodAnnotation() {}",
"",
" // BUG: Diagnostic contains:",
" @Test",
" public static class B {",
" // BUG: Diagnostic contains:",
" public void nestedClass() {}",
" }",
"",

View File

@@ -33,23 +33,31 @@ final class TestNGScannerTest {
"@Test",
"class A {",
"",
" void inferClassLevelAnnotation() {}",
" public void inferClassLevelAnnotation() {}",
"",
" void packagePrivateNotATest() {}",
"",
" private void privateNotATest() {}",
"",
" static void staticNotATest() {}",
"",
" public static void publicStaticNotATest() {}",
"",
" // BUG: Diagnostic contains: class: A arguments: { }",
" @Test",
" void localAnnotation() {}",
" public void localAnnotation() {}",
"",
" // BUG: Diagnostic contains: class: A arguments: { description: \"foo\" }",
" @Test(description = \"foo\")",
" void singleArgument() {}",
" public void singleArgument() {}",
"",
" // BUG: Diagnostic contains: class: A arguments: { priority: 1, description: \"foo\" }",
" @Test(priority = 1, description = \"foo\")",
" void multipleArguments() {}",
" public void multipleArguments() {}",
"",
" // BUG: Diagnostic contains: class: A arguments: { dataProvider: \"dataProviderTestCases\" }",
" @Test(dataProvider = \"dataProviderTestCases\")",
" void dataProvider() {}",
" public void dataProvider() {}",
"",
" // BUG: Diagnostic contains: class: B arguments: { description: \"nested\" }",
" @Test(description = \"nested\")",