From f231d964c418ba28c9fb991a668ef60cc4264c3b Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 16 Jul 2013 17:30:27 +0200 Subject: [PATCH] IDEA-107730@VisibleForTesting is marked as a warning when called within class --- .../testOnly/TestOnlyInspection.java | 39 ++++++++++++++++--- .../testOnly/visibleForTesting/expected.xml | 8 ++++ .../testOnly/visibleForTesting/src/A.java | 16 ++++++++ .../testOnly/visibleForTesting/src/B.java | 6 +++ .../common/annotations/VisibleForTesting.java | 14 +++++++ .../testOnly/visibleForTesting/test/Test.java | 6 +++ .../TestOnlyInspectionTest.java | 2 + 7 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/testOnly/visibleForTesting/expected.xml create mode 100644 java/java-tests/testData/inspection/testOnly/visibleForTesting/src/A.java create mode 100644 java/java-tests/testData/inspection/testOnly/visibleForTesting/src/B.java create mode 100644 java/java-tests/testData/inspection/testOnly/visibleForTesting/src/com/google/common/annotations/VisibleForTesting.java create mode 100644 java/java-tests/testData/inspection/testOnly/visibleForTesting/test/Test.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/testOnly/TestOnlyInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/testOnly/TestOnlyInspection.java index 9f69dda02b26..1cbfb1eb9510 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/testOnly/TestOnlyInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/testOnly/TestOnlyInspection.java @@ -18,9 +18,12 @@ package com.intellij.codeInspection.testOnly; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.TestFrameworks; import com.intellij.codeInspection.*; +import com.intellij.lang.java.JavaLanguage; import com.intellij.openapi.roots.ProjectRootManager; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; +import com.intellij.psi.impl.light.LightModifierList; +import com.intellij.psi.impl.source.resolve.JavaResolveUtil; import com.intellij.psi.util.PsiTreeUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -55,16 +58,42 @@ public class TestOnlyInspection extends BaseJavaBatchLocalInspectionTool { } private static void validate(PsiCallExpression e, ProblemsHolder h) { - if (!isTestOnlyMethodCalled(e)) return; + PsiMethod method = e.resolveMethod(); + + if (method == null || !isAnnotatedAsTestOnly(method)) return; if (isInsideTestOnlyMethod(e)) return; if (isInsideTestClass(e)) return; if (isUnderTestSources(e)) return; + PsiAnnotation anno = findVisibleForTestingAnnotation(method); + if (anno != null) { + LightModifierList modList = new LightModifierList(method.getManager(), JavaLanguage.INSTANCE, getAccessModifierWithoutTesting(anno)); + if (JavaResolveUtil.isAccessible(method, method.getContainingClass(), modList, e, null, null)) { + return; + } + } + reportProblem(e, h); } - private static boolean isTestOnlyMethodCalled(PsiCallExpression e) { - return isAnnotatedAsTestOnly(e.resolveMethod()); + private static String getAccessModifierWithoutTesting(PsiAnnotation anno) { + String modifier = PsiModifier.PRIVATE; + PsiAnnotationMemberValue ref = anno.findDeclaredAttributeValue("visibility"); + if (ref instanceof PsiReferenceExpression) { + PsiElement target = ((PsiReferenceExpression)ref).resolve(); + if (target instanceof PsiEnumConstant) { + String name = ((PsiEnumConstant)target).getName(); + modifier = "PRIVATE".equals(name) ? PsiModifier.PRIVATE : "PROTECTED".equals(name) ? PsiModifier.PROTECTED : PsiModifier.PACKAGE_LOCAL; + } + } + return modifier; + } + + @Nullable + private static PsiAnnotation findVisibleForTestingAnnotation(@NotNull PsiMethod method) { + PsiModifierList modifierList = method.getModifierList(); + PsiAnnotation anno = modifierList.findAnnotation("com.google.common.annotations.VisibleForTesting"); + return anno != null ? anno : modifierList.findAnnotation("com.android.annotations.VisibleForTesting"); } private static boolean isInsideTestOnlyMethod(PsiCallExpression e) { @@ -73,9 +102,7 @@ public class TestOnlyInspection extends BaseJavaBatchLocalInspectionTool { } private static boolean isAnnotatedAsTestOnly(@Nullable PsiMethod m) { - return m != null && - (AnnotationUtil.isAnnotated(m, AnnotationUtil.TEST_ONLY, false, false) || - AnnotationUtil.isAnnotated(m, "com.google.common.annotations.VisibleForTesting", false, false)); + return m != null && (AnnotationUtil.isAnnotated(m, AnnotationUtil.TEST_ONLY, false, false) || findVisibleForTestingAnnotation(m) != null); } private static boolean isInsideTestClass(PsiCallExpression e) { diff --git a/java/java-tests/testData/inspection/testOnly/visibleForTesting/expected.xml b/java/java-tests/testData/inspection/testOnly/visibleForTesting/expected.xml new file mode 100644 index 000000000000..ea1f69350f49 --- /dev/null +++ b/java/java-tests/testData/inspection/testOnly/visibleForTesting/expected.xml @@ -0,0 +1,8 @@ + + + + B.java + 3 + Test-only method is called in production code + + diff --git a/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/A.java b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/A.java new file mode 100644 index 000000000000..c88fa9e437ba --- /dev/null +++ b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/A.java @@ -0,0 +1,16 @@ +public class A { + void publicMethod() { + invisibleMethod(1); + visibleMethod(1); + } + + @com.google.common.annotations.VisibleForTesting() + void invisibleMethod(int a) { + + } + + @com.google.common.annotations.VisibleForTesting(visibility=com.google.common.annotations.VisibleForTesting.Visibility.PROTECTED) + void visibleMethod(int a) { + + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/B.java b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/B.java new file mode 100644 index 000000000000..ec069c893a6c --- /dev/null +++ b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/B.java @@ -0,0 +1,6 @@ +public class B { + void publicMethod() { + new A().invisibleMethod(2); + new A().visibleMethod(2); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/com/google/common/annotations/VisibleForTesting.java b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/com/google/common/annotations/VisibleForTesting.java new file mode 100644 index 000000000000..50a1589b25f7 --- /dev/null +++ b/java/java-tests/testData/inspection/testOnly/visibleForTesting/src/com/google/common/annotations/VisibleForTesting.java @@ -0,0 +1,14 @@ +package com.google.common.annotations; + +public @interface VisibleForTesting { + enum Visibility { + /** The element should be considered protected. */ + PROTECTED, + /** The element should be considered package-private. */ + PACKAGE, + /** The element should be considered private. */ + PRIVATE + } + + Visibility visibility() default Visibility.PRIVATE; +} diff --git a/java/java-tests/testData/inspection/testOnly/visibleForTesting/test/Test.java b/java/java-tests/testData/inspection/testOnly/visibleForTesting/test/Test.java new file mode 100644 index 000000000000..0ad73852cf33 --- /dev/null +++ b/java/java-tests/testData/inspection/testOnly/visibleForTesting/test/Test.java @@ -0,0 +1,6 @@ +public class Test { + void publicMethod() { + new A().invisibleMethod(3); + new A().visibleMethod(3); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/TestOnlyInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/TestOnlyInspectionTest.java index 91b0ee7db989..e368d0e73e14 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/TestOnlyInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/TestOnlyInspectionTest.java @@ -36,6 +36,8 @@ public class TestOnlyInspectionTest extends InspectionTestCase { doTest(); } + public void testVisibleForTesting() throws Exception { doTest(); } + public void testUnresolved() throws Exception { doTest(); // shouldn't throw }