[build] MRI-4494 preserve user XML comments inside <dependencies>

`updateXmlDependencies` dropped user-authored comments inside <dependencies>
when regenerating the block: the XML parser tracked only <plugin> and <module>
elements. This lost `<!-- IJ/AS-DEPENDENCY-PLACEHOLDER -->` in 1d4dfa49b7cb.

Capture comments as a new `DepEntry.Comment` variant, skip generator-owned
region/fold markers, and re-emit them in their original positions. Adds
regression tests for placeholder loss and fold-marker duplication.

GitOrigin-RevId: 59a6ebd4f92bb38aa00658dab577e3932f17ea9e
This commit is contained in:
Mikhail Filippov
2026-06-04 16:31:42 +00:00
committed by intellij-monorepo-bot
parent 755177d264
commit 9c683a042e
2 changed files with 99 additions and 0 deletions
@@ -21,6 +21,18 @@ private const val REGION_MARKER = "region Generated dependencies"
// Legacy marker for backward compatibility with existing files
private const val LEGACY_MARKER = "editor-fold desc=\"Generated dependencies"
/**
* Returns true when the comment is one of the fold/region markers owned by the generator
* (region start/end or legacy editor-fold start/end). Such comments must not be treated as
* user-authored content to preserve when rewriting the `<dependencies>` section.
*/
private fun isFoldMarkerComment(commentText: String): Boolean {
return commentText.contains(REGION_MARKER) ||
commentText.contains(LEGACY_MARKER) ||
commentText.contains("endregion") ||
commentText.contains("end editor-fold")
}
private enum class RegionType { NONE, WRAPS_ENTIRE_SECTION, INSIDE_SECTION }
private data class RemovalRange(
@@ -31,6 +43,7 @@ private data class RemovalRange(
private sealed class DepEntry {
data class Plugin(val id: String) : DepEntry()
data class Module(val name: String) : DepEntry()
data class Comment(val text: String) : DepEntry()
}
private data class DepOccurrence(
@@ -180,6 +193,7 @@ internal fun buildUpdatedXmlDependenciesContent(
when (val entry = occurrence.entry) {
is DepEntry.Plugin -> preserveExistingPlugin?.invoke(entry.id) == true
is DepEntry.Module -> preserveExistingModule?.invoke(entry.name) == true
is DepEntry.Comment -> true
}
}
@@ -335,6 +349,20 @@ private fun parseDependenciesInfo(content: String, allowInsideSectionRegion: Boo
}
}
}
XMLStreamConstants.COMMENT -> if (depth > 0) {
val locOffset = reader.location.characterOffset
val startOffset = content.lastIndexOf("<!--", locOffset)
if (startOffset >= 0) {
val endIdx = content.indexOf("-->", startOffset)
if (endIdx >= 0) {
val fullText = content.substring(startOffset, endIdx + "-->".length)
if (!isFoldMarkerComment(fullText)) {
entries.add(DepEntry.Comment(fullText))
entryOffsets.add(startOffset)
}
}
}
}
XMLStreamConstants.END_ELEMENT -> if (depth > 0) {
depth--
if (depth == 0) {
@@ -428,6 +456,7 @@ private fun dedupeOccurrencesKeepingFirst(
is DepEntry.Plugin -> {
if (entry.id in generatedPluginIds || !seenPlugins.add(entry.id)) continue
}
is DepEntry.Comment -> Unit
}
result.add(entry)
}
@@ -447,6 +476,7 @@ private fun collectDuplicateRemovalRanges(
val shouldRemove = when (val entry = occurrence.entry) {
is DepEntry.Module -> entry.name in generatedModuleNames || !seenModules.add(entry.name)
is DepEntry.Plugin -> entry.id in generatedPluginIds || !seenPlugins.add(entry.id)
is DepEntry.Comment -> false
}
if (shouldRemove) {
createEntryRemovalRange(content, occurrence)?.let(ranges::add)
@@ -459,6 +489,7 @@ private fun createEntryRemovalRange(content: String, occurrence: DepOccurrence):
val tagName = when (occurrence.entry) {
is DepEntry.Module -> "module"
is DepEntry.Plugin -> "plugin"
is DepEntry.Comment -> return null
}
val startOffset = occurrence.startOffset
if (startOffset < 0) return null
@@ -621,6 +652,7 @@ private fun buildWithEntries(indent: String, manualEntries: List<DepEntry>, auto
when (entry) {
is DepEntry.Plugin -> append(indent).append(" <plugin id=\"").append(entry.id).append("\"/>\n")
is DepEntry.Module -> append(indent).append(" <module name=\"").append(entry.name).append("\"/>\n")
is DepEntry.Comment -> append(indent).append(" ").append(entry.text).append("\n")
}
}
if (autoPlugins.isNotEmpty() || autoModules.isNotEmpty()) {
@@ -237,6 +237,73 @@ class XmlDependencyUpdaterTest {
assertThat(xml).contains("<plugin id=\"new.plugin\"/>")
}
@Test
fun `preserves placeholder comment when transitioning from no region to inside region`(@TempDir tempDir: Path) {
// Repro of IJI-2993-style regression: a plugin.xml with hand-authored placeholder comments
// and an explicit <dependencies> section but no generated region was rewritten by the
// generator, which dropped every comment along with the rebuilt block.
val content = """
<idea-plugin>
<id>org.example</id>
<name>Example</name>
<dependencies>
<!-- IJ/AS-DEPENDENCY-PLACEHOLDER -->
<plugin id="com.intellij.java"/>
<module name="intellij.platform.collaborationTools"/>
</dependencies>
</idea-plugin>
""".trimIndent()
val updater = DeferredFileUpdater(tempDir)
val path = tempDir.resolve("META-INF/plugin.xml")
updateXmlDependencies(
path = path,
content = content,
moduleDependencies = listOf("intellij.java.backend"),
pluginDependencies = emptyList(),
preserveExistingPlugin = { it == "com.intellij.java" },
preserveExistingModule = { it == "intellij.platform.collaborationTools" },
strategy = updater,
)
val xml = updater.getDiffs().single().expectedContent
assertThat(xml).contains("<!-- IJ/AS-DEPENDENCY-PLACEHOLDER -->")
assertThat(xml).contains("<plugin id=\"com.intellij.java\"/>")
assertThat(xml).contains("<module name=\"intellij.platform.collaborationTools\"/>")
assertThat(xml).contains("<module name=\"intellij.java.backend\"/>")
assertThat(xml.indexOf("<!-- IJ/AS-DEPENDENCY-PLACEHOLDER -->"))
.isLessThan(xml.indexOf("<!-- region Generated dependencies"))
}
@Test
fun `does not duplicate or re-emit fold region markers as comments`(@TempDir tempDir: Path) {
val content = """
<idea-plugin>
<id>org.example</id>
<name>Example</name>
<dependencies>
<!-- region Generated dependencies - run `Generate Product Layouts` to regenerate -->
<module name="intellij.platform.debugger.impl.ui"/>
<!-- endregion -->
</dependencies>
</idea-plugin>
""".trimIndent()
val updater = DeferredFileUpdater(tempDir)
val path = tempDir.resolve("META-INF/plugin.xml")
updateXmlDependencies(
path = path,
content = content,
moduleDependencies = listOf("intellij.platform.debugger.impl.ui", "intellij.platform.collaborationTools"),
pluginDependencies = emptyList(),
strategy = updater,
)
val xml = updater.getDiffs().single().expectedContent
assertThat(xml).containsOnlyOnce("<!-- region Generated dependencies")
assertThat(xml).containsOnlyOnce("<!-- endregion -->")
}
@Test
fun `removes duplicate legacy depends for modern plugin deps`() {
val content = """