From 06cc032edf924c1a12d1909f540c34b304523417 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Fri, 2 Dec 2016 18:28:41 +0300 Subject: [PATCH] Java: Created an inspection (with a quick fix) that checks if a package is exported to the same module where it's declared. Tests added. (IDEA-164129) --- .../impl/analysis/ModuleHighlightUtil.java | 9 +- ...oduleExportsPackageToItselfInspection.java | 128 ++++++++++++++++++ .../src/messages/JavaErrorMessages.properties | 1 - .../daemon/ModuleHighlightingTest.kt | 1 - .../Java9DeleteExportsToModuleFixTest.kt | 70 ++++++++++ .../Java9ModuleExportsPackageToItselfTest.kt | 55 ++++++++ .../src/messages/InspectionsBundle.properties | 5 + .../Java9ModuleExportsPackageToItself.html | 7 + resources/src/META-INF/IdeaPlugin.xml | 4 + 9 files changed, 273 insertions(+), 7 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleExportsPackageToItselfInspection.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/Java9DeleteExportsToModuleFixTest.kt create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/Java9ModuleExportsPackageToItselfTest.kt create mode 100644 resources-en/src/inspectionDescriptions/Java9ModuleExportsPackageToItself.html 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 b9c77368001c..62d827426b34 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 @@ -43,7 +43,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.PropertyKey; -import java.util.*; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -291,10 +294,6 @@ public class ModuleHighlightUtil { String message = JavaErrorMessages.message("module.duplicate.export", refText); results.add(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(refElement).description(message).create()); } - else if (target == container) { - String message = JavaErrorMessages.message("module.self.export"); - results.add(HighlightInfo.newHighlightInfo(HighlightInfoType.WARNING).range(refElement).description(message).create()); - } } return results; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleExportsPackageToItselfInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleExportsPackageToItselfInspection.java new file mode 100644 index 000000000000..96cd9ad2130f --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/java19modules/Java9ModuleExportsPackageToItselfInspection.java @@ -0,0 +1,128 @@ +/* + * Copyright 2000-2016 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.java19modules; + +import com.intellij.codeInsight.FileModificationService; +import com.intellij.codeInspection.*; +import com.intellij.openapi.project.Project; +import com.intellij.pom.java.LanguageLevel; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.List; + +/** + * @author Pavel.Dolgov + */ +public class Java9ModuleExportsPackageToItselfInspection extends BaseJavaLocalInspectionTool { + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + PsiFile file = holder.getFile(); + if (file instanceof PsiJavaFile) { + PsiJavaFile javaFile = (PsiJavaFile)file; + if (javaFile.getLanguageLevel().isAtLeast(LanguageLevel.JDK_1_9) && javaFile.getModuleDeclaration() != null) { + return new ExportedToSelfVisitor(holder); + } + } + return PsiElementVisitor.EMPTY_VISITOR; + } + + private static class ExportedToSelfVisitor extends JavaElementVisitor { + private final ProblemsHolder myHolder; + + public ExportedToSelfVisitor(ProblemsHolder holder) { myHolder = holder; } + + @Override + public void visitExportsStatement(PsiExportsStatement statement) { + super.visitExportsStatement(statement); + PsiJavaModule javaModule = PsiTreeUtil.getParentOfType(statement, PsiJavaModule.class); + if (javaModule != null) { + String moduleName = javaModule.getModuleName(); + List referenceElements = ContainerUtil.newArrayList(statement.getModuleReferences()); + for (PsiJavaModuleReferenceElement referenceElement : referenceElements) { + if (moduleName.equals(referenceElement.getReferenceText())) { + String message = InspectionsBundle.message(referenceElements.size() == 1 + ? "inspection.module.exports.package.to.itself.only.message" + : "inspection.module.exports.package.to.itself.message"); + myHolder.registerProblem(referenceElement, message, + new DeleteExportsToModuleFix(referenceElement)); + } + } + } + } + } + + private static class DeleteExportsToModuleFix implements LocalQuickFix { + private final String myModuleName; + + public DeleteExportsToModuleFix(PsiJavaModuleReferenceElement reference) { + myModuleName = reference.getReferenceText(); + } + + @Nls + @NotNull + @Override + public String getName() { + return InspectionsBundle.message("exports.to.itself.delete.module.fix.name", myModuleName); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("exports.to.itself.delete.module.fix.family.name"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement psiElement = descriptor.getPsiElement(); + if (!FileModificationService.getInstance().prepareFileForWrite(psiElement.getContainingFile())) return; + + PsiElement comma = findNearestComma(psiElement); + if (comma != null) { + comma.delete(); + } + else { + PsiKeyword keyword = PsiTreeUtil.getPrevSiblingOfType(psiElement, PsiKeyword.class); + if (keyword != null && keyword.getTokenType() == JavaTokenType.TO_KEYWORD) { + keyword.delete(); + } + } + psiElement.delete(); + } + + @Nullable + private static PsiElement findNearestComma(PsiElement psiElement) { + for (PsiElement next = psiElement.getNextSibling(); next != null; next = next.getNextSibling()) { + if (next instanceof PsiJavaToken && ((PsiJavaToken)next).getTokenType() == JavaTokenType.COMMA) { + return next; + } + } + for (PsiElement prev = psiElement.getPrevSibling(); prev != null; prev = prev.getPrevSibling()) { + if (prev instanceof PsiJavaToken && ((PsiJavaToken)prev).getTokenType() == JavaTokenType.COMMA) { + return prev; + } + } + return null; + } + } +} diff --git a/java/java-psi-impl/src/messages/JavaErrorMessages.properties b/java/java-psi-impl/src/messages/JavaErrorMessages.properties index fbe8ff23eb40..602d5a172451 100644 --- a/java/java-psi-impl/src/messages/JavaErrorMessages.properties +++ b/java/java-psi-impl/src/messages/JavaErrorMessages.properties @@ -405,7 +405,6 @@ module.not.on.path=Module is not in dependencies: {0} module.cyclic.dependence=Cyclic dependence: {0} package.not.found=Package not found: {0} package.is.empty=Package is empty: {0} -module.self.export=Exports to itself module.service.enum=The service definition is an enum: {0} module.service.subtype=The service implementation type must be a subtype of the service interface type module.service.abstract=The service implementation is an abstract class: {0} diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt index af13c4403f20..83d8c8a0500a 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt @@ -86,7 +86,6 @@ class ModuleHighlightingTest : LightJava9ModulesCodeInsightFixtureTestCase() { exports pkg.missing; exports pkg.empty; exports pkg.main to M.missing, M2, M2; - exports pkg.other to M; }""".trimIndent()) } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/Java9DeleteExportsToModuleFixTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/Java9DeleteExportsToModuleFixTest.kt new file mode 100644 index 000000000000..06880d54158f --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/Java9DeleteExportsToModuleFixTest.kt @@ -0,0 +1,70 @@ +/* + * Copyright 2000-2016 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.codeInsight.daemon.quickFix + +import com.intellij.codeInspection.InspectionsBundle +import com.intellij.codeInspection.java19modules.Java9ModuleExportsPackageToItselfInspection +import com.intellij.testFramework.fixtures.LightJava9ModulesCodeInsightFixtureTestCase +import com.intellij.testFramework.fixtures.MultiModuleJava9ProjectDescriptor +import org.intellij.lang.annotations.Language +import org.jetbrains.annotations.NonNls + +/** + * @author Pavel.Dolgov + */ +class Java9DeleteExportsToModuleFixTest : LightJava9ModulesCodeInsightFixtureTestCase() { + val message = InspectionsBundle.message("exports.to.itself.delete.module.fix.name", "M")!! + + override fun setUp() { + super.setUp() + myFixture.enableInspections(Java9ModuleExportsPackageToItselfInspection()) + + addFile("module-info.java", "module M2 { }", MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M2) + addFile("module-info.java", "module M4 { }", MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M4) + addFile("pkg/main/C.java", "package pkg.main; public class C {}") + } + + fun testOnlySelfModule() { + doFix("module M { exports pkg.main to M; }", + "module M { exports pkg.main; }") + } + + fun testOnlySelfModuleWithComments() { + doFix("module M { exports pkg.main to /*a*/ M /*b*/; }", + "module M { exports pkg.main /*a*/ /*b*/; }") + } + + fun testSelfModuleInList() { + doFix("module M { exports pkg.main to M2, M , M4; }", + "module M { exports pkg.main to M2, M4; }") + } + + fun testSelfModuleInListWithComments() { + doFix("module M { exports pkg.main to M2, /*a*/ M /*b*/,/*c*/ M4; }", + "module M { exports pkg.main to M2, /*a*/ /*b*//*c*/ M4; }") + } + + private fun doFix(textBefore: String, @Language("JAVA") @NonNls textAfter: String) { + myFixture.configureByText("module-info.java", textBefore) + + val action = myFixture.findSingleIntention(message) + assertNotNull(action) + myFixture.launchAction(action) + + myFixture.checkHighlighting() // no warning + myFixture.checkResult("module-info.java", textAfter, false) + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/Java9ModuleExportsPackageToItselfTest.kt b/java/java-tests/testSrc/com/intellij/codeInspection/Java9ModuleExportsPackageToItselfTest.kt new file mode 100644 index 000000000000..8c8a697a90d9 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/Java9ModuleExportsPackageToItselfTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2000-2016 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.codeInspection.java19modules.Java9ModuleExportsPackageToItselfInspection +import com.intellij.testFramework.fixtures.LightJava9ModulesCodeInsightFixtureTestCase +import com.intellij.testFramework.fixtures.MultiModuleJava9ProjectDescriptor + +/** + * @author Pavel.Dolgov + */ +class Java9ModuleExportsPackageToItselfTest : LightJava9ModulesCodeInsightFixtureTestCase() { + override fun setUp() { + super.setUp() + myFixture.enableInspections(Java9ModuleExportsPackageToItselfInspection()) + + addFile("module-info.java", "module M2 { }", MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M2) + addFile("module-info.java", "module M4 { }", MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M4) + addFile("pkg/main/C.java", "package pkg.main; public class C {}") + } + + fun testNoSelfModule() { + highlight("module M { exports pkg.main to M2, M4; }") + } + + fun testOnlySelfModule() { + val message = InspectionsBundle.message("inspection.module.exports.package.to.itself.only.message") + highlight("module M { exports pkg.main to M; }") + } + + fun testSelfModuleInList() { + val message = InspectionsBundle.message("inspection.module.exports.package.to.itself.message") + highlight("module M { exports pkg.main to M2, M, M4; }") + } + + private fun highlight(text: String) = highlight("module-info.java", text) + + private fun highlight(path: String, text: String) { + myFixture.configureFromExistingVirtualFile(addFile(path, text)) + myFixture.checkHighlighting() + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 0b9e98ef4fe7..31593a8dd658 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -744,3 +744,8 @@ nullable.stuff.inspection.navigate.null.argument.usages.view.name=''null'' argum inspection.excessive.lambda.message=Excessive lambda usage inspection.excessive.lambda.fix.family.name=Replace lambda with constant inspection.excessive.lambda.fix.name=Use ''{0}'' method without lambda + +inspection.module.exports.package.to.itself.message=Module exports package to itself +inspection.module.exports.package.to.itself.only.message=Module exports package only to itself +exports.to.itself.delete.module.fix.name=Delete reference to module ''{0}'' +exports.to.itself.delete.module.fix.family.name=Delete reference to module \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/Java9ModuleExportsPackageToItself.html b/resources-en/src/inspectionDescriptions/Java9ModuleExportsPackageToItself.html new file mode 100644 index 000000000000..0a6e3045008a --- /dev/null +++ b/resources-en/src/inspectionDescriptions/Java9ModuleExportsPackageToItself.html @@ -0,0 +1,7 @@ + + +The inspection detects a situation where a package is exported to the same Java 9 module where it's defined. +
Example: +module B { exports org.example to A, B, C; } + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index af99e8b1d513..11c1fca4d633 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -879,6 +879,10 @@ groupBundle="messages.InspectionsBundle" groupKey="group.names.visibility.issues" enabledByDefault="true" level="WARNING" displayName="Non-accessible type is exposed" implementationClass="com.intellij.codeInspection.java19modules.Java9NonAccessibleTypeExposedInspection"/> + com.intellij.codeInsight.intention.impl.SplitIfAction