Skip to content

Commit caa5693

Browse files
author
Fahad Zubair
committed
Constraints are written to the correct module
1 parent d238cee commit caa5693

File tree

3 files changed

+54
-18
lines changed

3 files changed

+54
-18
lines changed

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ class ConstraintViolationSymbolProvider(
107107
return RustModule.new(
108108
name = name,
109109
visibility = visibility,
110-
// FZ rebase
111-
//parent = ServerRustModule.Model,
112-
parent = ServerRustModule.Model,
110+
parent = module,
113111
inline = true,
114112
documentation = documentation,
115113
)

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCrateInlineModuleComposingWriter.kt

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ private data class InlineModuleWithWriter(val inlineModule : RustModule.LeafModu
131131
private val crateToInlineModule: ConcurrentHashMap<RustCrate, InnerModule> =
132132
ConcurrentHashMap()
133133

134-
class InnerModule(private val debugMode : Boolean) {
134+
class InnerModule(debugMode : Boolean) {
135135
private val topLevelModuleWriters: MutableSet<RustWriter> = mutableSetOf()
136136
private val inlineModuleWriters: HashMap<RustWriter, MutableList<InlineModuleWithWriter>> = hashMapOf()
137137
private val docWriters: HashMap<RustModule.LeafModule, MutableList<DocWriter>> = hashMapOf()
@@ -179,10 +179,10 @@ class InnerModule(private val debugMode : Boolean) {
179179
rustCrate.withModule(topMost) {
180180
var writer = this
181181
hierarchy.forEach {
182-
writer = getWriter(writer, it as RustModule.LeafModule)
182+
writer = getWriter(writer, it)
183183
}
184184

185-
withInlineModule(writer, bottomMost as RustModule.LeafModule, docWriter, writable)
185+
withInlineModule(writer, bottomMost, docWriter, writable)
186186
}
187187
} else {
188188
check(!bottomMost.isInline()) {
@@ -214,10 +214,10 @@ class InnerModule(private val debugMode : Boolean) {
214214
// Create an entry in the HashMap for all the descendent modules in the hierarchy.
215215
var writer = outerWriter
216216
hierarchy.forEach {
217-
writer = getWriter(writer, it as RustModule.LeafModule)
217+
writer = getWriter(writer, it)
218218
}
219219

220-
withInlineModule(writer, bottomMost as RustModule.LeafModule, docWriter, writable)
220+
withInlineModule(writer, bottomMost, docWriter, writable)
221221
}
222222

223223
/**
@@ -228,7 +228,7 @@ class InnerModule(private val debugMode : Boolean) {
228228
var hierarchy = listOf<RustModule.LeafModule>()
229229

230230
while (current is RustModule.LeafModule) {
231-
hierarchy = listOf(current as RustModule.LeafModule) + hierarchy
231+
hierarchy = listOf(current) + hierarchy
232232
current = current.parent
233233
}
234234

@@ -275,7 +275,6 @@ class InnerModule(private val debugMode : Boolean) {
275275
* has never been registered before then a new `RustWriter` is created and returned.
276276
*/
277277
private fun getWriter(outerWriter: RustWriter, inlineModule: RustModule.LeafModule): RustWriter {
278-
// Is this one of our inner writers?
279278
val nestedModuleWriter = inlineModuleWriters[outerWriter]
280279
if (nestedModuleWriter != null) {
281280
return findOrAddToList(nestedModuleWriter, inlineModule)
@@ -301,16 +300,42 @@ class InnerModule(private val debugMode : Boolean) {
301300
inlineModuleList: MutableList<InlineModuleWithWriter>,
302301
lookForModule: RustModule.LeafModule
303302
): RustWriter {
304-
val inlineModule = inlineModuleList.firstOrNull() {
305-
it.inlineModule == lookForModule
303+
val inlineModuleAndWriter = inlineModuleList.firstOrNull() {
304+
it.inlineModule.name == lookForModule.name
306305
}
307-
return if (inlineModule == null) {
306+
return if (inlineModuleAndWriter == null) {
308307
val inlineWriter = createNewInlineModule()
309308
inlineModuleList.add(InlineModuleWithWriter(lookForModule, inlineWriter))
310309
inlineWriter
311310
} else {
312-
inlineModule.writer
311+
check(inlineModuleAndWriter.inlineModule == lookForModule) {
312+
"the two inline modules have the same name but different attributes on them"
313+
}
314+
315+
inlineModuleAndWriter.writer
316+
}
317+
}
318+
319+
private fun combine(inlineModuleWithWriter: InlineModuleWithWriter, inlineModule: RustModule.LeafModule) : InlineModuleWithWriter {
320+
check(inlineModuleWithWriter.inlineModule.name == inlineModule.name) {
321+
"only inline module objects that have the same name can be combined"
322+
}
323+
check(inlineModuleWithWriter.inlineModule.rustMetadata.visibility == inlineModule.rustMetadata.visibility) {
324+
"only inline modules with same visibility can be combined"
313325
}
326+
check(inlineModuleWithWriter.inlineModule.inline && inlineModule.inline) {
327+
"both modules need to be inline to be combined together"
328+
}
329+
330+
val newModule = RustModule.new(
331+
name = inlineModule.name,
332+
parent = inlineModuleWithWriter.inlineModule.parent,
333+
documentation = inlineModuleWithWriter.inlineModule.documentation?.plus("\n")?.plus(inlineModule.documentation) ?: inlineModule.documentation,
334+
visibility = inlineModule.rustMetadata.visibility,
335+
additionalAttributes = inlineModuleWithWriter.inlineModule.rustMetadata.additionalAttributes + inlineModule.rustMetadata.additionalAttributes,
336+
inline = inlineModule.inline
337+
)
338+
return InlineModuleWithWriter(newModule, inlineModuleWithWriter.writer)
314339
}
315340

316341
private fun writeDocs(innerModule: RustModule.LeafModule) {

codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCrateInlineModuleComposingWriterTest.kt

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import software.amazon.smithy.rust.codegen.core.testutil.unitTest
1919
import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestCodegenContext
2020
import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestSymbolProvider
2121
import java.io.File
22-
import kotlin.collections.Map.Entry
2322

2423
class RustCrateInlineModuleComposingWriterTest {
2524
private val rustCrate: RustCrate
@@ -66,11 +65,11 @@ class RustCrateInlineModuleComposingWriterTest {
6665
rustCrate = RustCrate(context.fileManifest, codegenContext.symbolProvider, settings.codegenConfig)
6766
}
6867

69-
private fun createTestInlineModule(parentModule: RustModule, moduleName : String) : RustModule.LeafModule =
68+
private fun createTestInlineModule(parentModule: RustModule, moduleName : String, documentation : String? = null) : RustModule.LeafModule =
7069
RustModule.new(
7170
moduleName,
7271
visibility = Visibility.PUBLIC,
73-
documentation = moduleName,
72+
documentation = documentation ?: moduleName,
7473
parent = parentModule,
7574
inline = true,
7675
)
@@ -98,6 +97,13 @@ class RustCrateInlineModuleComposingWriterTest {
9897
}
9998
}
10099

100+
// private fun extraRustFun(writer: RustWriter, moduleName: String) {
101+
// writer.rustBlock("pub fn extra()") {
102+
// writer.comment("Module $moduleName")
103+
// writer.rust("""println!("extra function defined inside $moduleName");""")
104+
// }
105+
// }
106+
101107
@Test
102108
fun `simple inline module works`() {
103109
val testProject = TestWorkspace.testProject(serverTestSymbolProvider(model))
@@ -145,6 +151,8 @@ class RustCrateInlineModuleComposingWriterTest {
145151
modules["f"] = createTestInlineModule(ServerRustModule.Output, "f")
146152
modules["g"] = createTestInlineModule(modules["f"]!!, "g")
147153
modules["h"] = createTestInlineModule(ServerRustModule.Output, "h")
154+
// A different kotlin object but would still go in the right place
155+
// val moduleB = createTestInlineModule(ServerRustModule.Model, "b", "A new module")
148156

149157
testProject.withModule(ServerRustModule.Model) {
150158
testProject.getInlineModuleWriter().withInlineModule(this, modules["a"]!!) {
@@ -165,6 +173,10 @@ class RustCrateInlineModuleComposingWriterTest {
165173
testProject.getInlineModuleWriter().withInlineModule(this, modules["b"]!!) {
166174
byeWorld(this, "b")
167175
}
176+
177+
// testProject.getInlineModuleWriter().withInlineModule(this, moduleB) {
178+
// extraRustFun(this, "b")
179+
// }
168180
}
169181

170182
// Write directly to an inline module without specifying the immediate parent. crate::model::b::c
@@ -175,7 +187,7 @@ class RustCrateInlineModuleComposingWriterTest {
175187
}
176188
}
177189
// Write to a different top level module to confirm that works.
178-
testProject.withModule(ServerRustModule.Model) {
190+
testProject.withModule(ServerRustModule.Input) {
179191
testProject.getInlineModuleWriter().withInlineModuleHierarchy(this, modules["e"]!!) {
180192
helloWorld(this, "e")
181193
}
@@ -226,6 +238,7 @@ class RustCrateInlineModuleComposingWriterTest {
226238
this.unitTest("test_b") {
227239
rust("crate::model::b::hello_world();")
228240
rust("crate::model::b::bye_world();")
241+
// rust("crate::model::b::extra();")
229242
}
230243
this.unitTest("test_someother_writer_wrote") {
231244
rust("crate::model::b::some_other_writer_wrote_this();")

0 commit comments

Comments
 (0)