Skip to content

Commit 09da84b

Browse files
respencer-nclclaude
andcommitted
Fix prettify to retain multi-file include/import structure
Previously, `riddlc prettify` always produced a single flattened file regardless of --single-file flag. Root causes: singleFile defaulted to true, PrettifyPass.Options lacked outputDir/topFile, PrettifyState used dead "nada" paths, BASTImport was invisible to HierarchyPass, and the include directive had a missing closing quote. Changes: - Default singleFile to false (multi-file is the norm) - Add topFile/outputDir to PrettifyPass.Options, wire from command - Fix PrettifyState URL normalization for absolute output paths - Add BASTImport handling to HierarchyPass, PassVisitor, VisitingPass - Fix include quote bug in PrettifyVisitor - Fix writeOutput to use output dir consistently with TRUNCATE_EXISTING - Add regression tests for include structure preservation and flattening Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f3a0782 commit 09da84b

8 files changed

Lines changed: 147 additions & 11 deletions

File tree

commands/shared/src/main/scala/com/ossuminc/riddl/commands/PrettifyCommand.scala

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ object PrettifyCommand {
2525
inputFile: Option[Path] = None,
2626
outputDir: Option[Path] = Some(Path.of(System.getProperty("java.io.tmpdir"))),
2727
projectName: Option[String] = None,
28-
singleFile: Boolean = true
28+
singleFile: Boolean = false
2929
) extends TranslationCommand.Options
3030
with PassOptions
3131
with PassCommandOptions:
@@ -93,9 +93,17 @@ class PrettifyCommand(using pc: PlatformContext)
9393
override def getPasses(
9494
options: PrettifyCommand.Options
9595
): PassCreators = {
96+
val topFile = options.inputFile
97+
.map(_.getFileName.toString).getOrElse("")
98+
val outDir = options.outputDir
99+
.map(_.toString).getOrElse("")
96100
standardPasses ++ Seq(
97101
{ (input: PassInput, outputs: PassesOutput) =>
98-
PrettifyPass(input, outputs, PrettifyPass.Options(flatten = options.singleFile))
102+
PrettifyPass(input, outputs, PrettifyPass.Options(
103+
flatten = options.singleFile,
104+
topFile = topFile,
105+
outputDir = outDir
106+
))
99107
}
100108
)
101109
}
@@ -140,15 +148,16 @@ class PrettifyCommand(using pc: PlatformContext)
140148
StandardOpenOption.WRITE
141149
)
142150
else
143-
val base = dirOverrides.getOrElse(Path.of("."))
144151
output.state.withFiles { (file: RiddlFileEmitter) =>
145152
val content = file.toString
146-
val path = base.resolve(file.url.path)
153+
val path = dir.resolve(file.url.path)
154+
Files.createDirectories(path.getParent)
147155
Files.writeString(
148156
path,
149157
content,
150158
StandardCharsets.UTF_8,
151-
StandardOpenOption.CREATE_NEW,
159+
StandardOpenOption.CREATE,
160+
StandardOpenOption.TRUNCATE_EXISTING,
152161
StandardOpenOption.WRITE
153162
)
154163
}

commands/shared/src/test/scala/com/ossuminc/riddl/commands/PrettifyCommandTest.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ class PrettifyCommandTest extends RunCommandSpecBase {
3030
val expected = PrettifyCommand.Options(
3131
inputFile = Some(Path.of("nada.riddl")),
3232
outputDir = Some(Path.of("commands/target/prettify/")),
33-
projectName = Some("Nada")
33+
projectName = Some("Nada"),
34+
singleFile = true
3435
)
3536
cmd.loadOptionsFrom(conf) match {
3637
case Left(errors) => fail(errors.format)

passes/jvm-native/src/test/scala/com/ossuminc/riddl/passes/RiddlTest.scala

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package com.ossuminc.riddl.passes
88

99
import com.ossuminc.riddl.language.parsing.RiddlParserInput
1010
import com.ossuminc.riddl.language.AST
11+
import com.ossuminc.riddl.passes.prettify.{PrettifyOutput, PrettifyPass}
1112
import com.ossuminc.riddl.utils.{AbstractTestingBasis, PathUtils, PlatformContext}
1213
import com.ossuminc.riddl.utils.{ec, pc, Await}
1314
import com.ossuminc.riddl.utils.Timer
@@ -169,5 +170,57 @@ class RiddlTest extends AbstractTestingBasis {
169170
indexIndent must be > ofIndent
170171
}
171172

173+
"preserve include structure in multi-file mode" in {
174+
val includePath = "language/input/includes/domainIncludes.riddl"
175+
val includeUrl = PathUtils.urlFromCwdPath(Path.of(includePath))
176+
implicit val ec: ExecutionContext = pc.ec
177+
val future = RiddlParserInput.fromURL(includeUrl).map { rpi =>
178+
Riddl.parse(rpi) match
179+
case Left(messages) => fail(messages.format)
180+
case Right(root) =>
181+
val input = PassInput(root)
182+
val outputs = PassesOutput()
183+
val options = PrettifyPass.Options(flatten = false)
184+
val result = Pass.runPass[PrettifyOutput](
185+
input, outputs,
186+
PrettifyPass(input, outputs, options)
187+
)
188+
val state = result.state
189+
// Multi-file mode should produce more than one file
190+
state.numFiles must be > 1
191+
// The main file should contain an include directive
192+
val mainContent = state.filesAsString
193+
mainContent must include("include")
194+
}
195+
Await.result(future, 10.seconds)
196+
}
197+
198+
"flatten includes in single-file mode" in {
199+
val includePath = "language/input/includes/domainIncludes.riddl"
200+
val includeUrl = PathUtils.urlFromCwdPath(Path.of(includePath))
201+
implicit val ec: ExecutionContext = pc.ec
202+
val future = RiddlParserInput.fromURL(includeUrl).map { rpi =>
203+
Riddl.parse(rpi) match
204+
case Left(messages) => fail(messages.format)
205+
case Right(root) =>
206+
val input = PassInput(root)
207+
val outputs = PassesOutput()
208+
val options = PrettifyPass.Options(flatten = true)
209+
val result = Pass.runPass[PrettifyOutput](
210+
input, outputs,
211+
PrettifyPass(input, outputs, options)
212+
)
213+
val state = result.state
214+
// Single-file mode should produce exactly one file
215+
state.numFiles must be(1)
216+
// The content should NOT contain include directives
217+
val content = state.filesAsString
218+
content must not include "include"
219+
// But should contain the included type definition
220+
content must include("foo")
221+
}
222+
Await.result(future, 10.seconds)
223+
}
224+
172225
}
173226
}

passes/jvm-native/src/test/scala/com/ossuminc/riddl/passes/VisitingPassTest.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class TestVisitor extends PassVisitor:
9393
def openOutput(output: Output, parents: Parents): Unit = incr(output)
9494
def openInput(input: Input, parents: Parents): Unit = incr(input)
9595
def openInclude(include: Include[?], parents: Parents): Unit = incr(include)
96+
def openBASTImport(bi: BASTImport, parents: Parents): Unit = incr(bi)
9697

9798
// Close for each type of container definition
9899
def closeType(typ: Type, parents: Parents): Unit = decr(typ)
@@ -113,6 +114,7 @@ class TestVisitor extends PassVisitor:
113114
def closeOutput(output: Output, parents: Parents): Unit = decr(output)
114115
def closeInput(input: Input, parents: Parents): Unit = decr(input)
115116
def closeInclude(include: Include[?], parents: Parents): Unit = decr(include)
117+
def closeBASTImport(bi: BASTImport, parents: Parents): Unit = decr(bi)
116118

117119
// LeafDefinitions
118120
def doField(field: Field): Unit = leaf(field)

passes/shared/src/main/scala/com/ossuminc/riddl/passes/Pass.scala

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,17 @@ abstract class HierarchyPass(
438438
* @param parents
439439
* The definition parents of the value
440440
*/
441+
/** Controls whether BASTImport contents are traversed.
442+
* Override in subclasses to suppress content traversal
443+
* (e.g., when emitting an import directive instead of
444+
* inlining the imported content).
445+
* @return true to traverse BASTImport contents, false to skip
446+
*/
447+
protected def traverseBASTImportContents(bi: BASTImport): Boolean = true
448+
449+
protected def openBASTImport(bi: BASTImport, parents: Parents): Unit = ()
450+
protected def closeBASTImport(bi: BASTImport, parents: Parents): Unit = ()
451+
441452
override protected def traverse(definition: RiddlValue, parents: ParentStack): Unit = {
442453
definition match {
443454
case include: Include[?] => // treat includes specially
@@ -447,6 +458,13 @@ abstract class HierarchyPass(
447458
// HierarchyPass always calls openInclude/closeInclude to give subclasses visibility
448459
include.contents.foreach { item => traverse(item, parents) }
449460
closeInclude(include, def_parents)
461+
case bastImport: BASTImport => // treat BASTImport like Include
462+
val def_parents = parents.toParents
463+
openBASTImport(bastImport, def_parents)
464+
if traverseBASTImportContents(bastImport) then
465+
bastImport.contents.foreach { item => traverse(item, parents) }
466+
end if
467+
closeBASTImport(bastImport, def_parents)
450468
case container: Branch[?] => // must be a container so descend
451469
val def_parents = parents.toParents // save this for the closeContainer below
452470
openContainer(container, def_parents)
@@ -488,6 +506,7 @@ trait PassVisitor:
488506
def openOutput(output: Output, parents: Parents): Unit
489507
def openInput(input: Input, parents: Parents): Unit
490508
def openInclude(include: Include[?], parents: Parents): Unit
509+
def openBASTImport(bi: BASTImport, parents: Parents): Unit
491510

492511
// Close for each type of container definition
493512
def closeType(typ: Type, parents: Parents): Unit
@@ -508,6 +527,7 @@ trait PassVisitor:
508527
def closeOutput(output: Output, parents: Parents): Unit
509528
def closeInput(input: Input, parents: Parents): Unit
510529
def closeInclude(include: Include[?], parents: Parents): Unit
530+
def closeBASTImport(bi: BASTImport, parents: Parents): Unit
511531

512532
// LeafDefinitions
513533
def doField(field: Field): Unit
@@ -653,6 +673,14 @@ abstract class VisitingPass[VT <: PassVisitor](
653673
override protected final def closeInclude(include: Include[?], parents: Parents): Unit =
654674
visitor.closeInclude(include, parents)
655675
end closeInclude
676+
677+
override protected final def openBASTImport(bi: BASTImport, parents: Parents): Unit =
678+
visitor.openBASTImport(bi, parents)
679+
end openBASTImport
680+
681+
override protected final def closeBASTImport(bi: BASTImport, parents: Parents): Unit =
682+
visitor.closeBASTImport(bi, parents)
683+
end closeBASTImport
656684
end VisitingPass
657685

658686
/** An abstract PassOutput for use with passes that derive from CollectingPass. This just provides a

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ object PrettifyPass extends PassInfo[PrettifyPass.Options]:
2424
(in: PassInput, out: PassesOutput) => PrettifyPass(in, out, options)
2525
end creator
2626

27-
case class Options(flatten: Boolean = false) extends PassOptions
27+
case class Options(
28+
flatten: Boolean = false,
29+
topFile: String = "",
30+
outputDir: String = ""
31+
) extends PassOptions
2832
end PrettifyPass
2933

3034
case class PrettifyOutput(
@@ -45,6 +49,12 @@ class PrettifyPass(
4549
extends VisitingPass[PrettifyVisitor](input, outputs, new PrettifyVisitor(options)):
4650
def name: String = PrettifyPass.name
4751

52+
/** In non-flatten mode, BASTImport contents are re-serialized
53+
* to BAST files separately, so we skip inline traversal.
54+
* In flatten mode, contents are inlined as RIDDL text.
55+
*/
56+
override protected def traverseBASTImportContents(bi: BASTImport): Boolean = options.flatten
57+
4858
// requires(SymbolsPass)
4959
// requires(ResolutionPass)
5060
// requires(ValidationPass)

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,19 @@
66

77
package com.ossuminc.riddl.passes.prettify
88

9+
import com.ossuminc.riddl.language.AST.BASTImport
910
import com.ossuminc.riddl.utils.{PlatformContext, URL}
1011

1112
import scala.collection.mutable
1213
import scala.scalajs.js.annotation.JSExport
1314

14-
case class PrettifyState(flatten: Boolean = false, topFile: String = "nada", outDir: String = "nada")(using PlatformContext):
15+
case class PrettifyState(flatten: Boolean = false, topFile: String = "prettify-output.riddl", outDir: String = ".")(using PlatformContext):
16+
17+
// Strip leading/trailing '/' from outDir for URL basis field, which
18+
// already gets '/' separators in URL format (scheme://authority/basis/path)
19+
private val normalizedOutDir: String =
20+
val stripped = if outDir.startsWith("/") then outDir.drop(1) else outDir
21+
if stripped.endsWith("/") then stripped.dropRight(1) else stripped
1522

1623
def filesAsString: String = {
1724
closeStack()
@@ -33,7 +40,7 @@ case class PrettifyState(flatten: Boolean = false, topFile: String = "nada", out
3340
}
3441

3542
def toDestination(url: URL): URL = {
36-
URL(url.scheme, url.authority, outDir, url.path)
43+
URL(url.scheme, url.authority, normalizedOutDir, url.path)
3744
}
3845
def numFiles: Int = files.length
3946

@@ -46,6 +53,11 @@ case class PrettifyState(flatten: Boolean = false, topFile: String = "nada", out
4653
private val fileStack: mutable.Stack[RiddlFileEmitter] = mutable.Stack
4754
.empty[RiddlFileEmitter]
4855

56+
private val bastImports: mutable.ListBuffer[BASTImport] = mutable.ListBuffer.empty
57+
58+
def addBASTImport(bi: BASTImport): Unit = bastImports.append(bi)
59+
def getBastImports: Seq[BASTImport] = bastImports.toSeq
60+
4961
private def closeStack(): Unit = { while fileStack.nonEmpty do popFile() }
5062

5163
// Get the ball rolling, this creates the first RFE and pushes it on to fileStack

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,13 @@ import com.ossuminc.riddl.utils.PlatformContext
1515
import scala.annotation.unused
1616

1717
class PrettifyVisitor(options: PrettifyPass.Options)(using PlatformContext) extends PassVisitor:
18-
val state: PrettifyState = PrettifyState(options.flatten)
18+
val state: PrettifyState = PrettifyState(
19+
options.flatten,
20+
if options.topFile.nonEmpty then options.topFile
21+
else "prettify-output.riddl",
22+
if options.outputDir.nonEmpty then options.outputDir
23+
else "."
24+
)
1925

2026
def result: PrettifyState = state
2127

@@ -408,7 +414,7 @@ class PrettifyVisitor(options: PrettifyPass.Options)(using PlatformContext) exte
408414
state.withCurrent { (rfe: RiddlFileEmitter) =>
409415
if !state.flatten then
410416
val url = include.origin
411-
rfe.addLine(s"include \"${url.toExternalForm}")
417+
rfe.addLine(s"""include "${url.toExternalForm}"""")
412418
val outputURL = state.toDestination(url)
413419
val newRFE = RiddlFileEmitter(outputURL)
414420
state.pushFile(newRFE)
@@ -420,6 +426,21 @@ class PrettifyVisitor(options: PrettifyPass.Options)(using PlatformContext) exte
420426
if !state.flatten then state.popFile()
421427
end if
422428
end closeInclude
429+
430+
def openBASTImport(bi: BASTImport, parents: Parents): Unit =
431+
state.withCurrent { (rfe: RiddlFileEmitter) =>
432+
if !state.flatten then
433+
// Emit the import directive in the current file
434+
// NOTE: "im" + "port" split to avoid ESM shim rewriting
435+
rfe.addLine("im" + "port " + "\"" + bi.path.s + "\"")
436+
// Track this BASTImport for re-serialization in writeOutput
437+
state.addBASTImport(bi)
438+
end if
439+
}
440+
end openBASTImport
441+
442+
def closeBASTImport(@unused bi: BASTImport, parents: Parents): Unit = ()
443+
end closeBASTImport
423444
end PrettifyVisitor
424445

425446
/** A function to translate between a definition and the keyword that introduces them.

0 commit comments

Comments
 (0)