Improve warnings and invalid values for systemd

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

@@ -0,0 +1,100 @@
+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.PsiElement
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.util.PsiTreeUtil
+import net.sjrx.intellij.plugins.systemdunitfiles.UnitFileLanguage
+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.UnitFileValueType
+import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFileVisitor
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.SemanticDataRepository
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.ValueWarning
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.ValueWarnings
+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.deprecatedTokens
+
+/**
+ * Warns about values systemd accepts but complains about (#565): deprecated or obsolete spellings, unsafe settings,
+ * and forms systemd silently corrects. Invalid values are the InvalidValue inspection's job; this one only speaks up
+ * when the value is otherwise accepted.
+ *
+ * It reports two sources:
+ *  - tokens a grammar marks deprecated ([deprecatedTokens], e.g. `FlexibleLiteralChoiceTerminal.deprecating`);
+ *  - whatever a validator implementing [ValueWarnings] returns.
+ */
+class ValueWarningInspection : LocalInspectionTool() {
+  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+    val file = holder.file
+    return if (file !is UnitFile || !file.getLanguage().isKindOf(UnitFileLanguage.INSTANCE)) {
+      PsiElementVisitor.EMPTY_VISITOR
+    } else 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)
+
+      val warnings = mutableListOf<ValueWarning>()
+      if (validator is GrammarOptionValue) {
+        validator.combinator.deprecatedTokens(value).mapTo(warnings) { ValueWarning(it.start, it.end, it.message, ProblemHighlightType.GENERIC_ERROR_OR_WARNING) }
+      }
+      if (validator is ValueWarnings) {
+        warnings += validator.valueWarnings(property.key, value)
+      }
+
+      if (warnings.isEmpty()) return
+      val valueElement = property.valueNode.psi
+      val offsets = valueTextOffsets(valueElement, value)
+      for (warning in warnings) {
+        // The warning's span is in valueText (lines joined, whitespace trimmed); map it back onto the value element.
+        val range = if (offsets != null && warning.start < warning.end && warning.end <= value.length) {
+          TextRange(offsets[warning.start], offsets[warning.end - 1] + 1)
+        } else {
+          TextRange(0, valueElement.textLength)
+        }
+        holder.registerProblem(valueElement, warning.message, warning.highlightType, range)
+      }
+    }
+  }
+}
+
+/**
+ * For each character of [value] (the element's valueText), its offset inside [valueElement]. Mirrors
+ * UnitFileValueType.getValue(): each value line is trimmed (String.trim(), i.e. chars <= ' '), a trailing continuation
+ * backslash becomes a space, and the joined result is trimmed again. Null if the reconstruction does not match [value].
+ */
+private fun valueTextOffsets(valueElement: PsiElement, value: String): IntArray? {
+  val text = StringBuilder()
+  val offsets = mutableListOf<Int>()
+  var child = valueElement.firstChild
+  while (child != null) {
+    if (UnitFileValueType.valueTypes.contains(child.node.elementType)) {
+      val line = child.text
+      var from = 0
+      var to = line.length
+      while (from < to && line[from] <= ' ') from++
+      while (to > from && line[to - 1] <= ' ') to--
+      for (i in from until to) {
+        text.append(if (i == to - 1 && line[i] == '\\') ' ' else line[i])
+        offsets += child.startOffsetInParent + i
+      }
+    }
+    child = child.nextSibling
+  }
+  var from = 0
+  var to = text.length
+  while (from < to && text[from] <= ' ') from++
+  while (to > from && text[to - 1] <= ' ') to--
+  if (text.substring(from, to) != value) return null
+  return offsets.subList(from, to).toIntArray()
+}

src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/ValueWarnings.kt

@@ -0,0 +1,57 @@
+package net.sjrx.intellij.plugins.systemdunitfiles.semanticdata
+
+import com.intellij.codeInspection.ProblemHighlightType
+
+/**
+ * A part of a value that systemd accepts but warns about (#565): an obsolete spelling it rewrites, a setting it calls
+ * unsafe, or a form it silently corrects. [start, end) is the span inside the value; [message] says what systemd does
+ * with it and what to write instead. [highlightType] follows how loudly systemd itself complains: GENERIC_ERROR_OR_WARNING
+ * for its warnings, WEAK_WARNING for what it only logs as a notice.
+ */
+data class ValueWarning(val start: Int, val end: Int, val message: String, val highlightType: ProblemHighlightType)
+
+/**
+ * Implemented by option validators that know values systemd accepts with a warning. The ValueWarning inspection
+ * reports what [valueWarnings] returns.
+ *
+ * Contract: return warnings only for values the validator otherwise accepts, so a warning never piles on an error.
+ */
+interface ValueWarnings {
+  fun valueWarnings(key: String, value: String): List<ValueWarning>
+}
+
+/**
+ * patch_var_run(): an absolute path whose first two components are `var` and `run` is rewritten to /run/ with a
+ * notice. path_startswith() skips empty and "." components, so `//var/./run/x` counts as well.
+ * https://github.com/systemd/systemd/blob/0f0682b773/src/core/load-fragment.c#L573-L601
+ */
+fun legacyVarRunWarning(key: String, value: String): ValueWarning? {
+  if (!value.startsWith("/")) return null
+  val components = value.split('/').filter { it.isNotEmpty() && it != "." }
+  if (components.size < 2 || components[0] != "var" || components[1] != "run") return null
+  val replacement = "/run/" + components.drop(2).joinToString("/")
+  return ValueWarning(
+    0,
+    value.length,
+    "$key=$value is below the legacy directory /var/run/; systemd rewrites it to $replacement and logs a notice. " +
+      "Use $replacement instead.",
+    highlightType = ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+  )
+}
+
+/**
+ * config_parse_in_addr_prefix() with the "warn" ltype (Address= and Peer= in [Network]/[Address]): an address without
+ * "/N" is accepted with the full-width prefix and a warning.
+ * https://github.com/systemd/systemd/blob/0f0682b773/src/shared/conf-parser.c#L2114-L2149
+ */
+fun missingPrefixLengthWarning(key: String, value: String): ValueWarning? {
+  if (value.contains('/')) return null
+  val prefix = if (value.contains(':')) 128 else 32
+  return ValueWarning(
+    0,
+    value.length,
+    "$key=$value has no prefix length; systemd assumes /$prefix and logs a warning. Write the prefix length explicitly, " +
+      "e.g. $value/$prefix for a single address or the length of the subnet.",
+    highlightType = ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+  )
+}

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

@@ -1,8 +1,11 @@
 package net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues
 
 import com.google.common.collect.ImmutableSet
+import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.openapi.project.Project
 import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.Validator
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.ValueWarning
+import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.ValueWarnings
 import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.AlternativeCombinator
 import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.BOOLEAN
 import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.EOF
@@ -34,7 +37,23 @@ class EmergencyActionOptionValue : AbstractGrammarEnumOptionValue(validOptions,
   }
 }
 
-class KillModeOptionValue : AbstractGrammarEnumOptionValue(validOptions, VALIDATOR_NAME) {
+class KillModeOptionValue : AbstractGrammarEnumOptionValue(validOptions, VALIDATOR_NAME), ValueWarnings {
+
+  /**
+   * KillMode=none is accepted with a warning that it is unsafe and deprecated.
+   * https://github.com/systemd/systemd/blob/0f0682b773/src/core/load-fragment.c#L871-L876
+   */
+  override fun valueWarnings(key: String, value: String): List<ValueWarning> {
+    if (value != "none") return emptyList()
+    return listOf(
+      ValueWarning(
+        0, value.length,
+        "$key=none is unsafe and deprecated: systemd stops managing the service's processes, so they keep running after " +
+          "the service is stopped or restarted. Use mixed or control-group instead.",
+        highlightType = ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+      )
+    )
+  }
 
   companion object {
     private val validOptions: Set<String> = ImmutableSet.of("control-group", "process", "mixed", "none")