IJPL-233553 IJ-MR-181153 remove suppression for "intellij.xslt.debugger", fix legacy <depends> handling: remove duplicates and inject modern <dependencies> section where applicable

GitOrigin-RevId: f3bd7586d006f1baa3dc1a03cb447b6002b19591
2026-09-27 10:03:11 +07:00 · 2026-02-05 08:40:54 +00:00
parent dc619c4cf7
commit b4b63b6a42
4 changed files with 240 additions and 13 deletions
@@ -30,6 +30,8 @@ import org.jetbrains.intellij.build.productLayout.pipeline.Slots
 import org.jetbrains.intellij.build.productLayout.stats.SuppressionType
 import org.jetbrains.intellij.build.productLayout.stats.SuppressionUsage
 import org.jetbrains.intellij.build.productLayout.xml.LegacyMigrationResult
+import org.jetbrains.intellij.build.productLayout.xml.extractDependenciesEntries
+import org.jetbrains.intellij.build.productLayout.xml.removeDuplicateLegacyDepends

 /**
 * Planner for plugin.xml dependency XML files.
@@ -215,12 +217,29 @@ private suspend fun buildPluginDependencyPlan(

  val deps = filterPluginDependencies(graphDeps, info, effectiveFilter, suppressionConfig, updateSuppressions)

-  // Legacy <depends> entries are NOT migrated - they stay as-is
-  // Generator only manages <dependencies> section, doesn't touch legacy format
-  val legacyMigration = LegacyMigrationResult(content = info.pluginXmlContent, pluginDepsToAdd = emptyList())
+  // Remove duplicate legacy <depends> only when modern deps are present or we are generating a <dependencies> section.
+  val hasDependenciesSection = extractDependenciesEntries(info.pluginXmlContent) != null
+  val hasModernDepsInXIncludes = info.depsByFile.drop(1).any { it.pluginDependencies.isNotEmpty() || it.moduleDependencies.isNotEmpty() }
+  val legacyPluginIds = info.legacyDepends.map { it.pluginId.value }.sorted()
+  val autoPluginIds = deps.pluginDependencies.map { it.value }.sorted()
+  val shouldRemoveLegacyDuplicates = hasDependenciesSection || hasModernDepsInXIncludes || deps.moduleDependencies.isNotEmpty() || autoPluginIds != legacyPluginIds

-  // Merge legacy plugin deps with auto-generated plugin deps (convert to String for XML processing)
-  val allPluginDepsStrings = (deps.pluginDependencies.map { it.value } + legacyMigration.pluginDepsToAdd).distinct().sorted()
+  val modernPluginIds = HashSet<PluginId>().apply {
+    addAll(deps.pluginDependencies)
+    for (fileDeps in info.depsByFile) {
+      addAll(fileDeps.pluginDependencies)
+    }
+  }
+
+  val legacyMigration = if (shouldRemoveLegacyDuplicates) {
+    removeDuplicateLegacyDepends(info.pluginXmlContent, modernPluginIds)
+  }
+  else {
+    LegacyMigrationResult(content = info.pluginXmlContent)
+  }
+
+  // Merge auto-generated plugin deps (convert to String for XML processing)
+  val allPluginDepsStrings = deps.pluginDependencies.map { it.value }.distinct().sorted()

  // Compute xi:include deps from depsByFile (first entry = main file, rest = xi:includes)
  // These are deps already present in xi:included files, so we don't need to add them to the main file
@@ -240,6 +259,7 @@ private suspend fun buildPluginDependencyPlan(
  }

  val preserveExistingPluginDeps = existingXmlPluginDeps.filterTo(LinkedHashSet()) { it.value !in allPluginDepsStrings }
+  val effectiveLegacyDepends = info.legacyDepends.filterNot { it.pluginId in legacyMigration.removedLegacyPluginIds }

  return PluginDependencyPlan(
    pluginContentModuleName = pluginContentModuleName,
@@ -247,7 +267,7 @@ private suspend fun buildPluginDependencyPlan(
    pluginXmlContent = legacyMigration.content,
    moduleDependencies = deps.moduleDependencies.distinctBy { it.value }.sortedBy { it.value },
    pluginDependencies = deps.pluginDependencies.distinctBy { it.value }.sortedBy { it.value },
-    legacyPluginDependencies = info.legacyDepends.map { it.pluginId },
+    legacyPluginDependencies = effectiveLegacyDepends.map { it.pluginId },
    xiIncludeModuleDeps = xiIncludeModuleDeps,
    xiIncludePluginDeps = xiIncludePluginDeps,
    existingXmlModuleDependencies = existingXmlModuleDeps,
@@ -23,6 +23,11 @@ private const val LEGACY_MARKER = "editor-fold desc=\"Generated dependencies"

 private enum class RegionType { NONE, WRAPS_ENTIRE_SECTION, INSIDE_SECTION }

+private data class RemovalRange(
+  @JvmField val start: Int,
+  @JvmField val end: Int,
+)
+
 private sealed class DepEntry {
  data class Plugin(val id: String) : DepEntry()
  data class Module(val name: String) : DepEntry()
@@ -300,8 +305,82 @@ private fun insertDependenciesSection(content: String, modules: List<String>, pl

  val nextLine = content.indexOf('\n', pos + 1)
  val indent = (if (nextLine != -1) content.substring(nextLine + 1).takeWhile { it == ' ' || it == '\t' } else "").ifEmpty { "  " }
-  val suffix = content.substring(pos + 1).let { if (it.startsWith('\n')) it.substring(1) else it }
-  return content.substring(0, pos + 1) + "\n" + buildFullBlock(indent, emptyList(), modules, plugins) + suffix
+  val insertPos = findDependenciesInsertPos(content, pos + 1)
+  val prefix = content.substring(0, insertPos)
+  val suffix = content.substring(insertPos)
+  val normalizedPrefix = if (prefix.isNotEmpty() && prefix.last() != '\n') "$prefix\n" else prefix
+  return normalizedPrefix + buildFullBlock(indent, emptyList(), modules, plugins) + suffix
+}
+
+private val METADATA_TAGS = setOf(
+  "id",
+  "name",
+  "description",
+  "category",
+  "vendor",
+  "version",
+  "idea-version",
+  "change-notes",
+)
+
+private fun findDependenciesInsertPos(content: String, rootContentStart: Int): Int {
+  val metadataEnd = findMetadataEndOffset(content) ?: return rootContentStart
+  var pos = content.indexOf('\n', metadataEnd).let { if (it == -1) content.length else it + 1 }
+  while (pos < content.length) {
+    val lineEnd = content.indexOf('\n', pos).let { if (it == -1) content.length else it + 1 }
+    if (content.substring(pos, lineEnd).trim().isNotEmpty()) break
+    pos = lineEnd
+  }
+  return pos
+}
+
+private fun findMetadataEndOffset(content: String): Int? {
+  val reader = createXmlStreamReaderWithLocation(StringReader(content))
+  try {
+    var depth = 0
+    var lastEnd = -1
+    while (reader.hasNext()) {
+      when (reader.next()) {
+        XMLStreamConstants.START_ELEMENT -> {
+          val localName = reader.localName
+          if (depth == 0 && localName == "idea-plugin") {
+            depth = 1
+            continue
+          }
+          if (depth > 0) {
+            val currentDepth = depth
+            depth++
+            if (currentDepth == 1 && METADATA_TAGS.contains(localName)) {
+              val startOffset = content.lastIndexOf("<$localName", reader.location.characterOffset)
+              if (startOffset != -1) {
+                val endOffset = findElementEndOffset(content, localName, startOffset)
+                if (endOffset != null && endOffset > lastEnd) {
+                  lastEnd = endOffset
+                }
+              }
+            }
+          }
+        }
+        XMLStreamConstants.END_ELEMENT -> if (depth > 0) depth--
+      }
+    }
+    return if (lastEnd == -1) null else lastEnd
+  }
+  finally {
+    reader.close()
+  }
+}
+
+private fun findElementEndOffset(content: String, tag: String, startOffset: Int): Int? {
+  val startTagEnd = content.indexOf('>', startOffset)
+  if (startTagEnd == -1) return null
+  var i = startTagEnd - 1
+  while (i >= startOffset && content[i].isWhitespace()) i--
+  val selfClosing = i >= startOffset && content[i] == '/'
+  if (selfClosing) return startTagEnd + 1
+  val closeTag = "</$tag>"
+  val closeIndex = content.indexOf(closeTag, startTagEnd + 1)
+  return if (closeIndex == -1) null else closeIndex + closeTag.length
 }

 /** Region only with auto-generated modules/plugins (for INSIDE_SECTION - replacing fold content) */
@@ -355,8 +434,72 @@ private fun buildWithEntries(indent: String, manualEntries: List<DepEntry>, auto
 * Result of migrating legacy `<depends>` entries to `<plugin id="..."/>` format.
 */
 internal data class LegacyMigrationResult(
-  /** XML content with `<depends>` entries removed */
+  /** XML content with duplicate legacy `<depends>` entries removed */
  @JvmField val content: String,
-  /** Plugin IDs to add as `<plugin id="..."/>` dependencies */
-  @JvmField val pluginDepsToAdd: List<String>,
+  /** Legacy plugin IDs removed from the content */
+  @JvmField val removedLegacyPluginIds: Set<PluginId> = emptySet(),
 )
+
+/**
+ * Removes legacy `<depends>` entries that duplicate modern `<dependencies><plugin/>` declarations.
+ * Only non-optional entries without config-file are removed.
+ */
+internal fun removeDuplicateLegacyDepends(content: String, modernPluginIds: Set<PluginId>): LegacyMigrationResult {
+  if (modernPluginIds.isEmpty() || !content.contains("<depends")) {
+    return LegacyMigrationResult(content = content)
+  }
+
+  val idsToRemove = modernPluginIds.mapTo(HashSet()) { it.value }
+  val ranges = ArrayList<RemovalRange>()
+  val removed = LinkedHashSet<PluginId>()
+
+  val reader = createXmlStreamReaderWithLocation(StringReader(content))
+  try {
+    while (reader.hasNext()) {
+      when (reader.next()) {
+        XMLStreamConstants.START_ELEMENT -> if (reader.localName == "depends") {
+          val optionalAttr = reader.getAttributeValue(null, "optional")
+          val configFileAttr = reader.getAttributeValue(null, "config-file")
+          val isOptional = optionalAttr?.equals("true", ignoreCase = true) == true
+          val hasConfigFile = !configFileAttr.isNullOrEmpty()
+          val startOffset = content.lastIndexOf("<depends", reader.location.characterOffset)
+          val pluginId = reader.elementText.trim()
+          if (!isOptional && !hasConfigFile && startOffset != -1 && pluginId in idsToRemove) {
+            var start = startOffset
+            val lineStart = content.lastIndexOf('\n', startOffset - 1).let { if (it == -1) 0 else it + 1 }
+            if (content.substring(lineStart, startOffset).all { it == ' ' || it == '\t' }) {
+              start = lineStart
+            }
+
+            val closeIndex = content.indexOf("</depends>", startOffset)
+            if (closeIndex != -1) {
+              var end = closeIndex + "</depends>".length
+              if (end < content.length && content[end] == '\r') end++
+              if (end < content.length && content[end] == '\n') end++
+              ranges.add(RemovalRange(start, end))
+              removed.add(PluginId(pluginId))
+            }
+          }
+        }
+      }
+    }
+  }
+  finally {
+    reader.close()
+  }
+
+  if (ranges.isEmpty()) {
+    return LegacyMigrationResult(content = content)
+  }
+
+  ranges.sortBy { it.start }
+  val result = StringBuilder(content.length)
+  var cursor = 0
+  for (range in ranges) {
+    if (range.start < cursor) continue
+    result.append(content, cursor, range.start)
+    cursor = range.end
+  }
+  result.append(content, cursor, content.length)
+  return LegacyMigrationResult(content = result.toString(), removedLegacyPluginIds = removed)
+}
@@ -0,0 +1,61 @@
+// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package org.jetbrains.intellij.build.productLayout.xml
+
+import com.intellij.platform.pluginGraph.PluginId
+import org.assertj.core.api.Assertions.assertThat
+import org.jetbrains.intellij.build.productLayout.TestFailureLogger
+import org.jetbrains.intellij.build.productLayout.util.DeferredFileUpdater
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.junit.jupiter.api.io.TempDir
+import java.nio.file.Path
+
+@ExtendWith(TestFailureLogger::class)
+class XmlDependencyUpdaterTest {
+  @Test
+  fun `inserts dependencies after metadata`(@TempDir tempDir: Path) {
+    val content = """
+      <idea-plugin>
+        <name>Test Plugin</name>
+        <id>test.plugin</id>
+        <description>Test description</description>
+
+        <extensions defaultExtensionNs="com.intellij">
+        </extensions>
+      </idea-plugin>
+    """.trimIndent()
+
+    val updater = DeferredFileUpdater(tempDir)
+    val path = tempDir.resolve("META-INF/plugin.xml")
+    updateXmlDependencies(
+      path = path,
+      content = content,
+      moduleDependencies = emptyList(),
+      pluginDependencies = listOf("dep.plugin"),
+      strategy = updater,
+    )
+
+    val diffs = updater.getDiffs()
+    assertThat(diffs).hasSize(1)
+    val xml = diffs.single().expectedContent
+    assertThat(xml.indexOf("<dependencies>")).isGreaterThan(xml.indexOf("</description>"))
+  }
+
+  @Test
+  fun `removes duplicate legacy depends for modern plugin deps`() {
+    val content = """
+      <idea-plugin>
+        <name>Test</name>
+        <id>test</id>
+        <depends>XPathView</depends>
+        <depends optional="true">XPathView</depends>
+        <depends>com.intellij.modules.xml</depends>
+      </idea-plugin>
+    """.trimIndent()
+
+    val migrated = removeDuplicateLegacyDepends(content, setOf(PluginId("XPathView")))
+    assertThat(migrated.content).doesNotContain("<depends>XPathView</depends>")
+    assertThat(migrated.content).contains("<depends optional=\"true\">XPathView</depends>")
+    assertThat(migrated.content).contains("<depends>com.intellij.modules.xml</depends>")
+  }
+}
@@ -9,8 +9,11 @@

  <vendor>JetBrains</vendor>

-  <depends>XPathView</depends>
-  <depends>com.intellij.modules.xml</depends>
+  <!-- region Generated dependencies - run `Generate Product Layouts` to regenerate -->
+  <dependencies>
+    <plugin id="XPathView"/>
+  </dependencies>
+  <!-- endregion -->

  <extensions defaultExtensionNs="com.intellij">
    <xdebugger.breakpointType implementation="org.intellij.plugins.xsltDebugger.XsltBreakpointType"/>