Skip to content

Commit 5d00535

Browse files
reid-spencerclaude
andcommitted
Quote special-character identifiers in Identifier.format and prettify
RIDDL identifiers may be single-quoted to carry special characters (spaces, /, @, etc.), and the parser accepts that form. The prettifier emitted them unquoted, so parse -> prettify -> parse was not a round-trip for such names (e.g. a user named 'CI/CD Pipeline'). - Identifier.format now single-quotes non-bare names. Adds public Identifier.isBareIdentifier(String) and Identifier.format(String), matching CommonParser.simpleIdentifier ([A-Za-z][A-Za-z0-9_-]*) and the EBNF simple_identifier rule exactly. Hyphens are bare (correcting the original proposal); empty stays empty. - PathIdentifier.format wraps a whole path in one pair of quotes when any component is special ('a.CI/CD Pipeline.c') rather than quoting each component. - Parser: pathIdentifier gains a whole-path quoted form (dottedPathIdentifier | quotedPathIdentifier); the dotted form is tried first so existing inputs (including per-component a.'x'.b) parse unchanged. EBNF grammar updated and GBNF regenerated; both the TatSu EBNF validator and the GBNF validator pass. - Prettifier: five emit sites that rendered names via raw .id.value (including user names -- the reported repro) now go through .id.format. - Display: format() is also used by identify() and error-message templates, which supply their own quotes. The two identify methods plus 13 direct templates now use the raw value/path inside those quotes, so special and synthetic names (e.g. OnMessageClause 'command DooFoo') are not double-quoted. Restores exact pre-change message output. Tests: ASTTest covers Identifier.format (bare, hyphen, special, digit-leading, full quoted charset, empty), isBareIdentifier, and PathIdentifier whole-path quoting; new IdentifierQuotingRoundTripTest covers parse -> prettify -> parse fidelity for a special-char user, type, and path reference. Green on JVM and Native; JS compiles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 802867b commit 5d00535

12 files changed

Lines changed: 229 additions & 30 deletions

File tree

language/shared/src/main/resources/riddl/grammar/ebnf-grammar.ebnf

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ any_char_except_triple_backtick = /(?:[^`]|`(?!``)|``(?!`))+/ ;
1515
identifier = simple_identifier | quoted_identifier ;
1616
simple_identifier = /[A-Za-z][A-Za-z0-9_-]*/ ;
1717
quoted_identifier = /'[-A-Za-z0-9_+\\|\/@$%&,: ]+'/ ;
18-
path_identifier = identifier { "." identifier } ;
18+
path_identifier = dotted_path_identifier | quoted_path_identifier ;
19+
dotted_path_identifier = identifier { "." identifier } ;
20+
quoted_path_identifier = /'[-A-Za-z0-9_+\\|\/@$%&,: .]+'/ ;
1921
literal_string = '"' { string_char | escape_sequence } '"' ;
2022
escape_sequence = /\\[\\\"aefnrt]/ | hexEscape | unicodeEscape ;
2123
hexEscape = /\\x[0-9a-fA-F]{2,8}/ ;

language/shared/src/main/resources/riddl/grammar/riddl-grammar.gbnf

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# (llama.cpp constrained generation)
33
# AUTO-GENERATED from ebnf-grammar.ebnf by ebnf_to_gbnf.py
44
# DO NOT EDIT - modify the source EBNF or gbnf_overrides.gbnf instead
5-
# Generated: 2026-06-25 00:35 UTC
5+
# Generated: 2026-07-10 19:08 UTC
66

77
# Whitespace primitives
88
ws ::= [ \t\n\r]*
@@ -24,7 +24,9 @@ any-char-except-triple-backtick ::= ([^`] | "`" [^`] | "``" [^`])+ # (from over
2424
identifier ::= simple-identifier | quoted-identifier
2525
simple-identifier ::= [A-Za-z] [A-Za-z0-9_-]*
2626
quoted-identifier ::= "'" [-A-Za-z0-9_+\\|/@$%&,: ]+ "'"
27-
path-identifier ::= identifier ("." identifier)*
27+
path-identifier ::= dotted-path-identifier | quoted-path-identifier
28+
dotted-path-identifier ::= identifier ws ("." identifier)*
29+
quoted-path-identifier ::= "'" ws [-A-Za-z0-9_+\\|/@$%&,: .]+ ws "'"
2830
literal-string ::= "\"" (string-char | escape-sequence)* "\""
2931
escape-sequence ::= "\\" [\\"aefnrt] | hex-escape | unicode-escape
3032
hex-escape ::= "\\x" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F]? [0-9a-fA-F]? [0-9a-fA-F]? [0-9a-fA-F]? [0-9a-fA-F]? [0-9a-fA-F]?

language/shared/src/main/scala/com/ossuminc/riddl/language/AST.scala

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,45 @@ object AST:
161161
* The parsed value of the [[Identifier]]
162162
*/
163163
case class Identifier(loc: At, value: String) extends RiddlValue:
164-
override def format: String = value
164+
override def format: String = Identifier.format(value)
165165
override def isEmpty: Boolean = value.isEmpty
166166
end Identifier
167167

168168
/** Companion object for the Identifier class to provide the empty value */
169169
object Identifier:
170170
/** Definition of the empty [[Identifier]] */
171171
val empty: Identifier = Identifier(At.empty, "")
172+
173+
private def isAsciiLetter(c: Char): Boolean =
174+
(c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')
175+
176+
/** A character permitted after the first in a bare identifier. Kept in
177+
* sync with `CommonParser.simpleIdentifier`
178+
* (`[A-Za-z][A-Za-z0-9_\-]*`) and the EBNF `simple_identifier` rule.
179+
*/
180+
private def isBareIdChar(c: Char): Boolean =
181+
isAsciiLetter(c) || (c >= '0' && c <= '9') || c == '_' || c == '-'
182+
183+
/** True when `value` is a bare (unquoted) identifier and therefore can be
184+
* emitted to RIDDL source verbatim. Matches the parser's
185+
* `simpleIdentifier` rule exactly: an ASCII letter followed by any number
186+
* of ASCII letters, digits, underscores, or hyphens.
187+
*/
188+
def isBareIdentifier(value: String): Boolean =
189+
value.nonEmpty && isAsciiLetter(value.head) && value.tail.forall(isBareIdChar)
190+
191+
/** Render an identifier `value` as valid RIDDL source. A bare identifier is
192+
* emitted unchanged; anything else is single-quoted using the parser's
193+
* `quotedIdentifier` form (`'...'`). An empty value is preserved as empty.
194+
*
195+
* Note: RIDDL's quoted-identifier syntax has no escape for a single quote
196+
* and only admits `[A-Za-z0-9_+\-|/@$%&, :]`; a value outside that set
197+
* (e.g. containing `.` or `'`) cannot be represented in RIDDL at all, but
198+
* such values never arise from parsing (only from in-memory / JSON
199+
* construction). Quoting is the best available rendering.
200+
*/
201+
def format(value: String): String =
202+
if value.isEmpty || isBareIdentifier(value) then value else s"'$value'"
172203
end Identifier
173204

174205
/** Represents a segmented identifier to a definition in the model. Path Identifiers are parsed
@@ -181,7 +212,15 @@ object AST:
181212
* The list of strings that make up the path identifier
182213
*/
183214
case class PathIdentifier(loc: At, value: Seq[String]) extends RiddlValue:
184-
override def format: String = value.mkString(".")
215+
/** Render the path to RIDDL source. When every component is bare (or
216+
* empty), emit the plain dotted form `a.b.c`. When any component carries
217+
* special characters, wrap the whole dotted path in a single pair of
218+
* quotes — `'a.CI/CD Pipeline.c'` — using the parser's quoted-path form,
219+
* rather than quoting each component individually.
220+
*/
221+
override def format: String =
222+
if value.forall(p => p.isEmpty || Identifier.isBareIdentifier(p)) then value.mkString(".")
223+
else s"'${value.mkString(".")}'"
185224
override def isEmpty: Boolean = value.isEmpty || value.forall(_.isEmpty)
186225
end PathIdentifier
187226

@@ -352,11 +391,13 @@ object AST:
352391
* String A string that describes this reference
353392
*/
354393
def identify: String =
394+
// Human-readable display: supply our own quotes around the raw path, so
395+
// special-character names are not double-quoted by Identifier.format.
355396
s"${classTag[T].runtimeClass.getSimpleName} ${
356397
if id.nonEmpty then {
357-
id.map(_.format + ": ")
398+
id.map(_.value + ": ")
358399
} else ""
359-
}'${pathId.format}'"
400+
}'${pathId.value.mkString(".")}'"
360401
end identify
361402

362403
override def isEmpty: Boolean = pathId.isEmpty
@@ -385,7 +426,9 @@ object AST:
385426
if id.isEmpty then {
386427
s"Anonymous $kind"
387428
} else {
388-
s"$kind '${id.format}'"
429+
// Display supplies its own quotes; use the raw value so that
430+
// special-character names are not double-quoted by Identifier.format.
431+
s"$kind '${id.value}'"
389432
}
390433
end identify
391434

language/shared/src/main/scala/com/ossuminc/riddl/language/parsing/CommonParser.scala

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,29 @@ private[parsing] trait CommonParser(using pc: PlatformContext)
208208
}
209209
}
210210

211+
private def dottedPathIdentifier[u: P]: P[Seq[String]] = {
212+
P(anyIdentifier ~~ (Punctuation.dot ~~ anyIdentifier).repX(0)).map { case (first, strings) =>
213+
first +: strings
214+
}
215+
}
216+
217+
/** A whole path wrapped in a single pair of quotes with `.` separating the
218+
* components, e.g. `'a.CI/CD Pipeline.c'`. This lets an emitter quote a path
219+
* containing special-character components without quoting each component.
220+
* The character class is `quotedIdentifier`'s set plus `.`.
221+
*/
222+
private def quotedPathIdentifier[u: P]: P[Seq[String]] = {
223+
P("'" ~~ CharsWhileIn("a-zA-Z0-9_+\\-|/@$%&, :.", 1).! ~~ "'").map { s =>
224+
s.split('.').toIndexedSeq
225+
}
226+
}
227+
211228
def pathIdentifier[u: P]: P[PathIdentifier] = {
212-
P(Index ~ anyIdentifier ~~ (Punctuation.dot ~~ anyIdentifier).repX(0) ~~ Index).map {
213-
case (off1, first, strings, off2) =>
214-
PathIdentifier(at(off1, off2), first +: strings)
229+
// Try the dotted form first so existing inputs (including per-component
230+
// quoted parts like `a.'x'.b`) parse unchanged; fall back to the
231+
// whole-path quoted form only when a `.` appears inside the quotes.
232+
P(Index ~ (dottedPathIdentifier | quotedPathIdentifier) ~~ Index).map { case (off1, parts, off2) =>
233+
PathIdentifier(at(off1, off2), parts)
215234
}
216235
}
217236

language/shared/src/test/scala/com/ossuminc/riddl/language/ASTTest.scala

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,50 @@ class ASTTest extends AbstractTestingBasis {
3333
}
3434
}
3535

36+
"Identifier" should {
37+
"emit bare names verbatim" in {
38+
Identifier(At.empty, "Order").format mustBe "Order"
39+
Identifier(At.empty, "Order_2").format mustBe "Order_2"
40+
}
41+
"treat hyphenated names as bare (matching the parser's rule)" in {
42+
// simpleIdentifier is [A-Za-z][A-Za-z0-9_-]* — hyphens are allowed
43+
Identifier(At.empty, "my-entity").format mustBe "my-entity"
44+
}
45+
"quote names with special characters" in {
46+
Identifier(At.empty, "CI/CD Pipeline").format mustBe "'CI/CD Pipeline'"
47+
Identifier(At.empty, "Order Item").format mustBe "'Order Item'"
48+
}
49+
"quote names that start with a digit" in {
50+
Identifier(At.empty, "3dModel").format mustBe "'3dModel'"
51+
}
52+
"quote every character the quoted-identifier form allows" in {
53+
Identifier(At.empty, "a+b-c|d/e@f$g%h&i,j:k").format mustBe "'a+b-c|d/e@f$g%h&i,j:k'"
54+
}
55+
"preserve an empty value" in {
56+
Identifier(At.empty, "").format mustBe ""
57+
}
58+
"expose isBareIdentifier that agrees with simpleIdentifier" in {
59+
Identifier.isBareIdentifier("Order") mustBe true
60+
Identifier.isBareIdentifier("my-entity") mustBe true
61+
Identifier.isBareIdentifier("A1_b-2") mustBe true
62+
Identifier.isBareIdentifier("3d") mustBe false // starts with a digit
63+
Identifier.isBareIdentifier("_x") mustBe false // must start with a letter
64+
Identifier.isBareIdentifier("CI/CD") mustBe false
65+
Identifier.isBareIdentifier("") mustBe false
66+
}
67+
}
68+
69+
"PathIdentifier quoting" should {
70+
"emit an all-bare path as a plain dotted form" in {
71+
PathIdentifier(At.empty, Seq("A", "B", "C")).format mustBe "A.B.C"
72+
PathIdentifier(At.empty, Seq("my-ctx", "my-entity")).format mustBe "my-ctx.my-entity"
73+
}
74+
"wrap the whole path in one pair of quotes when a component is special" in {
75+
PathIdentifier(At.empty, Seq("A", "CI/CD Pipeline", "C")).format mustBe "'A.CI/CD Pipeline.C'"
76+
PathIdentifier(At.empty, Seq("CI/CD Pipeline")).format mustBe "'CI/CD Pipeline'"
77+
}
78+
}
79+
3680
"Types" should {
3781
"support domain definitions" in {
3882
Domain((0, 0), Identifier((1, 1), "foo")) must be
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2019-2026 Ossum Inc.
3+
*
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package com.ossuminc.riddl.passes.prettify
8+
9+
import com.ossuminc.riddl.language.AST.*
10+
import com.ossuminc.riddl.language.Finder
11+
import com.ossuminc.riddl.language.parsing.{RiddlParserInput, TopLevelParser}
12+
import com.ossuminc.riddl.passes.validate.AbstractValidatingTest
13+
import com.ossuminc.riddl.passes.{Pass, PassInput, PassesOutput}
14+
import com.ossuminc.riddl.utils.pc
15+
16+
import org.scalatest.*
17+
18+
/** Verifies that special-character identifiers survive a
19+
* parse -> prettify -> parse round-trip now that `Identifier.format` /
20+
* `PathIdentifier.format` single-quote non-bare names.
21+
*/
22+
class IdentifierQuotingRoundTripTest extends AbstractValidatingTest {
23+
24+
private def parse(src: String, origin: String): Root =
25+
TopLevelParser.parseInput(RiddlParserInput(src, origin)) match
26+
case Right(root) => root
27+
case Left(msgs) => fail(s"parse of $origin failed:\n${msgs.format}")
28+
29+
/** Run the prettifier (flatten) over a Root and return the rendered source. */
30+
private def prettify(root: Root): String =
31+
val creators = Pass.standardPasses :+ { (in: PassInput, out: PassesOutput) =>
32+
PrettifyPass(in, out, PrettifyPass.Options(flatten = true, inputDir = ""))
33+
}
34+
val result = Pass.runThesePasses(PassInput(root), creators)
35+
result.outputs
36+
.outputOf[PrettifyOutput](PrettifyPass.name)
37+
.getOrElse(fail("PrettifyPass produced no output"))
38+
.state
39+
.filesAsString
40+
41+
"Identifier quoting" should {
42+
43+
"round-trip a user whose name has special characters (the repro)" in {
44+
(td: TestData) =>
45+
val src = """domain D is { user 'CI/CD Pipeline' is "an operator" }"""
46+
val root1 = parse(src, "src")
47+
Finder(root1).recursiveFindByType[User].head.id.value mustBe "CI/CD Pipeline"
48+
49+
val pretty = prettify(root1)
50+
pretty must include("user 'CI/CD Pipeline'")
51+
52+
// The prettified output must re-parse to the same identifier value.
53+
val root2 = parse(pretty, "regen")
54+
Finder(root2).recursiveFindByType[User].head.id.value mustBe "CI/CD Pipeline"
55+
}
56+
57+
"round-trip a type whose name has special characters" in { (td: TestData) =>
58+
val src = """domain D is { type 'CI/CD Pipeline' is String }"""
59+
val pretty = prettify(parse(src, "src"))
60+
pretty must include("type 'CI/CD Pipeline' is")
61+
val types = Finder(parse(pretty, "regen")).recursiveFindByType[Type]
62+
types.map(_.id.value) must contain("CI/CD Pipeline")
63+
}
64+
65+
"round-trip a path reference with a special-character component" in {
66+
(td: TestData) =>
67+
// A path whose middle component has a space: emitted as one quoted
68+
// whole path 'D.Weird Name', parsed back to the same components.
69+
val src =
70+
"""domain D is {
71+
| type 'Weird Name' is String
72+
| type Alias is 'D.Weird Name'
73+
|}
74+
|""".stripMargin
75+
def aliasPath(root: Root): Seq[String] =
76+
Finder(root).recursiveFindByType[Type].find(_.id.value == "Alias").get.typEx match
77+
case ate: AliasedTypeExpression => ate.pathId.value
78+
case other => fail(s"expected AliasedTypeExpression, got $other")
79+
80+
val root1 = parse(src, "src")
81+
aliasPath(root1) mustBe Seq("D", "Weird Name")
82+
83+
val pretty = prettify(root1)
84+
pretty must include("'D.Weird Name'")
85+
86+
aliasPath(parse(pretty, "regen")) mustBe Seq("D", "Weird Name")
87+
}
88+
}
89+
}

passes/shared/src/main/scala/com/ossuminc/riddl/passes/prettify/PrettifyVisitor.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ class PrettifyVisitor(options: PrettifyPass.Options)(using PlatformContext) exte
271271
def doUser(user: User): Unit =
272272
state.withCurrent { rfe =>
273273
rfe
274-
.addIndent(s"user ${user.id.value} is \"${user.is_a.s}\"")
274+
.addIndent(s"user ${user.id.format} is \"${user.is_a.s}\"")
275275
.emitMetaData(user.metadata)
276276
}
277277
end doUser

passes/shared/src/main/scala/com/ossuminc/riddl/passes/prettify/RiddlFileEmitter.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ case class RiddlFileEmitter(url: URL)(using PlatformContext) extends FileBuilder
162162
add(s"any of {").nl.incr
163163
val enumerators: String = enumeration.enumerators.toSeq
164164
.map { enumerator =>
165-
enumerator.id.value + enumerator.enumVal.fold("")(x => s"($x)")
165+
enumerator.id.format + enumerator.enumVal.fold("")(x => s"($x)")
166166
}
167167
.mkString(s"$spc", s",$new_line$spc", new_line)
168168
add(enumerators).decr.addLine("}")
@@ -179,14 +179,14 @@ case class RiddlFileEmitter(url: URL)(using PlatformContext) extends FileBuilder
179179
}
180180

181181
def emitField(field: Field): this.type =
182-
add(s"${field.id.value}: ")
182+
add(s"${field.id.format}: ")
183183
emitTypeExpression(field.typeEx)
184184
emitMetaData(field.metadata)
185185
this
186186
end emitField
187187

188188
def emitMethod(method: Method): this.type =
189-
add(s"${method.id.value}(${method.args.map(_.format).mkString(", ")}): ")
189+
add(s"${method.id.format}(${method.args.map(_.format).mkString(", ")}): ")
190190
emitTypeExpression(method.typeEx)
191191
emitMetaData(method.metadata)
192192
this
@@ -308,7 +308,7 @@ case class RiddlFileEmitter(url: URL)(using PlatformContext) extends FileBuilder
308308
}
309309

310310
def emitType(t: Type): this.type = {
311-
add(s"${spc}type ${t.id.value} is ")
311+
add(s"${spc}type ${t.id.format} is ")
312312
emitTypeExpression(t.typEx)
313313
emitMetaData(t.metadata)
314314
this

passes/shared/src/main/scala/com/ossuminc/riddl/passes/resolve/ReferenceMap.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ case class ReferenceMap(messages: Messages.Accumulator) {
9191
val className = classTag[T].runtimeClass.getSimpleName
9292
messages.addError(
9393
pid.loc,
94-
s"Path Id '${pid.format} found ${x.identify} but a $className was expected",
95-
suggestion = s"Point '${pid.format}' at a $className, or rename the reference to match the intended $className."
94+
s"Path Id '${pid.value.mkString(".")} found ${x.identify} but a $className was expected",
95+
suggestion = s"Point '${pid.value.mkString(".")}' at a $className, or rename the reference to match the intended $className."
9696
)
9797
None
9898
}

0 commit comments

Comments
 (0)