From 35be5f5b7bd6eee1dd540ac3e7f6b6b1bb8c4840 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Thu, 13 Oct 2016 16:57:29 +0200 Subject: [PATCH] [java] package accessibility check for library classes --- .../impl/analysis/JavaModuleGraphUtil.java | 13 ++-- .../impl/analysis/ModuleHighlightUtil.java | 62 ++++++++++-------- .../testData/codeInsight/jigsaw/lib1.jar | Bin 0 -> 1218 bytes .../daemon/ModuleHighlightingTest.kt | 6 +- .../MultiModuleJava9ProjectDescriptor.kt | 5 ++ 5 files changed, 52 insertions(+), 34 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/jigsaw/lib1.jar diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java index f26cf534f6c0..170e6a8e5d2d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java @@ -152,11 +152,14 @@ public class JavaModuleGraphUtil { } public boolean reads(PsiJavaModule source, PsiJavaModule destination) { - Iterator directReaders = myGraph.getOut(destination); - while (directReaders.hasNext()) { - PsiJavaModule next = directReaders.next(); - if (source.equals(next) || myPublicEdges.contains(key(destination, next)) && reads(source, next)) { - return true; + Collection nodes = myGraph.getNodes(); + if (nodes.contains(destination) && nodes.contains(source)) { + Iterator directReaders = myGraph.getOut(destination); + while (directReaders.hasNext()) { + PsiJavaModule next = directReaders.next(); + if (source.equals(next) || myPublicEdges.contains(key(destination, next)) && reads(source, next)) { + return true; + } } } return false; diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java index f268eefe3391..6ee5c450a1bb 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/ModuleHighlightUtil.java @@ -55,16 +55,13 @@ import static com.intellij.psi.SyntaxTraverser.psiTraverser; public class ModuleHighlightUtil { @Nullable - static PsiJavaModule getModuleDescriptor(@NotNull PsiElement element) { - VirtualFile file = Optional.of(element) - .map(e -> e instanceof PsiFileSystemItem ? (PsiFileSystemItem)e : e.getContainingFile()) - .map(PsiFileSystemItem::getVirtualFile) - .orElse(null); + static PsiJavaModule getModuleDescriptor(@NotNull PsiFileSystemItem fsItem) { + VirtualFile file = fsItem.getVirtualFile(); if (file == null) return null; - Project project = element.getProject(); + Project project = fsItem.getProject(); ProjectFileIndex index = ProjectFileIndex.SERVICE.getInstance(project); - if (element instanceof PsiCompiledElement) { + if (index.isInLibraryClasses(file)) { return Optional.ofNullable(index.getClassRootForFile(file)) .map(r -> r.findChild(PsiJavaModule.MODULE_INFO_CLS_FILE)) .map(PsiManager.getInstance(project)::findFile) @@ -96,7 +93,7 @@ public class ModuleHighlightUtil { @Nullable static HighlightInfo checkFileDuplicates(@NotNull PsiJavaModule element, @NotNull PsiFile file) { - Module module = ModuleUtilCore.findModuleForPsiElement(element); + Module module = findModule(file); if (module != null) { Project project = file.getProject(); Collection others = FilenameIndex.getVirtualFilesByName(project, MODULE_INFO_FILE, module.getModuleScope(false)); @@ -173,12 +170,12 @@ public class ModuleHighlightUtil { } } - Module host = ModuleUtilCore.findModuleForPsiElement(module); + Module host = findModule(module); for (PsiProvidesStatement statement : psiTraverser().children(module).filter(PsiProvidesStatement.class)) { PsiJavaCodeReferenceElement ref = statement.getInterfaceReference(); if (ref != null) { PsiElement target = ref.resolve(); - if (target instanceof PsiClass && ModuleUtilCore.findModuleForPsiElement(target) == host) { + if (target instanceof PsiClass && findModule(target) == host) { String className = refText(ref), packageName = StringUtil.getPackageName(className); if (!exports.contains(packageName) && !uses.contains(className)) { String message = JavaErrorMessages.message("module.service.unused"); @@ -243,7 +240,7 @@ public class ModuleHighlightUtil { if (refElement != null) { PsiElement target = refElement.resolve(); if (target instanceof PsiPackage) { - Module module = ModuleUtilCore.findModuleForPsiElement(refElement); + Module module = findModule(refElement); if (module != null) { String packageName = ((PsiPackage)target).getQualifiedName(); PsiDirectory[] directories = ((PsiPackage)target).getDirectories(module.getModuleScope(false)); @@ -339,12 +336,12 @@ public class ModuleHighlightUtil { static HighlightInfo checkPackageAccessibility(@NotNull PsiJavaCodeReferenceElement ref, @NotNull PsiElement target, @NotNull PsiJavaModule refModule) { - Module module = ModuleUtilCore.findModuleForPsiElement(ref); + Module module = findModule(refModule); if (module != null) { - if (target instanceof PsiClass && !(target instanceof PsiCompiledElement) && module != ModuleUtilCore.findModuleForPsiElement(target)) { + if (target instanceof PsiClass) { PsiElement targetFile = target.getParent(); if (targetFile instanceof PsiClassOwner) { - PsiJavaModule targetModule = getModuleDescriptor(target); + PsiJavaModule targetModule = getModuleDescriptor((PsiFileSystemItem)targetFile); String packageName = ((PsiClassOwner)targetFile).getPackageName(); return checkPackageAccessibility(ref, refModule, targetModule, packageName); } @@ -353,7 +350,7 @@ public class ModuleHighlightUtil { PsiElement refImport = ref.getParent(); if (refImport instanceof PsiImportStatementBase && ((PsiImportStatementBase)refImport).isOnDemand()) { PsiDirectory[] dirs = ((PsiPackage)target).getDirectories(module.getModuleWithDependenciesAndLibrariesScope(false)); - if (dirs.length == 1 && ModuleUtilCore.findModuleForPsiElement(dirs[0]) != module) { + if (dirs.length == 1) { PsiJavaModule targetModule = getModuleDescriptor(dirs[0]); String packageName = ((PsiPackage)target).getQualifiedName(); return checkPackageAccessibility(ref, refModule, targetModule, packageName); @@ -369,26 +366,35 @@ public class ModuleHighlightUtil { PsiJavaModule refModule, PsiJavaModule targetModule, String packageName) { - if (targetModule == null) { - String message = JavaErrorMessages.message("module.package.on.classpath"); - return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); - } + if (!refModule.equals(targetModule)) { + if (targetModule == null) { + String message = JavaErrorMessages.message("module.package.on.classpath"); + return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); + } - String refModuleName = refModule.getModuleName(); - String requiredName = targetModule.getModuleName(); - if (!(targetModule instanceof PsiCompiledElement) && !JavaModuleGraphUtil.exports(targetModule, packageName, refModule)) { - String message = JavaErrorMessages.message("module.package.not.exported", requiredName, packageName, refModuleName); - return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); - } + String refModuleName = refModule.getModuleName(); + String requiredName = targetModule.getModuleName(); + if (!JavaModuleGraphUtil.exports(targetModule, packageName, refModule)) { + String message = JavaErrorMessages.message("module.package.not.exported", requiredName, packageName, refModuleName); + return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); + } - if (!(PsiJavaModule.JAVA_BASE.equals(requiredName) || JavaModuleGraphUtil.reads(refModule, targetModule))) { - String message = JavaErrorMessages.message("module.not.in.requirements", refModuleName, requiredName); - return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); + if (!(PsiJavaModule.JAVA_BASE.equals(requiredName) || JavaModuleGraphUtil.reads(refModule, targetModule))) { + String message = JavaErrorMessages.message("module.not.in.requirements", refModuleName, requiredName); + return HighlightInfo.newHighlightInfo(HighlightInfoType.WRONG_REF).range(ref).description(message).create(); + } } return null; } + private static Module findModule(PsiElement element) { + return Optional.ofNullable(element.getContainingFile()) + .map(PsiFile::getVirtualFile) + .map(f -> ModuleUtilCore.findModuleForFile(f, element.getProject())) + .orElse(null); + } + private static HighlightInfo moduleResolveError(PsiJavaModuleReferenceElement refElement, PsiPolyVariantReference ref) { boolean missing = ref.multiResolve(true).length == 0; String message = JavaErrorMessages.message(missing ? "module.not.found" : "module.not.on.path", refElement.getReferenceText()); diff --git a/java/java-tests/testData/codeInsight/jigsaw/lib1.jar b/java/java-tests/testData/codeInsight/jigsaw/lib1.jar new file mode 100644 index 0000000000000000000000000000000000000000..f719fc6306be74694a6e572a49da09973daeaff4 GIT binary patch literal 1218 zcmWIWW@h1HVBlb2I8^KF$$$h{7+4qzveWhdonl}Jz^RH8r~{-bCo{=VAEZo#frEpC zVOABYVM55te4GvSl5-M^i+%eJ@*NBiaDAV#HDu)vW^0{25fOsYouP-DZtTCt-JiAe zRDGnUPEA9<(B(}Alkc3_TdX$o`tOe{1}>Ky8+Y}#@%AV5maf^%dGJNab(Z2;IU)x? zm#%p>J$b>UsXR)4QFTJ6GIWFI{*KQ`v#wP$`Exl@TS=Cu#a_0DiTc?jsgKR}E-p!kqOATzfh2Pw2_YEVN<8bd8G$UK31 zAi~lJ@*Bz1rAne?Y#} zqI250L$ho9-ygj5f4w6IdtI)aR=82H!|3q)yjGNCn_raIF!fx%#3;WfTn_gy|`#t%=+DYD0D`%d|{6CeS z|C9T#4S{M~?wfI*og?UW%z7gazt*V>uAT4Cwf`ubc{LFfgzWVT;{Q!~u*u3bW8tA! z_KYD9MZZQl{^SP*A^-6etIh#EEyM^3LP#Pyh?XJ+fhuzIQ%ZAEbu;tQ@*#m?{Wbr( z9UB{4FdJVPTbUc%x(9zQEqW5N{mh&YH@3z*f2LfT6LR;=o-H+d*Bsg8;>Knczd=Bm zS-oTZ^eGWMDa9fw%U?fFQIlla`lG^1@|E(-?1Y4blmGZfHxx(7X$9h15|LDd05u>JDQht@i7+5C7jmKl zWiABp1v25<;8_iyV?jw10sMh+0M`#qs0h13NfkM^KuHw=sxkDUBwUaqV4eU)E^=ss cA{PNBF=2`70B=?{kPpkg.m5.C5; import pkg.m7.C7; + import pkg.lib1.LC1; + import pkg.lib1.impl.LC1Impl; + import pkg.lib1.impl.*; + import static pkg.m2.impl.C2Impl.make; class C { } diff --git a/java/java-tests/testSrc/com/intellij/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt b/java/java-tests/testSrc/com/intellij/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt index cdc211e11e3c..f29828fc4d91 100644 --- a/java/java-tests/testSrc/com/intellij/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt +++ b/java/java-tests/testSrc/com/intellij/testFramework/fixtures/MultiModuleJava9ProjectDescriptor.kt @@ -15,6 +15,7 @@ */ package com.intellij.testFramework.fixtures +import com.intellij.openapi.application.ex.PathManagerEx import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.module.Module import com.intellij.openapi.module.ModuleManager @@ -49,6 +50,7 @@ object MultiModuleJava9ProjectDescriptor : DefaultLightProjectDescriptor() { override fun setUpProject(project: Project, handler: SetupHandler) { super.setUpProject(project, handler) + runWriteAction { val main = ModuleManager.getInstance(project).findModuleByName(TEST_MODULE_NAME)!! @@ -68,6 +70,9 @@ object MultiModuleJava9ProjectDescriptor : DefaultLightProjectDescriptor() { val m7 = makeModule(project, ModuleDescriptor.M7) ModuleRootModificationUtil.addDependency(m6, m7, DependencyScope.COMPILE, true) + + val libDir = "jar://${PathManagerEx.getTestDataPath()}/codeInsight/jigsaw/" + ModuleRootModificationUtil.addModuleLibrary(main, libDir + "lib1.jar!/") } }