From 46cef88f3718d0d46878cba00dc06cce9255d4fb Mon Sep 17 00:00:00 2001 From: Nikolay Chashnikov Date: Wed, 11 Feb 2026 09:36:08 +0100 Subject: [PATCH] IJPL-234358 refactor: introduce RuntimeModuleId.getPresentableName method and use it instead of getStringId in messages GitOrigin-RevId: 9ab5a2b058b30cd78ab9d092a26cf97538a1c6b4 --- .../ModuleBasedPluginXmlPathResolver.kt | 2 +- .../ModuleBasedProductLoadingStrategy.kt | 8 ++--- .../RuntimeModuleRepositoryChecker.kt | 34 +++++++++---------- .../src/impl/IncludedRuntimeModuleImpl.java | 2 +- .../src/impl/PluginModuleGroupImpl.java | 2 +- .../product/src/impl/ServiceModuleMapping.kt | 8 ++--- .../ProductModulesSerialization.java | 2 +- .../src/serialization/impl/PluginXmlReader.kt | 6 ++-- .../repository/api-dump-experimental.txt | 1 + .../runtime/repository/RuntimeModuleId.java | 9 ++++- .../impl/RuntimeModuleRepositoryImpl.java | 8 ++--- 11 files changed, 45 insertions(+), 37 deletions(-) diff --git a/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedPluginXmlPathResolver.kt b/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedPluginXmlPathResolver.kt index ef3f2e0e6bf5..d4f2fbfc99e9 100644 --- a/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedPluginXmlPathResolver.kt +++ b/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedPluginXmlPathResolver.kt @@ -88,7 +88,7 @@ internal class ModuleBasedPluginXmlPathResolver( val inputStream = module.moduleDescriptor.readFile(path) if (inputStream != null) { val bytes = inputStream.use { it.readBytes() } - return LoadedXIncludeReference(bytes, module.moduleDescriptor.moduleId.stringId) + return LoadedXIncludeReference(bytes, module.moduleDescriptor.moduleId.presentableName) } } return null diff --git a/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedProductLoadingStrategy.kt b/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedProductLoadingStrategy.kt index ca4aee339b15..6d61e3b56a97 100644 --- a/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedProductLoadingStrategy.kt +++ b/platform/bootstrap/src/com/intellij/platform/bootstrap/ModuleBasedProductLoadingStrategy.kt @@ -161,8 +161,8 @@ internal class ModuleBasedProductLoadingStrategy(internal val moduleRepository: productModules.notLoadedBundledPluginModules.forEach { (notLoadedId, failedDependencyPath) -> // todo: convert this to an error after fixing the problem with intellij.performanceTesting.async plugin: IJPL-186414 logger().warn(""" - |Bundled plugin module '${notLoadedId.stringId}' couldn't be loaded because of missing dependency: - |${failedDependencyPath.joinToString(" -> ") { it.stringId }} + |Bundled plugin module '${notLoadedId.presentableName}' couldn't be loaded because of missing dependency: + |${failedDependencyPath.joinToString(" -> ") { it.presentableName }} |""".trimMargin()) } @@ -230,7 +230,7 @@ internal class ModuleBasedProductLoadingStrategy(internal val moduleRepository: return dataLoader.load(path = PluginManagerCore.PLUGIN_XML_PATH, pluginDescriptorSourceOnly = true) } catch (e: Throwable) { - logger().warn("Failed to load ${PluginManagerCore.PLUGIN_XML_PATH} from '${pluginModuleGroup.mainModule.moduleId.stringId}' module: $e", e) + logger().warn("Failed to load ${PluginManagerCore.PLUGIN_XML_PATH} from '${pluginModuleGroup.mainModule.moduleId.presentableName}' module: $e", e) return null } finally { @@ -337,7 +337,7 @@ internal class ModuleBasedProductLoadingStrategy(internal val moduleRepository: val resourceRoots = pluginModuleGroup.mainModule.resourceRootPaths if (resourceRoots.isEmpty()) { thisLogger().warn( - "Main plugin module must have at least one resource root, so '${pluginModuleGroup.mainModule.moduleId.stringId}' won't be loaded" + "Main plugin module must have at least one resource root, so '${pluginModuleGroup.mainModule.moduleId.presentableName}' won't be loaded" ) return null } diff --git a/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/RuntimeModuleRepositoryChecker.kt b/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/RuntimeModuleRepositoryChecker.kt index ebd3733019e4..0c538725d6ea 100644 --- a/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/RuntimeModuleRepositoryChecker.kt +++ b/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/RuntimeModuleRepositoryChecker.kt @@ -143,10 +143,10 @@ internal class RuntimeModuleRepositoryChecker private constructor( val mainModules = mainGroupModuleResourceRoots[resourcePath] if (mainModules != null) { val mainModuleListString = - if (mainModules.size < 3) mainModules.joinToString { it.stringId } - else "${mainModules.first().stringId} and ${mainModules.size - 1} more modules" - val moduleId = pluginModule.moduleId.stringId - val pluginModuleId = group.mainModule.moduleId.stringId + if (mainModules.size < 3) mainModules.joinToString { it.presentableName } + else "${mainModules.first().presentableName} and ${mainModules.size - 1} more modules" + val moduleId = pluginModule.moduleId.presentableName + val pluginModuleId = group.mainModule.moduleId.presentableName softly.collectAssertionErrorIfNotRegisteredYet( AssertionError(""" |Module '$moduleId' from plugin '$pluginModuleId' has resource root ${commonDistPath.relativize(resourcePath)}, @@ -184,9 +184,9 @@ internal class RuntimeModuleRepositoryChecker private constructor( """.trimMargin())) return@forEach } - val pluginPath = FList.singleton("bundled plugin ${group.mainModule.moduleId.stringId}") + val pluginPath = FList.singleton("bundled plugin ${group.mainModule.moduleId.presentableName}") group.includedModules.forEach { - repository.collectDependencies(it.moduleDescriptor, pluginPath.prepend(it.moduleDescriptor.moduleId.stringId), allProductModules) + repository.collectDependencies(it.moduleDescriptor, pluginPath.prepend(it.moduleDescriptor.moduleId.presentableName), allProductModules) } } @@ -216,21 +216,21 @@ internal class RuntimeModuleRepositoryChecker private constructor( val includedModules = productResourceRoots.getValue(included) val displayedModulesCount = 10 val firstIncludedModuleData = includedModules.take(displayedModulesCount).joinToString(separator = System.lineSeparator()) { - "'${it.stringId}' (<- ${allProductModules.getValue(it).joinToString(" <- ")})" + "'${it.presentableName}' (<- ${allProductModules.getValue(it).joinToString(" <- ")})" } val rest = includedModules.size - displayedModulesCount val embeddedProductPresentableName = "${context.applicationInfo.shortProductName} Frontend" val more = if (rest > 0) " and $rest more ${StringUtil.pluralize("module", rest)}" else "" softly.collectAssertionErrorIfNotRegisteredYet(AssertionError(""" - |Module '${moduleId.stringId}' is not part of $embeddedProductPresentableName included in the full ${context.applicationInfo.shortProductName} distribution, but it's packed in ${included.pathString}, + |Module '${moduleId.presentableName}' is not part of $embeddedProductPresentableName included in the full ${context.applicationInfo.shortProductName} distribution, but it's packed in ${included.pathString}, |which is included in the classpath of $embeddedProductPresentableName because: |$firstIncludedModuleData$more are also packed in it. - |This means that '${moduleId.stringId}' will be included in the classpath of $embeddedProductPresentableName as well. + |This means that '${moduleId.presentableName}' will be included in the classpath of $embeddedProductPresentableName as well. |Unnecessary code and resources in the classpath may cause performance problems, also, they may cause $embeddedProductPresentableName to behave differently in a standalone |installation and when invoked from ${context.applicationInfo.fullProductName}. To fix the problem, you should do one of the following: |* if other modules packed in '${included.pathString}' shouldn't be part of $embeddedProductPresentableName, remove incorrect dependencies shown above; this may require extracting additional modules; - |* if '${moduleId.stringId}' actually should be included in $embeddedProductPresentableName, make sure that it's included either by adding it as a content module in plugin.xml, or by adding it in the main module group in product-modules.xml; - |* if '${moduleId.stringId}' should not be included in $embeddedProductPresentableName, but other parts of ${included.pathString} should, ensure that they are put to + |* if '${moduleId.presentableName}' actually should be included in $embeddedProductPresentableName, make sure that it's included either by adding it as a content module in plugin.xml, or by adding it in the main module group in product-modules.xml; + |* if '${moduleId.presentableName}' should not be included in $embeddedProductPresentableName, but other parts of ${included.pathString} should, ensure that they are put to | separate JAR files; it may be enough to add a runtime dependency on 'intellij.platform.backend' to all modules which shouldn't be included to the frontend part, | the build scripts will take this into account to assign separate JARs automatically; however, if custom layout is specified for a plugin, you may need to put modules | to separate JARs using explicit 'withModule(...)' calls in the layout configuration. @@ -246,27 +246,27 @@ internal class RuntimeModuleRepositoryChecker private constructor( for (mainModuleId in rawProductModules.bundledPluginMainModules) { val mainModule = repository.resolveModule(mainModuleId) if (mainModule.resolvedModule == null) { - val problematicModule = if (mainModule.failedDependencyPath.size == 1) "it" else "its dependency ${mainModule.failedDependencyPath.reversed().joinToString(" <- ") { it.stringId }}" + val problematicModule = if (mainModule.failedDependencyPath.size == 1) "it" else "its dependency ${mainModule.failedDependencyPath.reversed().joinToString(" <- ") { it.presentableName }}" softly.collectAssertionErrorIfNotRegisteredYet( AssertionError( buildString { - append("Module '${mainModuleId.stringId}' is specified as the main module of a bundled plugin in product-modules.xml in '$productModulesModule',\n") + append("Module '${mainModuleId.presentableName}' is specified as the main module of a bundled plugin in product-modules.xml in '$productModulesModule',\n") append("but $problematicModule cannot be found in the runtime module repository in the distribution of $currentDistributionName.\n") if (isEmbeddedVariant) { append("It means that the corresponding plugin won't be loaded when '$productName Frontend' is started from the full\n") append("installation of $productName\n") } - append("If '${mainModuleId.stringId}' shouldn't be available in the frontend variant of $productName, remove it from product-modules.xml file\n") + append("If '${mainModuleId.presentableName}' shouldn't be available in the frontend variant of $productName, remove it from product-modules.xml file\n") append("(or use 'without-module' tag if it comes via 'include' tag).\n") if (isEmbeddedVariant) { - append("If it should, add all necessary modules to the plugin layout of the main variant of '${mainModuleId.stringId}' plugin.\n") + append("If it should, add all necessary modules to the plugin layout of the main variant of '${mainModuleId.presentableName}' plugin.\n") append("Modules used by the frontend variant only should be put in JAR files in 'frontend-split' subdirectory so they won't be loaded in the regular IDE.\n") } else { append("If it should, make sure that all necessary modules are included in the distribution of $currentDistributionName.\n") } if (mainModule.failedDependencyPath.size > 1) { - append("If some dependencies in the chain ${mainModule.failedDependencyPath.joinToString(" <- ") { it.stringId }}\n") + append("If some dependencies in the chain ${mainModule.failedDependencyPath.joinToString(" <- ") { it.presentableName }}\n") append("are not actually needed, they can be removed from configuration of the corresponding JPS modules (*.iml) to fix this problem.\n") } append("Please refer to https://youtrack.jetbrains.com/articles/IJPL-A-268 to learn more how the frontend process starts.") @@ -333,7 +333,7 @@ private fun RuntimeModuleRepository.collectDependencies( result: MutableMap> = LinkedHashMap(), ): MutableMap> { if (result.putIfAbsent(moduleDescriptor.moduleId, path) == null) { - val newPath = path.prepend(moduleDescriptor.moduleId.stringId) + val newPath = path.prepend(moduleDescriptor.moduleId.presentableName) for (dependency in moduleDescriptor.dependencies) { collectDependencies(dependency, newPath, result) } diff --git a/platform/runtime/product/src/impl/IncludedRuntimeModuleImpl.java b/platform/runtime/product/src/impl/IncludedRuntimeModuleImpl.java index 28f67d384f62..39dbd2dfaff0 100644 --- a/platform/runtime/product/src/impl/IncludedRuntimeModuleImpl.java +++ b/platform/runtime/product/src/impl/IncludedRuntimeModuleImpl.java @@ -27,6 +27,6 @@ public final class IncludedRuntimeModuleImpl implements IncludedRuntimeModule { @Override public String toString() { - return "IncludedRuntimeModule{moduleId=" + myModuleDescriptor.getModuleId().getStringId() + '}'; + return "IncludedRuntimeModule{moduleId=" + myModuleDescriptor.getModuleId().getPresentableName() + '}'; } } diff --git a/platform/runtime/product/src/impl/PluginModuleGroupImpl.java b/platform/runtime/product/src/impl/PluginModuleGroupImpl.java index eb05e62a930b..1b803399caab 100644 --- a/platform/runtime/product/src/impl/PluginModuleGroupImpl.java +++ b/platform/runtime/product/src/impl/PluginModuleGroupImpl.java @@ -90,6 +90,6 @@ public final class PluginModuleGroupImpl implements PluginModuleGroup { @Override public String toString() { - return "PluginModuleGroup{mainModule=" + myMainModule.getModuleId().getStringId() + "}"; + return "PluginModuleGroup{mainModule=" + myMainModule.getModuleId().getPresentableName() + "}"; } } diff --git a/platform/runtime/product/src/impl/ServiceModuleMapping.kt b/platform/runtime/product/src/impl/ServiceModuleMapping.kt index 90d32dab0ff6..877a046fd462 100644 --- a/platform/runtime/product/src/impl/ServiceModuleMapping.kt +++ b/platform/runtime/product/src/impl/ServiceModuleMapping.kt @@ -49,10 +49,10 @@ interface ServiceModuleMapping { val currentPath = showPath(dependency, dependencyPath) val previousPath = showPath(dependency, dependencyPathToModule?.get(dependency)) errors.add(""" - |Modules from two plugins depend on module '${dependency.moduleId.stringId}': - | '${previousGroup.mainModule.moduleId.stringId}'$previousPath and '${pluginGroup.mainModule.moduleId.stringId}'$currentPath + |Modules from two plugins depend on module '${dependency.moduleId.presentableName}': + | '${previousGroup.mainModule.moduleId.presentableName}'$previousPath and '${pluginGroup.mainModule.moduleId.presentableName}'$currentPath |Currently every module should belong to some plugin, but the system cannot automatically determine which plugin should be used. - |To fix the problem, register '${dependency.moduleId.stringId}' in some plugin explicitly or include it in the main module + |To fix the problem, register '${dependency.moduleId.presentableName}' in some plugin explicitly or include it in the main module |in product-modules.xml. """.trimMargin()) } @@ -74,7 +74,7 @@ interface ServiceModuleMapping { } private fun showPath(dependency: RuntimeModuleDescriptor, path: FList?): String = - if (path != null) " (${path.prepend(dependency).joinToString(" <- ") { it.moduleId.stringId }})" else "" + if (path != null) " (${path.prepend(dependency).joinToString(" <- ") { it.moduleId.presentableName }})" else "" } } diff --git a/platform/runtime/product/src/serialization/ProductModulesSerialization.java b/platform/runtime/product/src/serialization/ProductModulesSerialization.java index 22853201e26c..8c06356a3115 100644 --- a/platform/runtime/product/src/serialization/ProductModulesSerialization.java +++ b/platform/runtime/product/src/serialization/ProductModulesSerialization.java @@ -107,7 +107,7 @@ public final class ProductModulesSerialization { RuntimeModuleId includedId = includedFromData.getFromModule(); InputStream inputStream = resolver.readResourceFile(includedId, "META-INF/" + includedId.getStringId() + "/product-modules.xml"); if (inputStream == null) { - throw new MalformedRepositoryException("'" + includedId.getStringId() + "' included in " + + throw new MalformedRepositoryException("'" + includedId.getPresentableName() + "' included in " + debugName + " doesn't contain product-modules.xml"); } RawProductModules includedModules = ProductModulesXmlSerializer.parseModuleXml(inputStream); diff --git a/platform/runtime/product/src/serialization/impl/PluginXmlReader.kt b/platform/runtime/product/src/serialization/impl/PluginXmlReader.kt index 18edcacb9676..503d35fe1d6b 100644 --- a/platform/runtime/product/src/serialization/impl/PluginXmlReader.kt +++ b/platform/runtime/product/src/serialization/impl/PluginXmlReader.kt @@ -27,7 +27,7 @@ fun loadPluginModules( addedModules.add(mainModule.moduleId.stringId) resourceFileResolver.readResourceFile(mainModule.moduleId, PLUGIN_XML_PATH).use { inputStream -> if (inputStream == null) { - throw MalformedRepositoryException("$PLUGIN_XML_PATH is not found in '${mainModule.moduleId.stringId}' module in $repository " + + throw MalformedRepositoryException("$PLUGIN_XML_PATH is not found in '${mainModule.moduleId.presentableName}' module in $repository " + "using $resourceFileResolver; resources roots: ${mainModule.resourceRootPaths}") } val reader = XMLInputFactory.newDefaultFactory().createXMLStreamReader(inputStream) @@ -76,9 +76,9 @@ fun loadPluginModules( return modules } catch (e: IOException) { - throw MalformedRepositoryException("Failed to load included modules for ${mainModule.moduleId.stringId}", e) + throw MalformedRepositoryException("Failed to load included modules for ${mainModule.moduleId.presentableName}", e) } catch (e: XMLStreamException) { - throw MalformedRepositoryException("Failed to load included modules for ${mainModule.moduleId.stringId}", e) + throw MalformedRepositoryException("Failed to load included modules for ${mainModule.moduleId.presentableName}", e) } } diff --git a/platform/runtime/repository/api-dump-experimental.txt b/platform/runtime/repository/api-dump-experimental.txt index a3e8d4d3c7e3..b7d48f8a3aa1 100644 --- a/platform/runtime/repository/api-dump-experimental.txt +++ b/platform/runtime/repository/api-dump-experimental.txt @@ -8,6 +8,7 @@ - a:readFile(java.lang.String):java.io.InputStream *f:com.intellij.platform.runtime.repository.RuntimeModuleId - equals(java.lang.Object):Z +- getPresentableName():java.lang.String - getStringId():java.lang.String - hashCode():I - s:module(java.lang.String):com.intellij.platform.runtime.repository.RuntimeModuleId diff --git a/platform/runtime/repository/src/com/intellij/platform/runtime/repository/RuntimeModuleId.java b/platform/runtime/repository/src/com/intellij/platform/runtime/repository/RuntimeModuleId.java index ae05f1d8cccf..076a9590e70c 100644 --- a/platform/runtime/repository/src/com/intellij/platform/runtime/repository/RuntimeModuleId.java +++ b/platform/runtime/repository/src/com/intellij/platform/runtime/repository/RuntimeModuleId.java @@ -27,7 +27,14 @@ public final class RuntimeModuleId { } /** - * Creates ID from a raw string representation as it's written in the runtime module repository. + * Returns a human-readable name of the module. It can be used for debugging and logging purposes only. + */ + public @NotNull String getPresentableName() { + return myStringId; + } + + /** + * Creates ID from a raw string representation as it's written in the runtime module repository. * This method is supposed to be used to generate and transform the module repository only, other code should use other methods. */ @ApiStatus.Internal diff --git a/platform/runtime/repository/src/com/intellij/platform/runtime/repository/impl/RuntimeModuleRepositoryImpl.java b/platform/runtime/repository/src/com/intellij/platform/runtime/repository/impl/RuntimeModuleRepositoryImpl.java index 025adebfccd7..b6e3413b02e8 100644 --- a/platform/runtime/repository/src/com/intellij/platform/runtime/repository/impl/RuntimeModuleRepositoryImpl.java +++ b/platform/runtime/repository/src/com/intellij/platform/runtime/repository/impl/RuntimeModuleRepositoryImpl.java @@ -110,13 +110,13 @@ public class RuntimeModuleRepositoryImpl implements RuntimeModuleRepository { List failedDependencyPath = result.getFailedDependencyPath(); String message; if (failedDependencyPath.size() == 1) { - message = "Cannot find module '" + failedDependencyPath.get(0).getStringId() + "'"; + message = "Cannot find module '" + failedDependencyPath.get(0).getPresentableName() + "'"; } else { List reversed = new ArrayList<>(failedDependencyPath.subList(0, failedDependencyPath.size() - 1)); Collections.reverse(reversed); - message = "Cannot resolve module '" + moduleId.getStringId() + "': module '" + failedDependencyPath.get(failedDependencyPath.size() - 1).getStringId() + "' (" + - reversed.stream().map(id -> " <- '" + id.getStringId() + "'").collect(Collectors.joining()).trim() + ") is not found"; + message = "Cannot resolve module '" + moduleId.getPresentableName() + "': module '" + failedDependencyPath.get(failedDependencyPath.size() - 1).getStringId() + "' (" + + reversed.stream().map(id -> " <- '" + id.getPresentableName() + "'").collect(Collectors.joining()).trim() + ") is not found"; } throw new MalformedRepositoryException(message); } @@ -136,7 +136,7 @@ public class RuntimeModuleRepositoryImpl implements RuntimeModuleRepository { } } if (rawDescriptor == null) { - throw new MalformedRepositoryException("Cannot find module '" + moduleId.getStringId() + "'"); + throw new MalformedRepositoryException("Cannot find module '" + moduleId.getPresentableName() + "'"); } } //todo improve this to reuse the computed paths if the module is resolved later