feat: IPv6 canonicalization to RFC 5952 behind the experimental flag (#499)

* feat: IPv6 canonicalization to RFC 5952 (#363)

Offers a quick-fix to rewrite a non-canonical IPv6 address to its recommended form.
Behind the experimental flag.

- canonicalizeIpv6 (pure, dependency-free): parses to 8 groups and reformats per RFC
  5952 §4 — lowercase hex, drop leading zeros, compress the longest zero run to "::"
  (leftmost on ties, only for runs of 2+, never a single zero group). Returns null for
  non-IPv6 input and, for now, for embedded-IPv4 (§5 mixed notation) addresses.
- Combinator.labeledRegions(value): the grammar's explicit Labeled spans (e.g. a whole
  IP address) from the first fully-valid parse — lets features act on semantic spans.
- Ipv6CanonicalFormInspection: flag-gated; for grammar-backed options it scans labeled
  spans and registers a WEAK_WARNING + CanonicalizeIpv6QuickFix on any IPv6 that isn't
  already canonical. Reuses the IPV4_ADDR/IPV6_ADDR Labeled(LITERAL) spans we added for
  coloring, so no IPv6-specific engine markers were needed.

Tests: canonicalizer cases incl. zero-run/tie/single-zero/idempotence/non-IPv6; e2e
warning + quick-fix rewriting 2001:DB8::1 -> 2001:db8::1, and nothing when canonical or
the flag is off.

Closes #363. Refs #467

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs: attach colorize()'s KDoc to colorize(), not labeledRegions()

labeledRegions() was inserted between colorize()'s doc comment and its body;
reorder so each function sits under its own KDoc.

* refactor: match IPv6 spans by a grammar tag, not by re-sniffing text

The IPv6 canonicalization inspection walked every Labeled value span and
ran canonicalizeIpv6() on each, treating "the string happens to parse as
IPv6" as "this is an IPv6 address". That was only correct by luck: the
sole Labeled spans today are IPV4_ADDR and IPV6_ADDR, and IPv4 is excluded
because canonicalizeIpv6() bails on a dot. The invariant ("no other
Labeled span ever matches an 8-hextet shape") lived nowhere in the code,
and its violation wouldn't just false-positive — the quick-fix would
rewrite the span, corrupting whatever it really was.

Give Labeled an optional SemanticTag threaded onto Region, tag IPV6_ADDR
as SemanticTag.IPV6, and have the inspection act only on tagged spans. The
grammar now declares "this span is IPv6" and the inspection trusts it;
canonicalizeIpv6() is demoted from detector to pure formatter (it still
returns null for out-of-scope IPv4-tail forms). Role stays colour-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Steve Ramage <gitcommits@sjrx.net>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 3 people

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/Ipv6CanonicalFormInspection.kt

@@ -0,0 +1,62 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.inspections
+
+import com.intellij.codeInspection.LocalInspectionTool
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.util.TextRange
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.util.PsiTreeUtil
+import net.sjrx.intellij.plugins.systemdunitfiles.UnitFileLanguage
+import net.sjrx.intellij.plugins.systemdunitfiles.intentions.CanonicalizeIpv6QuickFix
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFile
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFilePropertyType
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFileSectionGroups
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFileVisitor
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.SemanticDataRepository
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.fileClass
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.GrammarOptionValue
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.canonicalizeIpv6
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.SemanticTag
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.labeledRegions
+import net.sjrx.intellij.plugins.systemdunitfiles.settings.ExperimentalSettings
+
+/**
+ * Suggests rewriting an IPv6 address to its RFC 5952 canonical form (#363), behind the experimental
+ * flag. It walks the grammar's labeled value spans and, for those the grammar tagged
+ * [SemanticTag.IPV6], offers a quick-fix when the address isn't already canonical. Keying off the tag
+ * (rather than re-sniffing every literal span) means it only ever touches spans the grammar declared
+ * to be IPv6 addresses.
+ */
+class Ipv6CanonicalFormInspection : LocalInspectionTool() {
+
+  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+    val file = holder.file
+    if (file !is UnitFile || !file.language.isKindOf(UnitFileLanguage.INSTANCE)) return PsiElementVisitor.EMPTY_VISITOR
+    if (!ExperimentalSettings.getInstance(file.project).state.useGrammarParseEngine) return PsiElementVisitor.EMPTY_VISITOR
+    return MyVisitor(holder)
+  }
+
+  private class MyVisitor(private val holder: ProblemsHolder) : UnitFileVisitor() {
+    override fun visitPropertyType(property: UnitFilePropertyType) {
+      val section = PsiTreeUtil.getParentOfType(property, UnitFileSectionGroups::class.java) ?: return
+      val value = property.valueText ?: return
+      val validator = SemanticDataRepository.instance
+        .getOptionValidator(section.containingFile.fileClass(), section.sectionName, property.key)
+      if (validator !is GrammarOptionValue) return
+
+      for (region in validator.combinator.labeledRegions(value)) {
+        if (region.tag != SemanticTag.IPV6) continue // act only on spans the grammar declared IPv6
+        val text = value.substring(region.start, region.end)
+        val canonical = canonicalizeIpv6(text) ?: continue // e.g. an IPv4-tail form, out of scope
+        if (canonical == text) continue
+        holder.registerProblem(
+          property.valueNode.psi,
+          "IPv6 address is not in canonical form (RFC 5952); use '$canonical'",
+          ProblemHighlightType.WEAK_WARNING,
+          TextRange(region.start, region.end),
+          CanonicalizeIpv6QuickFix(region.start, text, canonical),
+        )
+      }
+    }
+  }
+}

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/intentions/CanonicalizeIpv6QuickFix.kt

@@ -0,0 +1,25 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.intentions
+
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import com.intellij.psi.util.PsiTreeUtil
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.impl.UnitFilePropertyImpl
+
+/**
+ * Replaces the IPv6 address at [offset] (within the option value) with its RFC 5952 [canonical] form.
+ */
+class CanonicalizeIpv6QuickFix(private val offset: Int, private val original: String, private val canonical: String) : LocalQuickFix {
+
+  override fun getName(): String = "Convert to canonical IPv6 '$canonical'"
+
+  override fun getFamilyName(): String = "Convert to canonical IPv6 (RFC 5952)"
+
+  override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+    val fullPropertyValue = descriptor.psiElement.text
+    val newText = fullPropertyValue.substring(0, offset) + canonical + fullPropertyValue.substring(offset + original.length)
+    val property = PsiTreeUtil.getParentOfType(descriptor.psiElement, UnitFilePropertyImpl::class.java) ?: return
+    val newElement = UnitElementFactory.createProperty(project, property.key, newText)
+    property.replace(newElement)
+  }
+}

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/Coloring.kt

@@ -23,8 +23,12 @@ enum class Role {
   OPERATOR,
 }
 
-/** A coloured span `[start, end)` and its [role]. */
-data class Region(val start: Int, val end: Int, val role: Role)
+/**
+ * A coloured span `[start, end)` with its [role] and an optional [tag]. [tag] is the grammar's
+ * declared identity for the span (e.g. [SemanticTag.IPV6]); features that act on spans by meaning
+ * rather than colour filter on it. `null` for plain per-token coloring and untagged [Labeled] spans.
+ */
+data class Region(val start: Int, val end: Int, val role: Role, val tag: SemanticTag? = null)
 
 /**
  * The role a terminal should get when it is NOT wrapped in [Labeled]. `null` means "do not colour"
@@ -42,6 +46,17 @@ fun defaultRole(terminal: TerminalCombinator): Role? = when (terminal) {
 private fun Array<out String>.allPunctuation(): Boolean =
   isNotEmpty() && all { choice -> choice.isNotEmpty() && choice.none(Char::isLetterOrDigit) }
 
+/**
+ * The explicit [Labeled] spans in [value] (e.g. a whole IP address), from the first fully-valid
+ * parse — i.e. structure the grammar marked, without the per-token coloring defaults. Used by
+ * features that act on semantic spans, such as IPv6 canonicalization.
+ */
+fun Combinator.labeledRegions(value: String): List<Region> {
+  val parse = parse(value, 0).filterIsInstance<Parse>()
+    .firstOrNull { it.end == value.length && it.tokens.all { token -> token.valid } } ?: return emptyList()
+  return parse.regions
+}
+
 /**
  * The coloured regions for [value]. Explicit [Labeled] regions win; any token not inside a labeled
  * region gets its terminal's [defaultRole]. Returns empty if no full parse exists — we don't colour

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/Combinators.kt

@@ -52,7 +52,7 @@ val IPV6_IPV4_SUFFIX_FIVE_HEXTET_BEFORE_ZERO_COMP =  SequenceCombinator(IPV6_HEX
 
 //val IPV6_ALL_ZEROS = DOUBLE_COLON
 
-val IPV6_ADDR = Labeled(Role.LITERAL, AlternativeCombinator(
+val IPV6_ADDR = Labeled(Role.LITERAL, tag = SemanticTag.IPV6, inner = AlternativeCombinator(
   IPV6_IPV4_SUFFIX_FULL,
   IPV6_IPV4_SUFFIX_ZERO_HEXTET_BEFORE_ZERO_COMP,
   IPV6_IPV4_SUFFIX_ONE_HEXTET_BEFORE_ZERO_COMP,

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/Ipv6.kt

@@ -0,0 +1,77 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar
+
+/*
+ * IPv6 canonicalization to RFC 5952 §4 (#363):
+ *  - lowercase hex,
+ *  - drop leading zeros in each 16-bit group,
+ *  - compress the longest run of all-zero groups to "::" (leftmost on a tie, only if the run is 2+),
+ *  - never compress a single zero group.
+ *
+ * Hand-rolled and dependency-free. The address is parsed to eight groups, then re-formatted. The
+ * mixed IPv4-tail notation (RFC 5952 §5, e.g. ::ffff:1.2.3.4) is intentionally out of scope for now —
+ * [canonicalizeIpv6] returns null for addresses containing a dotted IPv4 part, so they're left alone.
+ */
+fun canonicalizeIpv6(address: String): String? {
+  if (address.isEmpty() || '.' in address) return null
+  val groups = parseGroups(address) ?: return null
+  return format(groups)
+}
+
+private fun parseGroups(address: String): IntArray? {
+  val doubleColon = address.indexOf("::")
+  val groups: List<Int>
+  if (doubleColon >= 0) {
+    if (address.indexOf("::", doubleColon + 1) >= 0) return null // at most one "::"
+    val head = address.substring(0, doubleColon).split(":").filter { it.isNotEmpty() }
+    val tail = address.substring(doubleColon + 2).split(":").filter { it.isNotEmpty() }
+    val missing = 8 - head.size - tail.size
+    if (missing < 1) return null // "::" must stand for at least one zero group
+    groups = (head + List(missing) { "0" } + tail).map { parseHextet(it) ?: return null }
+  } else {
+    val parts = address.split(":")
+    if (parts.size != 8) return null
+    groups = parts.map { parseHextet(it) ?: return null }
+  }
+  return groups.toIntArray()
+}
+
+private fun parseHextet(s: String): Int? {
+  if (s.isEmpty() || s.length > 4) return null
+  val value = s.toIntOrNull(16) ?: return null
+  return if (value in 0..0xFFFF) value else null
+}
+
+private fun format(groups: IntArray): String {
+  // Longest run of consecutive zero groups (leftmost on ties); only compressible if length >= 2.
+  var runStart = -1
+  var runLen = 0
+  var i = 0
+  while (i < groups.size) {
+    if (groups[i] == 0) {
+      var j = i
+      while (j < groups.size && groups[j] == 0) j++
+      if (j - i > runLen) {
+        runLen = j - i
+        runStart = i
+      }
+      i = j
+    } else {
+      i++
+    }
+  }
+  if (runLen < 2) runStart = -1
+
+  val sb = StringBuilder()
+  i = 0
+  while (i < groups.size) {
+    if (i == runStart) {
+      sb.append("::")
+      i += runLen
+      continue
+    }
+    if (sb.isNotEmpty() && !sb.endsWith("::")) sb.append(":")
+    sb.append(Integer.toHexString(groups[i]))
+    i++
+  }
+  return sb.toString()
+}

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/Labeled.kt

@@ -1,14 +1,17 @@
 package net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar
 
 /**
- * Wraps [inner] and tags the whole matched span with a coloring [role] (#467 / #342).
+ * Wraps [inner] and marks the whole matched span with a coloring [role] and, optionally, a
+ * [SemanticTag] (#467 / #342).
  *
  * It is OPTIONAL and TRANSPARENT: matching (SyntacticMatch / SemanticMatch / parse) is delegated to
- * [inner] unchanged, so wrapping a sub-grammar affects only coloring, never validation or
+ * [inner] unchanged, so wrapping a sub-grammar affects only coloring/tagging, never validation or
  * completion. Use it where a composite value should read as one unit — e.g.
  * `Labeled(Role.LITERAL, IPV4_ADDR)` colors `127.0.0.1` as a single literal instead of per-octet.
+ * Pass [tag] when a feature needs to recognise the span by what the grammar declared it to be rather
+ * than by re-sniffing its text — e.g. `Labeled(Role.LITERAL, ..., SemanticTag.IPV6)`.
  */
-class Labeled(private val role: Role, private val inner: Combinator) : Combinator {
+class Labeled(private val role: Role, private val inner: Combinator, private val tag: SemanticTag? = null) : Combinator {
 
   override fun SyntacticMatch(value: String, offset: Int): MatchResult = inner.SyntacticMatch(value, offset)
 
@@ -18,7 +21,7 @@ class Labeled(private val role: Role, private val inner: Combinator) : Combinato
     inner.parse(value, offset).map { step ->
       when (step) {
         is Parse ->
-          if (step.end > offset) Parse(step.end, step.tokens, step.regions + Region(offset, step.end, role))
+          if (step.end > offset) Parse(step.end, step.tokens, step.regions + Region(offset, step.end, role, tag))
           else step // matched nothing; no region to add
         is Stuck -> step
       }

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/SemanticTag.kt

@@ -0,0 +1,14 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar
+
+/**
+ * A semantic identity a grammar can attach to a [Labeled] span, independent of its coloring [Role].
+ *
+ * Where [Role] answers "what colour?", a tag answers "what *is* this span?". It lets a feature act on
+ * a span because the grammar *declared* what it is, rather than re-sniffing the raw text and hoping no
+ * other labeled span happens to look the same. Currently only IPv6 canonicalization keys off it (see
+ * Ipv6CanonicalFormInspection); add members as other features need structural identity.
+ */
+enum class SemanticTag {
+  /** A whole IPv6 address (possibly with an IPv4 tail), as matched by `IPV6_ADDR`. */
+  IPV6,
+}

src/main/resources/META-INF/plugin.xml

@@ -82,6 +82,10 @@
                      groupPath="Unit files (systemd)" language="Unit File (systemd)"
                      shortName="MissingRequiredKey" displayName="Missing required key"
                      groupName="Validity" enabledByDefault="true" level="ERROR"/>
+    <localInspection implementationClass="net.sjrx.intellij.plugins.systemdunitfiles.inspections.Ipv6CanonicalFormInspection"
+                     groupPath="Unit files (systemd)" language="Unit File (systemd)"
+                     shortName="Ipv6CanonicalForm" displayName="IPv6 address not in canonical form (RFC 5952)"
+                     groupName="Style" enabledByDefault="true" level="WEAK WARNING"/>
     <localInspection implementationClass="net.sjrx.intellij.plugins.systemdunitfiles.inspections.IPAddressAllowOnlyInspection"
                      groupPath="Unit files (systemd)" language="Unit File (systemd)"
                      shortName="IPAddressAllowOnly" displayName="IPAddressAllow without IPAddressDeny"

src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/Ipv6CanonicalFormInspectionTest.kt

@@ -0,0 +1,62 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.inspections
+
+import com.intellij.lang.annotation.HighlightSeverity
+import net.sjrx.intellij.plugins.systemdunitfiles.AbstractUnitFileTest
+import net.sjrx.intellij.plugins.systemdunitfiles.settings.ExperimentalSettings
+import org.junit.Test
+
+/** End-to-end: a non-canonical IPv6 address is flagged and the quick-fix rewrites it (flag-gated). */
+class Ipv6CanonicalFormInspectionTest : AbstractUnitFileTest() {
+
+  override fun tearDown() {
+    try {
+      ExperimentalSettings.getInstance(project).state.useGrammarParseEngine = false
+    } finally {
+      super.tearDown()
+    }
+  }
+
+  private fun enableNewEngine() {
+    ExperimentalSettings.getInstance(project).state.useGrammarParseEngine = true
+  }
+
+  private fun hasCanonicalWarning() = myFixture.doHighlighting().any {
+    it.severity == HighlightSeverity.WEAK_WARNING && it.description?.contains("canonical form") == true
+  }
+
+  @Test
+  fun testNonCanonicalIsFlagged() {
+    enableNewEngine()
+    setupFileInEditor("file.service", "[Service]\nIPAddressAllow=2001:DB8::1")
+    enableInspection(Ipv6CanonicalFormInspection::class.java)
+    assertTrue(hasCanonicalWarning())
+  }
+
+  @Test
+  fun testAlreadyCanonicalIsNotFlagged() {
+    enableNewEngine()
+    setupFileInEditor("file.service", "[Service]\nIPAddressAllow=2001:db8::1")
+    enableInspection(Ipv6CanonicalFormInspection::class.java)
+    assertFalse(hasCanonicalWarning())
+  }
+
+  @Test
+  fun testNotFlaggedWhenFlagOff() {
+    setupFileInEditor("file.service", "[Service]\nIPAddressAllow=2001:DB8::1")
+    enableInspection(Ipv6CanonicalFormInspection::class.java)
+    assertFalse(hasCanonicalWarning())
+  }
+
+  @Test
+  fun testQuickFixRewritesToCanonical() {
+    enableNewEngine()
+    setupFileInEditor("file.service", "[Service]\nIPAddressAllow=2001:D${COMPLETION_POSITION}B8::1")
+    enableInspection(Ipv6CanonicalFormInspection::class.java)
+    myFixture.doHighlighting()
+
+    val fix = myFixture.findSingleIntention("Convert to canonical IPv6")
+    myFixture.launchAction(fix)
+
+    myFixture.checkResult("[Service]\nIPAddressAllow=2001:db8::1")
+  }
+}

src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/ColoringTest.kt

@@ -47,6 +47,29 @@ class ColoringTest {
     assertEquals(listOf(Region(0, 7, Role.LITERAL)), SequenceCombinator(IPV4_ADDR, EOF()).colorize("1.2.3.4"))
   }
 
+  @Test
+  fun testLabeledCarriesSemanticTag() {
+    // The optional tag rides on the Region so a feature can recognise a span by what the grammar
+    // declared it to be, instead of re-sniffing the text. Untagged Labeled spans stay tag == null.
+    val tagged = Labeled(Role.LITERAL, LiteralChoiceTerminal("ab"), SemanticTag.IPV6)
+    assertEquals(listOf(Region(0, 2, Role.LITERAL, SemanticTag.IPV6)), tagged.labeledRegions("ab"))
+    assertEquals(listOf(Region(0, 2, Role.LITERAL, null)), Labeled(Role.LITERAL, LiteralChoiceTerminal("ab")).labeledRegions("ab"))
+  }
+
+  @Test
+  fun testIpCombinatorsDeclareTheirIdentityStructurally() {
+    // IPV6_ADDR declares itself IPv6; IPV4_ADDR is untagged. The IPv6 inspection keys off this tag,
+    // so it never has to guess whether a literal span "looks like" an IPv6 address.
+    assertEquals(
+      listOf(Region(0, 3, Role.LITERAL, SemanticTag.IPV6)),
+      SequenceCombinator(IPV6_ADDR, EOF()).labeledRegions("::1"),
+    )
+    assertEquals(
+      listOf(Region(0, 7, Role.LITERAL, null)),
+      SequenceCombinator(IPV4_ADDR, EOF()).labeledRegions("1.2.3.4"),
+    )
+  }
+
   @Test
   fun testLabeledIsTransparentToValidation() {
     // Wrapping changes only colour: validation behaves exactly as the bare grammar.