Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ jobs:
build:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
distribution: [ 'temurin' ]
java: [ '21' ]
name: Java ${{ matrix.Java }} Build
grammarParseEngine: [ 'false', 'true' ]
name: Java ${{ matrix.Java }} Build (grammar engine ${{ matrix.grammarParseEngine }})
steps:
- name: Check out Git repository
uses: actions/checkout@v7
Expand All @@ -46,10 +48,11 @@ jobs:
mkdir -p ./build
docker compose --project-directory ./systemd-build up --build
./generate-changelog > build/CHANGELOG
./gradlew test buildPlugin
./gradlew test buildPlugin -Dsystemd.unit.grammarParseEngine=${{ matrix.grammarParseEngine }}
./gradlew --stop
- name: Publish Unit Test Results
uses: EnricoMi/publish-unit-test-result-action@v2
if: always()
with:
check_name: "Unit Test Results (grammar engine ${{ matrix.grammarParseEngine }})"
junit_files: build/test-results/**/*.xml
3 changes: 3 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ tasks {
test {
testLogging.showExceptions = true
testLogging.setExceptionFormat("full")
// Forward the experimental grammar-engine flag so CI can run the whole suite twice: once on the
// original engine and once with -Dsystemd.unit.grammarParseEngine=true (see GrammarOptionValue).
systemProperty("systemd.unit.grammarParseEngine", System.getProperty("systemd.unit.grammarParseEngine", "false"))
}
}

Expand Down
26 changes: 26 additions & 0 deletions ci/release.Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,11 @@ pipeline {
}
}
stage("Build") {
// Two independent kubernetes pods => two separate workspaces => safe to run concurrently. The
// second pod re-runs the whole unit-test suite with the new grammar engine forced on, so the
// experimental path is validated on every release build before it ships.
parallel {
stage("Build & Publish") {
agent {
kubernetes {
//cloud 'kubernetes'
Expand Down Expand Up @@ -394,6 +399,27 @@ pipeline {
}
}
}
}
stage("Unit Tests (new grammar engine)") {
agent {
kubernetes {
//cloud 'kubernetes'
defaultContainer 'worker-pod'

// language=yaml
yaml buildPodDefinition("${env.DOCKER_REGISTRY_PREFIX}/systemd-plugin-build-environment:$buildEnvironmentHash", false,false)
//workspaceVolume hostPathWorkspaceVolume('/opt/jenkins/workspace')
}
}
steps {
unstash 'systemd-build-build'
unstash 'ubuntu-units'
sh("""
./gradlew --no-daemon -I ./build-cache-init.gradle.kts -I ./repo-cache-init.gradle.kts --build-cache test -Dsystemd.unit.grammarParseEngine=true
""")
}
}
}
}
}
post {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ open class AlternativeCombinator(vararg val tokens: Combinator) : Combinator {
return match(value, offset, Combinator::SemanticMatch)
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> =
// Offer every alternative's steps (matches and dead ends), so option order no longer affects
// correctness, and a failing branch still contributes what it expected.
tokens.asSequence().flatMap { it.parse(value, offset) }

override fun toString(): String = toStringIndented(0)

override fun toStringIndented(indent: Int): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,21 @@ interface Combinator {
*/
fun SemanticMatch(value : String, offset: Int): MatchResult

/**
* List-of-successes matcher (#467). Returns EVERY way this combinator can proceed from [offset] in
* [value], lazily, as a stream of [ParseStep]s: a [Parse] for each way it matched, and a [Stuck]
* for each dead end (carrying where it got stuck and what was expected there).
*
* This lives alongside Syntactic/SemanticMatch and is a single lenient pass: each [ParsedToken]
* carries a `valid` flag for the strict (semantic) check. Because every alternative is offered
* rather than the first greedy one committed to, matching is complete — e.g.
* Seq(ZeroOrMore("a"), "a") on "aa" matches, because ZeroOrMore offers the shorter match too.
*
* Returning [Stuck] as a value (rather than an empty sequence) means failure information — how far
* we got and what we expected — travels back through the return value, so no side channel is
* needed to localize errors.
*/
fun parse(value: String, offset: Int): Sequence<ParseStep>

fun toStringIndented(indent: Int): String
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class EOF : Combinator {
}
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> =
if (offset == value.length) sequenceOf(Parse(offset, emptyList()))
else sequenceOf(Stuck(offset, setOf(this))) // expected end-of-input here

override fun toStringIndented(indent: Int): String {
return "EOF"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ class FlexibleLiteralChoiceTerminal(vararg val choices: String) : TerminalCombin
return NoMatch.copy(longestMatch = offset)
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> {
// Lenient shape match (so a wrong token like AF_BOGUS still matches and can be highlighted),
// valid only if the matched text is one of the exact choices.
val m = syntaticMatch.matchAt(value, offset) ?: return sequenceOf(Stuck(offset, setOf(this)))
val text = m.value
val valid = choices.any { it == text }
return sequenceOf(Parse(offset + text.length, listOf(ParsedToken(offset, offset + text.length, text, this, valid))))
}

override fun toString(): String {
return if (choices.size == 1) {
"Literal(\"${choices[0]}\")"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ import com.intellij.codeInspection.LocalQuickFix
import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.openapi.diagnostic.Logger
import com.intellij.openapi.progress.ProcessCanceledException
import com.intellij.openapi.progress.ProgressManager
import com.intellij.openapi.project.Project
import com.intellij.openapi.util.TextRange
import net.sjrx.intellij.plugins.systemdunitfiles.intentions.ReplaceInvalidLiteralChoiceQuickFix
import net.sjrx.intellij.plugins.systemdunitfiles.psi.UnitFilePropertyType
import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.SemanticDataRepository
import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.OptionValueInformation
import net.sjrx.intellij.plugins.systemdunitfiles.settings.ExperimentalSettings

open class GrammarOptionValue(
override val validatorName: String,
Expand All @@ -34,6 +37,11 @@ open class GrammarOptionValue(
override fun generateProblemDescriptors(property: UnitFilePropertyType, holder: ProblemsHolder) {
val value = property.valueText ?: return

if (FORCE_PARSE_ENGINE || ExperimentalSettings.getInstance(property.project).state.useGrammarParseEngine) {
generateProblemDescriptorsViaParse(property, value, holder)
return
}

val syntaticMatch = combinator.SyntacticMatch(value, 0)

try {
Expand Down Expand Up @@ -114,7 +122,66 @@ open class GrammarOptionValue(

}

/**
* Experimental path (#467): validate via the list-of-successes engine and map the [ParseOutcome]
* onto the same problem descriptors the SyntacticMatch/SemanticMatch path produces. Gated behind
* [ExperimentalSettings.useGrammarParseEngine] so the original engine remains the default.
*/
private fun generateProblemDescriptorsViaParse(property: UnitFilePropertyType, value: String, holder: ProblemsHolder) {
val outcome = try {
combinator.validate(value) { ProgressManager.checkCanceled() }
} catch (e: ProcessCanceledException) {
throw e
} catch (e: RuntimeException) {
LOG.error("Error while processing ${property.key} with value $value", e)
holder.registerProblem(property.valueNode.psi, "Internal error, please report an bug to the systemd plugin. Include the Key and Value used.", ProblemHighlightType.ERROR)
return
}

when (outcome) {
is ParseOutcome.Valid -> return

is ParseOutcome.SyntaxError -> {
// Highlight from where parsing got stuck to the end (or everything if it reached the end).
val tr = if (outcome.furthest < value.length) {
TextRange(outcome.furthest, value.length)
} else {
TextRange(0, value.length)
}
holder.registerProblem(property.valueNode.psi, "${property.key}'s value does not match the expected format. Possible reasons include unrecognized characters or premature end of input.", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, tr)
}

is ParseOutcome.SemanticError -> {
// Well-formed but invalid: highlight the offending token, and offer literal replacements.
val bad = outcome.badToken
val tr = TextRange(bad.start, bad.end)

val quickFixes = mutableListOf<LocalQuickFix>()
val choices = when (val terminal = bad.terminal) {
is LiteralChoiceTerminal -> terminal.choices
is FlexibleLiteralChoiceTerminal -> terminal.choices
else -> emptyArray()
}
for (choice in choices) {
quickFixes.add(ReplaceInvalidLiteralChoiceQuickFix(bad.start, bad.text, choice))
}

holder.registerProblem(property.valueNode.psi, "${property.key}'s value is correctly formatted but seems invalid.", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, tr, *quickFixes.toTypedArray())
}
}
}

companion object {
private val LOG = Logger.getInstance(SemanticDataRepository::class.java)

/**
* Forces the new list-of-successes engine for validation regardless of the per-project setting,
* used to run the whole unit-test suite against it (CI runs the suite twice: once without and
* once with -Dsystemd.unit.grammarParseEngine=true). Only the validation engine is forced; the
* cosmetic annotators stay on the user flag, so problem counts are unchanged and only exact
* error spans/messages can differ between engines.
*/
@JvmField
val FORCE_PARSE_ENGINE: Boolean = java.lang.Boolean.getBoolean("systemd.unit.grammarParseEngine")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ class IntegerTerminal(private val minInclusive: Long,private val maxExclusive: L
}
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> {
val m = intRegex.matchAt(value, offset) ?: return sequenceOf(Stuck(offset, setOf(this)))
val text = m.value
// Lenient: any integer matches (so we can locate it); valid only if it is within range.
val valid = text.toLongOrNull()?.let { it >= minInclusive && it < maxExclusive } ?: false
return sequenceOf(Parse(offset + text.length, listOf(ParsedToken(offset, offset + text.length, text, this, valid))))
}

override fun toString(): String {
return "Int($minInclusive,$maxExclusive)"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ class LiteralChoiceTerminal(vararg var choices: String) : TerminalCombinator {
return match(value, offset)
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> {
// Offer EVERY choice that matches at this offset, not just the longest. When choices share a
// prefix more than one can match here -- e.g. CollectMode's ("inactive", "inactive-or-failed")
// both match at the start of "inactive-or-failed". A greedy matcher would have to commit to one
// and could dead-end later (taking "inactive" when the grammar needed "inactive-or-failed", or
// vice versa); returning both as separate Parses lets the rest of the grammar pick the branch
// that actually completes, with no backtracking. Every choice here is an exact literal, so each
// matched token is always strictly valid (valid = true) -- the wrong-value case is what
// FlexibleLiteralChoiceTerminal handles, where a token can match the shape but be valid = false.
val matches = choices.filter { value.startsWith(it, offset) }
return if (matches.isEmpty()) sequenceOf(Stuck(offset, setOf(this)))
else matches.asSequence()
.map { Parse(offset + it.length, listOf(ParsedToken(offset, offset + it.length, it, this, valid = true))) }
}

override fun toString(): String {
return if (choices.size == 1) {
"Literal(\"${choices[0]}\")"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,25 @@ class OneOrMore(val combinator : Combinator) : Combinator {
return match(value, offset, combinator::SemanticMatch)
}

override fun parse(value: String, offset: Int): Sequence<ParseStep> {
// Same as ZeroOrMore, but the first repetition is mandatory (and must make progress).
fun extend(from: Parse): Sequence<ParseStep> = sequence {
yield(from)
for (step in combinator.parse(value, from.end)) {
when (step) {
is Parse -> if (step.end > from.end) yieldAll(extend(Parse(step.end, from.tokens + step.tokens)))
is Stuck -> yield(step)
}
}
}
return combinator.parse(value, offset).flatMap { step ->
when (step) {
is Parse -> if (step.end > offset) extend(step) else emptySequence()
is Stuck -> sequenceOf<ParseStep>(step) // the mandatory first repetition failed
}
}
}

override fun toString(): String = toStringIndented(0)

override fun toStringIndented(indent: Int): String {
Expand Down
Loading
Loading