From c6c4aa93ca8fc6239fddd302c6327c28065684bc Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 26 Aug 2016 19:23:02 +0300 Subject: [PATCH] [java] module 'provides' statement highlighting --- .../impl/analysis/HighlightVisitorImpl.java | 10 +++ .../impl/analysis/ModuleHighlightUtil.java | 87 ++++++++++++++++++- .../intellij/psi/PsiProvidesStatement.java | 4 + .../PsiJavaCodeReferenceElementImpl.java | 2 +- .../impl/source/PsiProvidesStatementImpl.java | 27 +++++- .../src/messages/JavaErrorMessages.properties | 6 ++ .../daemon/ModuleHighlightingTest.kt | 29 +++++++ 7 files changed, 158 insertions(+), 7 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 09c8e2b599a4..a9694ca70a7d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -1619,6 +1619,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkFileName(module, myFile)); if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkFileDuplicates(module, myFile)); if (!myHolder.hasErrorResults()) myHolder.addAll(ModuleHighlightUtil.checkDuplicateStatements(module)); + if (!myHolder.hasErrorResults()) myHolder.addAll(ModuleHighlightUtil.checkUnusedServices(module)); if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkFileLocation(module, myFile)); } @@ -1651,6 +1652,15 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh } } + @Override + public void visitProvidesStatement(PsiProvidesStatement statement) { + super.visitProvidesStatement(statement); + if (PsiUtil.isLanguageLevel9OrHigher(myFile)) { + PsiJavaCodeReferenceElement intRef = statement.getInterfaceReference(), implRef = statement.getImplementationReference(); + if (!myHolder.hasErrorResults()) myHolder.add(ModuleHighlightUtil.checkServiceImplementation(implRef, intRef)); + } + } + @Nullable private HighlightInfo checkFeature(@NotNull PsiElement element, @NotNull Feature feature) { return HighlightUtil.checkFeature(element, feature, myLanguageLevel, myFile); 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 649466b31446..e030a6fac7a6 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 @@ -31,22 +31,29 @@ import com.intellij.openapi.module.impl.scopes.ModulesScope; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.ProjectFileIndex; import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.search.FilenameIndex; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.JBIterable; import com.intellij.util.graph.Graph; 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; +import static com.intellij.openapi.util.Pair.pair; import static com.intellij.psi.PsiJavaModule.MODULE_INFO_FILE; import static com.intellij.psi.SyntaxTraverser.psiTraverser; @@ -101,6 +108,46 @@ public class ModuleHighlightUtil { st -> Optional.ofNullable(st.getClassReference()).map(ModuleHighlightUtil::refText), "module.duplicate.uses", results); + checkDuplicateRefs( + psiTraverser().children(module).filter(PsiProvidesStatement.class), + st -> Optional.of(pair(st.getInterfaceReference(), st.getImplementationReference())) + .map(p -> p.first != null && p.second != null ? refText(p.first) + " / " + refText(p.second) : null), + "module.duplicate.provides", results); + + return results; + } + + @NotNull + static List checkUnusedServices(@NotNull PsiJavaModule module) { + List results = ContainerUtil.newSmartList(); + + Set exports = ContainerUtil.newTroveSet(), uses = ContainerUtil.newTroveSet(); + for (PsiElement child : psiTraverser().children(module)) { + if (child instanceof PsiExportsStatement) { + PsiJavaCodeReferenceElement ref = ((PsiExportsStatement)child).getPackageReference(); + if (ref != null) exports.add(refText(ref)); + } + else if (child instanceof PsiUsesStatement) { + PsiJavaCodeReferenceElement ref = ((PsiUsesStatement)child).getClassReference(); + if (ref != null) uses.add(refText(ref)); + } + } + + Module host = ModuleUtilCore.findModuleForPsiElement(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) { + String className = refText(ref), packageName = StringUtil.getPackageName(className); + if (!exports.contains(packageName) && !uses.contains(className)) { + String message = JavaErrorMessages.message("module.service.unused"); + results.add(HighlightInfo.newHighlightInfo(HighlightInfoType.WARNING).range(range(ref)).description(message).create()); + } + } + } + } + return results; } @@ -108,7 +155,7 @@ public class ModuleHighlightUtil { Function> ref, @PropertyKey(resourceBundle = JavaErrorMessages.BUNDLE) String key, List results) { - Set filter = ContainerUtil.newHashSet(); + Set filter = ContainerUtil.newTroveSet(); for (T statement : statements) { String refText = ref.apply(statement).orElse(null); if (refText != null && !filter.add(refText)) { @@ -199,7 +246,7 @@ public class ModuleHighlightUtil { static List checkExportTargets(@NotNull PsiExportsStatement statement, @NotNull PsiJavaModule container) { List results = ContainerUtil.newSmartList(); - Set targets = ContainerUtil.newHashSet(); + Set targets = ContainerUtil.newTroveSet(); for (PsiJavaModuleReferenceElement refElement : psiTraverser().children(statement).filter(PsiJavaModuleReferenceElement.class)) { String refText = refElement.getReferenceText(); PsiPolyVariantReference ref = refElement.getReference(); @@ -235,6 +282,40 @@ public class ModuleHighlightUtil { return null; } + @Nullable + static HighlightInfo checkServiceImplementation(@Nullable PsiJavaCodeReferenceElement implRef, + @Nullable PsiJavaCodeReferenceElement intRef) { + if (implRef != null && intRef != null) { + PsiElement implTarget = implRef.resolve(), intTarget = intRef.resolve(); + if (implTarget instanceof PsiClass && intTarget instanceof PsiClass) { + PsiClass implClass = (PsiClass)implTarget; + if (!InheritanceUtil.isInheritorOrSelf(implClass, (PsiClass)intTarget, true)) { + String message = JavaErrorMessages.message("module.service.subtype"); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range(implRef)).description(message).create(); + } + if (implClass.hasModifierProperty(PsiModifier.ABSTRACT)) { + String message = JavaErrorMessages.message("module.service.abstract", implClass.getName()); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range(implRef)).description(message).create(); + } + + PsiMethod[] constructors = implClass.getConstructors(); + if (constructors.length > 0) { + PsiMethod constructor = JBIterable.of(constructors).find(c -> c.getParameterList().getParametersCount() == 0); + if (constructor == null) { + String message = JavaErrorMessages.message("module.service.no.ctor", implClass.getName()); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range(implRef)).description(message).create(); + } + if (!constructor.hasModifierProperty(PsiModifier.PUBLIC)) { + String message = JavaErrorMessages.message("module.service.hidden.ctor", implClass.getName()); + return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(range(implRef)).description(message).create(); + } + } + } + } + + return null; + } + private static QuickFixFactory factory() { return QuickFixFactory.getInstance(); } diff --git a/java/java-psi-api/src/com/intellij/psi/PsiProvidesStatement.java b/java/java-psi-api/src/com/intellij/psi/PsiProvidesStatement.java index 3cab855f393b..240fe20f5b09 100644 --- a/java/java-psi-api/src/com/intellij/psi/PsiProvidesStatement.java +++ b/java/java-psi-api/src/com/intellij/psi/PsiProvidesStatement.java @@ -15,10 +15,14 @@ */ package com.intellij.psi; +import org.jetbrains.annotations.Nullable; + /** * Represents a {@code provides} statement of a Java module declaration. * * @since 2016.3 */ public interface PsiProvidesStatement extends PsiElement { + @Nullable PsiJavaCodeReferenceElement getInterfaceReference(); + @Nullable PsiJavaCodeReferenceElement getImplementationReference(); } \ No newline at end of file diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiJavaCodeReferenceElementImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiJavaCodeReferenceElementImpl.java index 0bfd02ada81d..3a21b097a3d9 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiJavaCodeReferenceElementImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiJavaCodeReferenceElementImpl.java @@ -175,7 +175,7 @@ public class PsiJavaCodeReferenceElementImpl extends CompositePsiElement impleme PsiJavaCodeReferenceCodeFragment fragment = (PsiJavaCodeReferenceCodeFragment)treeParent.getPsi(); return fragment.isClassesAccepted() ? CLASS_FQ_OR_PACKAGE_NAME_KIND : PACKAGE_NAME_KIND; } - if (i == JavaElementType.USES_STATEMENT) { + if (i == JavaElementType.USES_STATEMENT || i == JavaElementType.PROVIDES_STATEMENT) { return CLASS_FQ_NAME_KIND; } diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiProvidesStatementImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiProvidesStatementImpl.java index 2d02057f4a30..be04449029e2 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiProvidesStatementImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/PsiProvidesStatementImpl.java @@ -15,18 +15,39 @@ */ package com.intellij.psi.impl.source; -import com.intellij.psi.JavaElementVisitor; -import com.intellij.psi.PsiElementVisitor; -import com.intellij.psi.PsiProvidesStatement; +import com.intellij.psi.*; import com.intellij.psi.impl.source.tree.CompositePsiElement; import com.intellij.psi.impl.source.tree.JavaElementType; +import com.intellij.psi.util.PsiUtil; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public class PsiProvidesStatementImpl extends CompositePsiElement implements PsiProvidesStatement { public PsiProvidesStatementImpl() { super(JavaElementType.PROVIDES_STATEMENT); } + @Nullable + @Override + public PsiJavaCodeReferenceElement getInterfaceReference() { + for (PsiElement child = getFirstChild(); child != null; child = child.getNextSibling()) { + if (child instanceof PsiJavaCodeReferenceElement) return (PsiJavaCodeReferenceElement)child; + if (PsiUtil.isJavaToken(child, JavaTokenType.WITH_KEYWORD)) break; + } + return null; + } + + @Nullable + @Override + public PsiJavaCodeReferenceElement getImplementationReference() { + boolean afterWith = false; + for (PsiElement child = getFirstChild(); child != null; child = child.getNextSibling()) { + if (afterWith && child instanceof PsiJavaCodeReferenceElement) return (PsiJavaCodeReferenceElement)child; + if (PsiUtil.isJavaToken(child, JavaTokenType.WITH_KEYWORD)) afterWith = true; + } + return null; + } + @Override public void accept(@NotNull PsiElementVisitor visitor) { if (visitor instanceof JavaElementVisitor) { diff --git a/java/java-psi-impl/src/messages/JavaErrorMessages.properties b/java/java-psi-impl/src/messages/JavaErrorMessages.properties index 379b27eab585..8511f2617a50 100644 --- a/java/java-psi-impl/src/messages/JavaErrorMessages.properties +++ b/java/java-psi-impl/src/messages/JavaErrorMessages.properties @@ -396,6 +396,7 @@ module.file.duplicate='module-info.java' already exists in the module module.duplicate.requires=Duplicate requires: {0} module.duplicate.export=Duplicate export: {0} module.duplicate.uses=Duplicate uses: {0} +module.duplicate.provides=Duplicate provides: {0} module.file.wrong.location=Module declaration should be located in a module's source root module.open.duplicate.text=Go to duplicate module.not.found=Module not found: {0} @@ -404,6 +405,11 @@ 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} +module.service.no.ctor=The service implementation does not have a default constructor: {0} +module.service.hidden.ctor=The default constructor of the service implementation is not public: {0} +module.service.unused=Service interface provided but not exported or used feature.generics=Generics feature.annotations=Annotations 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 a38b3287ac38..e7ae0a212d75 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ModuleHighlightingTest.kt @@ -44,6 +44,7 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { fun testDuplicateStatements() { addFile("pkg/main/C.java", "package pkg.main;\npublic class C { }") + addFile("pkg/main/Impl.java", "package pkg.main;\npublic class Impl extends C { }") doTest(""" module M { requires M2; @@ -52,6 +53,17 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { exports pkg. main; uses pkg.main.C; uses pkg. main . /*...*/ C; + provides pkg .main .C with pkg.main.Impl; + provides pkg.main.C with pkg. main. Impl; + }""".trimIndent(), true) + } + + fun testUnusedStatements() { + addFile("pkg/main/C.java", "package pkg.main;\npublic class C { }") + addFile("pkg/main/Impl.java", "package pkg.main;\npublic class Impl extends C { }") + doTest(""" + module M { + provides pkg.main.C with pkg.main.Impl; }""".trimIndent(), true) } @@ -89,6 +101,23 @@ class ModuleHighlightingTest : LightCodeInsightFixtureTestCase() { }""".trimIndent()) } + fun testProvides() { + addFile("pkg/main/C.java", "package pkg.main;\npublic interface C { }") + addFile("pkg/main/Impl1.java", "package pkg.main;\nclass Impl1 { }") + addFile("pkg/main/Impl2.java", "package pkg.main;\npublic class Impl2 { }") + addFile("pkg/main/Impl3.java", "package pkg.main;\npublic abstract class Impl3 implements C { }") + addFile("pkg/main/Impl4.java", "package pkg.main;\npublic class Impl4 implements C {\n public Impl4(String s) { }\n}") + addFile("pkg/main/Impl5.java", "package pkg.main;\npublic class Impl5 implements C {\n protected Impl5() { }\n}") + doTest(""" + module M { + provides pkg.main.C with pkg.main.Impl1; + provides pkg.main.C with pkg.main.Impl2; + provides pkg.main.C with pkg.main.Impl3; + provides pkg.main.C with pkg.main.Impl4; + provides pkg.main.C with pkg.main.Impl5; + }""".trimIndent()) + } + // private fun addFile(path: String, text: String) = VfsTestUtil.createFile(LightPlatformTestCase.getSourceRoot(), path, text)