From bbcbea29b11608c8fab67174f49a71defa907888 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 9 Nov 2016 21:05:41 +0300 Subject: [PATCH] Java: Handle most common cases of annotations in the inspection "Non-accessible type is exposed" (IDEA-162768) --- ...va9NonAccessibleTypeExposedInspection.java | 51 +++++++++++++--- .../Java9NonAccessibleTypeExposedTest.kt | 59 +++++++++++++++++-- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9NonAccessibleTypeExposedInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9NonAccessibleTypeExposedInspection.java index d89687f166e9..42ce038b8ce9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9NonAccessibleTypeExposedInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9NonAccessibleTypeExposedInspection.java @@ -65,6 +65,7 @@ public class Java9NonAccessibleTypeExposedInspection extends BaseJavaLocalInspec } private static class NonAccessibleTypeExposedVisitor extends JavaElementVisitor { + public static final String CLASS_IS_NOT_EXPORTED = "The class is not exported from the module"; private final ProblemsHolder myHolder; private final ModuleFileIndex myModuleFileIndex; private final Set myExportedPackageNames; @@ -96,18 +97,39 @@ public class Java9NonAccessibleTypeExposedInspection extends BaseJavaLocalInspec } } + @Override + public void visitAnnotation(PsiAnnotation annotation) { + super.visitAnnotation(annotation); + PsiJavaCodeReferenceElement referenceElement = annotation.getNameReferenceElement(); + if (referenceElement != null) { + PsiElement resolved = referenceElement.resolve(); + if (resolved instanceof PsiClass && !isModulePublicApi((PsiClass)resolved)) { + PsiAnnotationOwner owner = annotation.getOwner(); + if (isModulePublicApi(owner)) { + myHolder.registerProblem(referenceElement, CLASS_IS_NOT_EXPORTED); + } + if (owner instanceof PsiParameter) { + PsiElement parent = ((PsiParameter)owner).getParent(); + if (parent instanceof PsiMember && isModulePublicApi((PsiMember)parent)) { + myHolder.registerProblem(referenceElement, CLASS_IS_NOT_EXPORTED); + } + } + } + } + } + private void checkType(@Nullable PsiType type, @Nullable PsiTypeElement typeElement) { if (typeElement != null) { PsiClass psiClass = PsiUtil.resolveClassInType(type); if (psiClass != null && isInModuleSource(psiClass) && !isModulePublicApi(psiClass)) { - myHolder.registerProblem(typeElement, "The class is not exported from the module"); + myHolder.registerProblem(typeElement, CLASS_IS_NOT_EXPORTED); } } } @Contract("null -> false") private boolean isModulePublicApi(@Nullable PsiMember member) { - if (member != null && isModulePublicApiMember(member)) { + if (member != null && (member.hasModifierProperty(PsiModifier.PUBLIC) || member.hasModifierProperty(PsiModifier.PROTECTED))) { PsiElement parent = member.getParent(); if (parent instanceof PsiClass) { return isModulePublicApi((PsiClass)parent); @@ -120,12 +142,27 @@ public class Java9NonAccessibleTypeExposedInspection extends BaseJavaLocalInspec return false; } - private static boolean isModulePublicApiMember(@NotNull PsiMember member) { - if (member.hasModifierProperty(PsiModifier.PUBLIC) || member.hasModifierProperty(PsiModifier.PROTECTED)) { - return true; + @Contract("null -> false") + private boolean isModulePublicApi(@Nullable PsiAnnotationOwner owner) { + if (owner instanceof PsiModifierList) { + PsiElement parent = ((PsiModifierList)owner).getParent(); + if (parent instanceof PsiMember) { + return isModulePublicApi((PsiMember)parent); + } + if (parent instanceof PsiParameter) { + PsiElement declarationScope = ((PsiParameter)parent).getDeclarationScope(); + if (declarationScope instanceof PsiMethod) { + return isModulePublicApi((PsiMethod)declarationScope); + } + } } - PsiClass containingClass = member.getContainingClass(); - return containingClass != null && containingClass.isInterface(); + else if (owner instanceof PsiTypeElement) { + PsiElement parent = ((PsiTypeElement)owner).getParent(); + if (parent instanceof PsiMember) { + return isModulePublicApi((PsiMember)parent); + } + } + return false; } private boolean isInModuleSource(@NotNull PsiClass psiClass) { diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/Java9NonAccessibleTypeExposedTest.kt b/java/java-tests/testSrc/com/intellij/codeInspection/Java9NonAccessibleTypeExposedTest.kt index 86965769e8b6..9961aa509e4b 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/Java9NonAccessibleTypeExposedTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInspection/Java9NonAccessibleTypeExposedTest.kt @@ -31,9 +31,9 @@ class Java9NonAccessibleTypeExposedTest : LightJava9ModulesCodeInsightFixtureTes super.setUp() myFixture.enableInspections(Java9NonAccessibleTypeExposedInspection()) addFile("module-info.java", "module MAIN { exports apiPkg; exports otherPkg; requires M2; }", MAIN) - addFile("apiPkg/PublicApi.java", "package apiPkg; public class PublicApi {}", MAIN) - addFile("apiPkg/PackageLocal.java", "package apiPkg; class PackageLocal {}", MAIN) - addFile("otherPkg/PublicOther.java", "package otherPkg; public class PublicOther {}", MAIN) + add("apiPkg", "PublicApi", "public class PublicApi {}") + add("apiPkg", "PackageLocal", "class PackageLocal {}") + add("otherPkg", "PublicOther", "public class PublicOther {}") } fun testPrimitives() { @@ -168,7 +168,7 @@ public interface Highlighted { } fun testNotExportedPackage() { - addFile("implPkg/NotExported.java", "package implPkg; public class NotExported {}", MAIN) + add("implPkg", "NotExported", "public class NotExported {}") highlight("""package apiPkg; import implPkg.NotExported; public class Highlighted { @@ -182,11 +182,11 @@ public class Highlighted { } fun testDoubleNested() { - addFile("apiPkg/PublicOuter.java", """package apiPkg; public class PublicOuter { + add("apiPkg", "PublicOuter", """public class PublicOuter { static class PackageLocal { public class DoubleNested {} } -}""", MAIN) +}""") highlight("""package apiPkg; import apiPkg.PublicOuter.PackageLocal; public class Highlighted { @@ -197,9 +197,56 @@ public class Highlighted { """) } + fun testPublicAnnotation() { + add("apiPkg", "MyAnnotation", "public @interface MyAnnotation {}") + highlight("""package apiPkg; +public class Highlighted { + @MyAnnotation public PublicApi field; + @MyAnnotation public Highlighted() {} + public Highlighted(@MyAnnotation PublicApi s) {field=s;} + @MyAnnotation protected void init() {} + protected @MyAnnotation PublicApi peek() {return field;} + public void set(@MyAnnotation PublicApi s) {field=s;} +} +""") + } + + fun testPackageLocalAnnotation() { + add("apiPkg", "MyAnnotation", "@interface MyAnnotation {}") + highlight("""package apiPkg; +public class Highlighted { + @MyAnnotation public PublicApi field; + @MyAnnotation public Highlighted() {} + public Highlighted(@MyAnnotation PublicApi s) {field=s;} + @MyAnnotation protected void init() {} + protected @MyAnnotation PublicApi peek() {return field;} + public void set(@MyAnnotation PublicApi s) {field=s;} +} +""") + } + + fun testNotExportedAnnotation() { + add("implPkg", "MyAnnotation", "public @interface MyAnnotation {}") + highlight("""package apiPkg; +import implPkg.MyAnnotation; +public class Highlighted { + @MyAnnotation public PublicApi field; + @MyAnnotation public Highlighted() {} + public Highlighted(@MyAnnotation PublicApi s) {field=s;} + @MyAnnotation protected void init() {} + protected @MyAnnotation PublicApi peek() {return field;} + public void set(@MyAnnotation PublicApi s) {field=s;} +} +""") + } + private fun highlight(@Language("JAVA") @NotNull @NonNls text: String) { val file = addFile("apiPkg/Highlighted.java", text, MAIN) myFixture.configureFromExistingVirtualFile(file) myFixture.checkHighlighting() } + + private fun add(packageName: String, className: String, @Language("JAVA") @NotNull @NonNls text: String) { + addFile("$packageName/$className.java", "package $packageName; $text", MAIN) + } }