diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddRequiredModuleFix.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddRequiredModuleFix.java index 1f41b4ac15e1..3fcd6db0f323 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddRequiredModuleFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddRequiredModuleFix.java @@ -16,12 +16,11 @@ package com.intellij.codeInsight.daemon.impl.quickfix; import com.intellij.codeInsight.daemon.QuickFixBundle; -import com.intellij.codeInsight.intention.IntentionAction; +import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; -import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -30,12 +29,11 @@ import org.jetbrains.annotations.Nullable; /** * @author Pavel.Dolgov */ -public class AddRequiredModuleFix implements IntentionAction { - private final SmartPsiElementPointer myModulePointer; +public class AddRequiredModuleFix extends LocalQuickFixAndIntentionActionOnPsiElement { private final String myRequiredName; public AddRequiredModuleFix(PsiJavaModule module, String requiredName) { - myModulePointer = SmartPointerManager.getInstance(module.getProject()).createSmartPsiElementPointer(module); + super(module); myRequiredName = requiredName; } @@ -54,16 +52,23 @@ public class AddRequiredModuleFix implements IntentionAction { } @Override - public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) { - if (!PsiUtil.isLanguageLevel9OrHigher(file)) return false; - PsiJavaModule module = myModulePointer.getElement(); - return module != null && module.isValid() && module.getManager().isInProject(module) && getLBrace(module) != null; + public boolean isAvailable(@NotNull Project project, + @NotNull PsiFile file, + @NotNull PsiElement startElement, + @NotNull PsiElement endElement) { + return PsiUtil.isLanguageLevel9OrHigher(file) && + startElement instanceof PsiJavaModule && + startElement.getManager().isInProject(startElement) && + getLBrace((PsiJavaModule)startElement) != null; } @Override - public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { - PsiJavaModule module = myModulePointer.getElement(); - if (module == null) return; + public void invoke(@NotNull Project project, + @NotNull PsiFile file, + @Nullable Editor editor, + @NotNull PsiElement startElement, + @NotNull PsiElement endElement) { + PsiJavaModule module = (PsiJavaModule)startElement; PsiJavaParserFacade parserFacade = JavaPsiFacade.getInstance(project).getParserFacade(); PsiJavaModule tempModule = parserFacade.createModuleFromText("module " + module.getName() + " { requires " + myRequiredName + "; }"); diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/Java9ReflectionClassVisibilityInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/Java9ReflectionClassVisibilityInspection.java new file mode 100644 index 000000000000..3cedcdbacdfa --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/Java9ReflectionClassVisibilityInspection.java @@ -0,0 +1,129 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.reflectiveAccess; + +import com.intellij.codeInsight.daemon.JavaErrorMessages; +import com.intellij.codeInsight.daemon.impl.analysis.JavaModuleGraphUtil; +import com.intellij.codeInsight.daemon.impl.quickfix.AddRequiredModuleFix; +import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import org.jetbrains.annotations.NotNull; + +import java.util.List; + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_CLASS; +import static com.intellij.psi.impl.source.resolve.reference.impl.JavaReflectionReferenceUtil.*; + +/** + * @author Pavel.Dolgov + */ +public class Java9ReflectionClassVisibilityInspection extends BaseJavaBatchLocalInspectionTool { + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + final PsiFile file = holder.getFile(); + if (PsiUtil.isLanguageLevel9OrHigher(file)) { + final PsiJavaModule javaModule = JavaModuleGraphUtil.findDescriptorByElement(file); + if (javaModule != null) { + return new JavaElementVisitor() { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + + if (isCallToMethod(expression, JAVA_LANG_CLASS, FOR_NAME) || isCallToMethod(expression, JAVA_LANG_CLASS_LOADER, LOAD_CLASS)) { + checkClassVisibility(expression, holder, javaModule); + } + } + }; + } + } + + return PsiElementVisitor.EMPTY_VISITOR; + } + + private static void checkClassVisibility(@NotNull PsiMethodCallExpression callExpression, + @NotNull ProblemsHolder holder, + @NotNull PsiJavaModule javaModule) { + + final PsiExpression[] arguments = callExpression.getArgumentList().getExpressions(); + if (arguments.length != 0) { + final PsiExpression classNameArgument = arguments[0]; + final String className = computeConstantExpression(classNameArgument, String.class); + if (className != null) { + final Project project = holder.getProject(); + final PsiClass psiClass = JavaPsiFacade.getInstance(project).findClass(className, GlobalSearchScope.allScope(project)); + if (psiClass != null) { + final PsiJavaModule otherModule = JavaModuleGraphUtil.findDescriptorByElement(psiClass); + if (otherModule != null && otherModule != javaModule) { + if (!JavaModuleGraphUtil.reads(javaModule, otherModule)) { + String message = JavaErrorMessages.message( + "module.not.in.requirements", javaModule.getName(), otherModule.getName()); + holder.registerProblem(classNameArgument, message, new AddRequiredModuleFix(javaModule, otherModule.getName())); + return; + } + + if (otherModule.hasModifierProperty(PsiModifier.OPEN)) { + return; + } + final PsiJavaFile file = PsiTreeUtil.getParentOfType(psiClass, PsiJavaFile.class); + if (file != null) { + final String packageName = file.getPackageName(); + if (isPackageAccessible(otherModule.getOpens(), packageName, javaModule)) { + return; + } + final boolean publicApi = isPublicApi(psiClass); + if (publicApi && isPackageAccessible(otherModule.getExports(), packageName, javaModule)) { + return; + } + final String message = JavaErrorMessages.message( + publicApi ? "module.package.not.exported" : "module.package.not.open", + otherModule.getName(), packageName, javaModule.getName()); + holder.registerProblem(classNameArgument, message); + } + } + } + } + } + } + + private static boolean isPackageAccessible(@NotNull Iterable statements, + @NotNull String packageName, + @NotNull PsiJavaModule javaModule) { + for (PsiPackageAccessibilityStatement statement : statements) { + if (packageName.equals(statement.getPackageName())) { + final List moduleNames = statement.getModuleNames(); + if (moduleNames.isEmpty() || moduleNames.contains(javaModule.getName())) { + return true; + } + } + } + return false; + } + + private static boolean isPublicApi(@NotNull PsiClass psiClass) { + if (psiClass.hasModifierProperty(PsiModifier.PUBLIC) || psiClass.hasModifierProperty(PsiModifier.PROTECTED)) { + final PsiElement parent = psiClass.getParent(); + return parent instanceof PsiJavaFile || parent instanceof PsiClass && isPublicApi((PsiClass)parent); + } + return false; + } +} diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java index 6d210a11311e..7e4dd3af7055 100644 --- a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java @@ -20,7 +20,6 @@ import com.intellij.codeInspection.InspectionsBundle; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; -import com.siyeh.ig.psiutils.MethodCallUtils; import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -213,10 +212,6 @@ public class JavaReflectionInvocationInspection extends BaseJavaBatchLocalInspec return null; } - private static boolean isCallToMethod(PsiMethodCallExpression methodCall, String className, String methodName) { - return MethodCallUtils.isCallToMethod(methodCall, className, null, methodName, (PsiType[])null); - } - private static class Arguments { final PsiExpression[] expressions; final boolean varargAsArray; diff --git a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java index 670c5eae7d60..7bd2c2bc9442 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java @@ -32,6 +32,7 @@ import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.PlatformIcons; import com.siyeh.ig.psiutils.DeclarationSearchUtils; +import com.siyeh.ig.psiutils.MethodCallUtils; import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -160,6 +161,7 @@ public class JavaReflectionReferenceUtil { return null; } + @Contract("null,_->null") @Nullable public static T computeConstantExpression(@Nullable PsiExpression expression, @NotNull Class expectedType) { expression = ParenthesesUtils.stripParentheses(expression); @@ -312,6 +314,10 @@ public class JavaReflectionReferenceUtil { return JAVA_LANG_INVOKE_METHOD_TYPE + "." + METHOD_TYPE + types; } + public static boolean isCallToMethod(@NotNull PsiMethodCallExpression methodCall, @NotNull String className, @NotNull String methodName) { + return MethodCallUtils.isCallToMethod(methodCall, className, null, methodName, (PsiType[])null); + } + public static class ReflectiveType { final PsiClass myPsiClass; diff --git a/java/java-psi-impl/src/messages/JavaErrorMessages.properties b/java/java-psi-impl/src/messages/JavaErrorMessages.properties index c1be49910e57..27fc884a7529 100644 --- a/java/java-psi-impl/src/messages/JavaErrorMessages.properties +++ b/java/java-psi-impl/src/messages/JavaErrorMessages.properties @@ -423,6 +423,7 @@ module.service.no.ctor=The service implementation does not have a public default module.service.provider.type=The ''provider'' method return type must be a subtype of the service interface type: {0} module.service.unused=Service interface provided but not exported or used module.package.not.exported=The module ''{0}'' does not export the package ''{1}'' to the module ''{2}'' +module.package.not.open=The module ''{0}'' does not open the package ''{1}'' to the module ''{2}'' module.package.on.classpath=A named module cannot access packages of an unnamed one module.not.in.requirements=The module ''{0}'' does not have the module ''{1}'' in requirements module.conflicting.reads=Module ''{0}'' reads package ''{1}'' from both ''{2}'' and ''{3}'' diff --git a/java/java-tests/testData/inspection/java9ReflectionClassVisibility/ExportsPackage.java b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/ExportsPackage.java new file mode 100644 index 000000000000..b386e7015471 --- /dev/null +++ b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/ExportsPackage.java @@ -0,0 +1,9 @@ +package my.main; + +class Main { + void foo() throws Exception { + Class.forName("my.api.Api"); + Class.forName("my.impl.Impl"); + Class.forName("my.api.PackageLocal"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9ReflectionClassVisibility/NotInRequirements.java b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/NotInRequirements.java new file mode 100644 index 000000000000..dacc7495e645 --- /dev/null +++ b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/NotInRequirements.java @@ -0,0 +1,8 @@ +package my.main; + +class Main { + void foo() throws Exception { + Class.forName("my.api.Api"); + Class.forName("my.impl.Impl"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpenModule.java b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpenModule.java new file mode 100644 index 000000000000..61667bc2e587 --- /dev/null +++ b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpenModule.java @@ -0,0 +1,8 @@ +package my.main; + +class Main { + void foo() throws Exception { + Class.forName("my.api.Api"); + Class.forName("my.impl.Impl"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpensPackage.java b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpensPackage.java new file mode 100644 index 000000000000..db08636f0a88 --- /dev/null +++ b/java/java-tests/testData/inspection/java9ReflectionClassVisibility/OpensPackage.java @@ -0,0 +1,8 @@ +package my.main; + +class Main { + void foo() throws Exception { + Class.forName("my.api.Api"); + Class.forName("my.impl.Impl"); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/Java9ReflectionClassVisibilityTest.kt b/java/java-tests/testSrc/com/intellij/codeInspection/Java9ReflectionClassVisibilityTest.kt new file mode 100644 index 000000000000..9747162c0cbc --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/Java9ReflectionClassVisibilityTest.kt @@ -0,0 +1,85 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection + +import com.intellij.JavaTestUtil +import com.intellij.codeInspection.reflectiveAccess.Java9ReflectionClassVisibilityInspection +import com.intellij.openapi.util.io.FileUtil +import com.intellij.testFramework.fixtures.LightJava9ModulesCodeInsightFixtureTestCase +import com.intellij.testFramework.fixtures.MultiModuleJava9ProjectDescriptor.ModuleDescriptor +import com.intellij.testFramework.fixtures.MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M2 +import com.intellij.testFramework.fixtures.MultiModuleJava9ProjectDescriptor.ModuleDescriptor.MAIN +import org.intellij.lang.annotations.Language + +/** + * @author Pavel.Dolgov + */ +class Java9ReflectionClassVisibilityTest : LightJava9ModulesCodeInsightFixtureTestCase() { + + override fun getBasePath() = JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/java9ReflectionClassVisibility" + + override fun setUp() { + super.setUp() + myFixture.enableInspections(Java9ReflectionClassVisibilityInspection()) + } + + fun testOpenModule() { + moduleInfo("module MAIN { requires API; }", MAIN) + moduleInfo("open module API { }", M2) + doTest() + } + + fun testOpensPackage() { + moduleInfo("module MAIN { requires API; }", MAIN) + moduleInfo("module API { opens my.api; }", M2) + doTest() + } + + fun testExportsPackage() { + moduleInfo("module MAIN { requires API; }", MAIN) + moduleInfo("module API { exports my.api; }", M2) + javaClass("my.api", "PackageLocal", M2, "") + doTest() + } + + fun testNotInRequirements() { + moduleInfo("module MAIN { }", MAIN) + moduleInfo("open module API { }", M2) + doTest() + } + + private fun doTest() { + javaClass("my.api", "Api", M2) + javaClass("my.impl", "Impl", M2) + + val testPath = "$testDataPath/${getTestName(false)}.java" + val mainFile = FileUtil.findFirstThatExist(testPath) + assertNotNull("Test data: $testPath", mainFile) + val mainText = String(FileUtil.loadFileText(mainFile!!)) + + myFixture.configureFromExistingVirtualFile(addFile("my/main/Main.java", mainText, MAIN)) + myFixture.checkHighlighting() + } + + + private fun moduleInfo(@Language("JAVA") moduleInfoText: String, descriptor: ModuleDescriptor) { + addFile("module-info.java", moduleInfoText, descriptor) + } + + private fun javaClass(packageName: String, className: String, descriptor: ModuleDescriptor, modifier: String = "public") { + addFile("${packageName.replace('.', '/')}/$className.java", "package $packageName; $modifier class $className {}", descriptor) + } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index b6a9271a8f64..2e2ffa1d3f61 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -819,4 +819,6 @@ inspection.reflection.invocation.argument.count={0,choice,0#No arguments are|1#O inspection.reflection.invocation.item.count={0,choice,0#Empty array is|1#Single-item array is|1<{0} array items are} expected inspection.reflection.invocation.argument.not.assignable=Argument is not assignable to ''{0}'' inspection.reflection.invocation.item.not.assignable=Array item is not assignable to ''{0}'' -inspection.reflection.invocation.array.not.assignable=Array {0,choice,1#item has|1 + +This inspection detects reflective access to classes which aren't visible due to Java 9 module accessibility rules. + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 160f2f5a7490..c70fa93b9482 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -920,6 +920,10 @@ groupPath="Java" groupBundle="messages.InspectionsBundle" groupKey="group.names.reflective.access.issues" bundle="messages.InspectionsBundle" key="inspection.handle.signature.name" implementationClass="com.intellij.codeInspection.reflectiveAccess.JavaLangInvokeHandleSignatureInspection"/> +