diff --git a/plugins/devkit/devkit-core/src/inspections/ContentModuleVisibilityInspection.kt b/plugins/devkit/devkit-core/src/inspections/ContentModuleVisibilityInspection.kt index 6baeee668c8c..8b1ad20ff72d 100644 --- a/plugins/devkit/devkit-core/src/inspections/ContentModuleVisibilityInspection.kt +++ b/plugins/devkit/devkit-core/src/inspections/ContentModuleVisibilityInspection.kt @@ -9,11 +9,14 @@ import com.intellij.codeInspection.util.IntentionName import com.intellij.ide.highlighter.XmlFileType import com.intellij.openapi.fileEditor.UniqueVFilePathBuilder import com.intellij.openapi.project.Project +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.psi.PsiManager import com.intellij.psi.PsiReference import com.intellij.psi.SmartPsiElementPointer import com.intellij.psi.createSmartPointer import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.GlobalSearchScopesCore +import com.intellij.psi.search.ProjectScope import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.util.parentOfType import com.intellij.psi.xml.XmlFile @@ -54,10 +57,10 @@ internal class ContentModuleVisibilityInspection : DevKitPluginXmlInspectionBase holder: DomElementAnnotationHolder, ) { val currentXmlFile = dependencyValue.xmlElement?.containingFile as? XmlFile ?: return - val productionXmlFilesScope = getProjectProductionXmlFilesScope(currentXmlFile.project) - val currentModuleIncludingFiles = getInclusionContextsForContentModuleOrPluginXmlFile(currentXmlFile, productionXmlFilesScope) + val xmlFilesScope = getXmlFilesScope(currentXmlFile.project) + val currentModuleIncludingFiles = getInclusionContextsForContentModuleOrPluginXmlFile(currentXmlFile, xmlFilesScope) val dependencyXmlFile = moduleDependency.xmlElement?.containingFile as? XmlFile ?: return - val dependencyIncludingFiles = getInclusionContextsForContentModuleOrPluginXmlFile(dependencyXmlFile, productionXmlFilesScope) + val dependencyIncludingFiles = getInclusionContextsForContentModuleOrPluginXmlFile(dependencyXmlFile, xmlFilesScope) for (currentModuleInclusionContext in currentModuleIncludingFiles) { for (dependencyInclusionContext in dependencyIncludingFiles) { @@ -149,15 +152,39 @@ internal class ContentModuleVisibilityInspection : DevKitPluginXmlInspectionBase val moduleVirtualFile = xmlFile.virtualFile ?: return emptyList() val psiManager = xmlFile.manager return PluginIdDependenciesIndex.findFilesIncludingContentModule(moduleVirtualFile, scope) - .mapNotNull { psiManager.findFile(it) as? XmlFile } - .flatMap { xmlFile -> - val currentDescriptor = DescriptorUtil.getIdeaPlugin(xmlFile) ?: return@flatMap emptyList() - getRootIncludingPlugins(xmlFile, currentDescriptor, registrationPlace = currentDescriptor, scope) - } + .mapToXmlFileAndIdeaPlugin(psiManager) + .withoutLibraryDuplicates(xmlFile.project) + .flatMap { (xmlFile, ideaPlugin) -> getRootIncludingPlugins(xmlFile, ideaPlugin, registrationPlace = ideaPlugin, scope) } .distinct() .sortedWith(compareBy { it.rootPlugin.pluginIdOrPlainFileName }.thenBy { it.registrationPlace.pluginIdOrPlainFileName }) } + private fun Collection.mapToXmlFileAndIdeaPlugin(psiManager: PsiManager): List> { + return mapNotNull { + val xmlFile = psiManager.findFile(it) as? XmlFile ?: return@mapNotNull null + val ideaPlugin = DescriptorUtil.getIdeaPlugin(xmlFile) ?: return@mapNotNull null + xmlFile to ideaPlugin + } + } + + private fun List>.withoutLibraryDuplicates(project: Project): List> { + val productionScope = GlobalSearchScopesCore.projectProductionScope(project) + return groupBy { it.second.pluginIdOrPlainFileName } + .flatMap { (_, files) -> + when { + files.size == 1 -> files + else -> { + val productionFiles = files.filter { productionScope.contains(it.first.virtualFile) } + return if (productionFiles.size < files.size && productionFiles.isNotEmpty()) { + productionFiles + } else { + files + } + } + } + } + } + private data class ContentModuleInclusionContext( /** Element of the root `plugin.xml` (or `Plugin.xml`) which includes the content module. */ val rootPlugin: IdeaPlugin, @@ -193,10 +220,10 @@ internal class ContentModuleVisibilityInspection : DevKitPluginXmlInspectionBase ) { val currentXmlFile = dependencyValue.xmlElement?.containingFile as? XmlFile ?: return val project = currentXmlFile.project - val productionXmlFilesScope = getProjectProductionXmlFilesScope(project) - val currentModuleInclusionContexts = getInclusionContextsForContentModuleOrPluginXmlFile(currentXmlFile, productionXmlFilesScope) + val xmlFilesScope = getXmlFilesScope(project) + val currentModuleInclusionContexts = getInclusionContextsForContentModuleOrPluginXmlFile(currentXmlFile, xmlFilesScope) val dependencyXmlFile = moduleDependency.xmlElement?.containingFile as? XmlFile ?: return - val dependencyInclusionContexts = getInclusionContextsForContentModuleOrPluginXmlFile(dependencyXmlFile, productionXmlFilesScope) + val dependencyInclusionContexts = getInclusionContextsForContentModuleOrPluginXmlFile(dependencyXmlFile, xmlFilesScope) for (currentModuleInclusionContext in currentModuleInclusionContexts) { val currentModuleIncludingPlugin = currentModuleInclusionContext.rootPlugin if (dependencyInclusionContexts.any { it.rootPlugin == currentModuleIncludingPlugin }) continue // are included in the same plugin @@ -234,8 +261,13 @@ internal class ContentModuleVisibilityInspection : DevKitPluginXmlInspectionBase } } - private fun getProjectProductionXmlFilesScope(project: Project): GlobalSearchScope { - return GlobalSearchScope.getScopeRestrictedByFileTypes(GlobalSearchScopesCore.projectProductionScope(project), XmlFileType.INSTANCE) + private fun getXmlFilesScope(project: Project): GlobalSearchScope { + val productionScope = GlobalSearchScopesCore.projectProductionScope(project) + val librariesScope = ProjectScope.getLibrariesScope(project) + return GlobalSearchScope.getScopeRestrictedByFileTypes( + productionScope.union(librariesScope), + XmlFileType.INSTANCE + ) } /** diff --git a/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module-another-namespace.jar b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module-another-namespace.jar new file mode 100644 index 000000000000..20ec753df8e8 Binary files /dev/null and b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module-another-namespace.jar differ diff --git a/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module.jar b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module.jar new file mode 100644 index 000000000000..70c7689daa7a Binary files /dev/null and b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.plugin.with.internal.module.jar differ diff --git a/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.private.module.jar b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.private.module.jar new file mode 100644 index 000000000000..69c514a43eb8 Binary files /dev/null and b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.private.module.jar differ diff --git a/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.public.or.private.module.jar b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.public.or.private.module.jar new file mode 100644 index 000000000000..2db9260f2207 Binary files /dev/null and b/plugins/devkit/devkit-java-tests/testData/contentModules/com.example.public.or.private.module.jar differ diff --git a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/ContentModuleVisibilityInspectionTest.kt b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/ContentModuleVisibilityInspectionTest.kt index e75e969de84f..209935b72e57 100644 --- a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/ContentModuleVisibilityInspectionTest.kt +++ b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/ContentModuleVisibilityInspectionTest.kt @@ -6,6 +6,7 @@ import com.intellij.testFramework.PsiTestUtil import com.intellij.testFramework.fixtures.CodeInsightTestFixture import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase import org.intellij.lang.annotations.Language +import org.jetbrains.idea.devkit.DevkitJavaTestsUtil import org.jetbrains.idea.devkit.module.PluginModuleType abstract class ContentModuleVisibilityInspectionTestBase : JavaCodeInsightFixtureTestCase() { @@ -927,6 +928,208 @@ class ContentModuleVisibilityInspectionTest : ContentModuleVisibilityInspectionT testHighlighting(testedFile) } + fun `test should report module dependency that is from different namespace in sources, but from the same namespace in a library`() { + // this module is duplicated by the library added with PsiTestUtil.addLibrary below + // (namespace in the library is test-namespace, as in com.example.plugin, so no issue would be reported, if the lib descriptor was taken) + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin.with.internalmodule", + "com.example.plugin.with.internalmodule/META-INF/plugin.xml", + """ + + com.example.plugin.with.internalmodule + + + + + """.trimIndent()) + + myFixture.addModuleWithPluginDescriptor( + "com.example.internalmodule", + "com.example.internalmodule/com.example.internalmodule.xml", + """ + + + """.trimIndent()) + + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin", + "com.example.plugin/META-INF/plugin.xml", + """ + + com.example.plugin + + + + + """.trimIndent()) + + val currentModuleName = "com.example.currentmodule" + val currentModule = PsiTestUtil.addModule( + project, PluginModuleType.getInstance(), currentModuleName, myFixture.tempDirFixture.findOrCreateDir(currentModuleName) + ) + PsiTestUtil.addLibrary( + currentModule, "${DevkitJavaTestsUtil.TESTDATA_ABSOLUTE_PATH}contentModules/com.example.plugin.with.internal.module.jar" + ) + + val testedFile = myFixture.addXmlFile( + "com.example.currentmodule/com.example.currentmodule.xml", + """ + + + com.example.internalmodule"/> + + + """.trimIndent() + ) + testHighlighting(testedFile) + } + + fun `test should report module dependency that is from different namespace and provider in a library`() { + myFixture.addModuleWithPluginDescriptor( + "com.example.internalmodule", + "com.example.internalmodule/com.example.internalmodule.xml", + """ + + + """.trimIndent()) + + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin", + "com.example.plugin/META-INF/plugin.xml", + """ + + com.example.plugin + + + + + """.trimIndent()) + + val currentModuleName = "com.example.currentmodule" + val currentModule = PsiTestUtil.addModule( + project, PluginModuleType.getInstance(), currentModuleName, myFixture.tempDirFixture.findOrCreateDir(currentModuleName) + ) + PsiTestUtil.addLibrary( + currentModule, + "${DevkitJavaTestsUtil.TESTDATA_ABSOLUTE_PATH}contentModules/com.example.plugin.with.internal.module-another-namespace.jar" + ) + + val testedFile = myFixture.addXmlFile( + "com.example.currentmodule/com.example.currentmodule.xml", + """ + + + com.example.internalmodule"/> + + + """.trimIndent() + ) + testHighlighting(testedFile) + } + + fun `test should report module dependency that is private in source, but public in library`() { + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin.with.public.or.private.module", + "com.example.plugin.with.public.or.private.module/META-INF/plugin.xml", + """ + + com.example.plugin.with.public.or.private.module + + + + + """.trimIndent()) + + // this module is duplicated by the library added with PsiTestUtil.addLibrary below + myFixture.addModuleWithPluginDescriptor( + "com.example.public.or.private.module", + "com.example.public.or.private.module/com.example.public.or.private.module.xml", + """ + + + """.trimIndent()) + + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin.with.currentmodule", + "com.example.plugin.with.currentmodule/META-INF/plugin.xml", + """ + + com.example.plugin.with.currentmodule + + + + + """.trimIndent()) + + val currentModuleName = "com.example.currentmodule" + val currentModule = PsiTestUtil.addModule( + project, PluginModuleType.getInstance(), currentModuleName, myFixture.tempDirFixture.findOrCreateDir(currentModuleName) + ) + PsiTestUtil.addLibrary(currentModule, + "${DevkitJavaTestsUtil.TESTDATA_ABSOLUTE_PATH}contentModules/com.example.public.or.private.module.jar") + + val testedFile = myFixture.addXmlFile( + "com.example.currentmodule/com.example.currentmodule.xml", + """ + + + com.example.public.or.private.module"/> + + + """.trimIndent() + ) + testHighlighting(testedFile) + } + + fun `test should report module dependency that is private and provided from a library`() { + val pluginWithPrivateModuleName = "com.example.plugin.with.private.module" + val pluginWithPrivateModuleModule = PsiTestUtil.addModule( + myFixture.project, PluginModuleType.getInstance(), pluginWithPrivateModuleName, + myFixture.tempDirFixture.findOrCreateDir(pluginWithPrivateModuleName) + ) + myFixture.addXmlFile("com.example.plugin.with.private.module/META-INF/plugin.xml", """ + + com.example.plugin.with.private.module + + + + + """.trimIndent()) + + val privateModuleJarPath = "${DevkitJavaTestsUtil.TESTDATA_ABSOLUTE_PATH}contentModules/com.example.private.module.jar" + PsiTestUtil.addLibrary(pluginWithPrivateModuleModule, privateModuleJarPath) + + myFixture.addModuleWithPluginDescriptor( + "com.example.plugin.with.currentmodule", + "com.example.plugin.with.currentmodule/META-INF/plugin.xml", + """ + + com.example.plugin.with.currentmodule + + + + + """.trimIndent()) + + val currentModuleName = "com.example.currentmodule" + val currentModule = PsiTestUtil.addModule( + project, PluginModuleType.getInstance(), currentModuleName, myFixture.tempDirFixture.findOrCreateDir(currentModuleName) + ) + PsiTestUtil.addLibrary(currentModule, privateModuleJarPath) + + val testedFile = myFixture.addXmlFile( + "com.example.currentmodule/com.example.currentmodule.xml", + """ + + + com.example.private.module"/> + + + """.trimIndent() + ) + testHighlighting(testedFile) + } + private fun testHighlighting(testedFile: PsiFile) { myFixture.testHighlighting(true, true, true, testedFile.virtualFile) }