Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not generate public empty modules #1803

Merged
merged 8 commits into from
Oct 4, 2022
10 changes: 10 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@
# references = ["smithy-rs#920"]
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"

[[smithy-rs]]
message = """
Mark `operation` and `operation_handler` modules as private in the generated server crate.
Both modules did not contain any public types, therefore there should be no actual breakage when updating.
"""
references = ["smithy-rs#1803"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server"}
author = "LukeMathWalker"

[[smithy-rs]]
message = "Pokémon Service example code now runs clippy during build."
references = ["smithy-rs#1727"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.Cli
import software.amazon.smithy.rust.codegen.client.smithy.protocols.ClientProtocolLoader
import software.amazon.smithy.rust.codegen.client.smithy.transformers.AddErrorMessage
import software.amazon.smithy.rust.codegen.client.smithy.transformers.RemoveEventStreamOperations
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitorConfig
Expand Down Expand Up @@ -81,10 +82,19 @@ class CodegenVisitor(
symbolProvider = RustCodegenPlugin.baseSymbolProvider(model, service, symbolVisitorConfig)

codegenContext = ClientCodegenContext(model, symbolProvider, service, protocol, settings)

val clientPublicModules = setOf(
RustModule.Error,
RustModule.Model,
RustModule.Input,
RustModule.Output,
RustModule.Config,
RustModule.operation(Visibility.PUBLIC),
).associateBy { it.name }
rustCrate = RustCrate(
context.fileManifest,
symbolProvider,
DefaultPublicModules,
clientPublicModules,
codegenContext.settings.codegenConfig,
)
protocolGenerator = protocolGeneratorFactory.buildProtocolGenerator(codegenContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.GenericTypeArg
import software.amazon.smithy.rust.codegen.core.rustlang.RustGenerics
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
import software.amazon.smithy.rust.codegen.core.rustlang.asType
import software.amazon.smithy.rust.codegen.core.rustlang.docs
import software.amazon.smithy.rust.codegen.core.rustlang.rust
Expand All @@ -34,7 +35,7 @@ class CustomizableOperationGenerator(
private val smithyTypes = CargoDependency.SmithyTypes(runtimeConfig).asType()

fun render(crate: RustCrate) {
crate.withModule(RustModule.Operation) { writer ->
crate.withModule(RustModule.operation(Visibility.PUBLIC)) { writer ->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected this to be enough to get the module to show up in lib.rs as pub mod operation, but apparently it wasn't - I had to add https://github.com/awslabs/smithy-rs/pull/1803/files#diff-f7da198414ffe3cefc591164d3b1bb41d67e22812d7ba3a86b8abd6d798cec49R87
I'd be curious to understand why.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From reading the code, it definitely seems like it should emit the mod statement when it's missing from DefaultPublicModules.

writer.docs("Operation customization and supporting types")
writer.rust("pub mod customize;")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,18 @@ data class RustModule(val name: String, val rustMetadata: RustMetadata, val docu
fun private(name: String, documentation: String? = null): RustModule =
default(name, visibility = Visibility.PRIVATE, documentation = documentation)

/* Common modules used across client, server and tests */
val Config = public("config", documentation = "Configuration for the service.")
val Error = public("error", documentation = "All error types that operations can return.")
val Operation = public("operation", documentation = "All operations that this crate can perform.")
val Model = public("model", documentation = "Data structures used by operation inputs/outputs.")
val Input = public("input", documentation = "Input structures for operations.")
val Output = public("output", documentation = "Output structures for operations.")

/**
* Helper method to generate the `operation` Rust module.
* Its visibility depends on the generation context (client or server).
*/
fun operation(visibility: Visibility): RustModule =
default("operation", visibility = visibility, documentation = "All operations that this crate can perform.")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,6 @@ open class RustCrate(
}
}

/**
* Allowlist of modules that will be exposed publicly in generated crates
*/
val DefaultPublicModules = setOf(
RustModule.Error,
RustModule.Operation,
RustModule.public("model", documentation = "Data structures used by operation inputs/outputs."),
RustModule.public("input", documentation = "Input structures for operations."),
RustModule.public("output", documentation = "Output structures for operations."),
RustModule.Config,
).associateBy { it.name }

/**
* Finalize all the writers by:
* - inlining inline dependencies that have been used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.EnumDefinition
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
import software.amazon.smithy.rust.codegen.core.rustlang.RustDependency
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.raw
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.smithy.CoreCodegenConfig
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.smithy.MaybeRenamed
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
Expand Down Expand Up @@ -189,6 +189,14 @@ fun RustWriter.unitTest(
return rustBlock("fn $name()", *args, block = block)
}

val DefaultTestPublicModules = setOf(
RustModule.Error,
RustModule.Model,
RustModule.Input,
RustModule.Output,
RustModule.Config,
).associateBy { it.name }

/**
* WriterDelegator used for test purposes
*
Expand All @@ -199,7 +207,12 @@ class TestWriterDelegator(
symbolProvider: RustSymbolProvider,
val codegenConfig: CoreCodegenConfig,
) :
RustCrate(fileManifest, symbolProvider, DefaultPublicModules, codegenConfig) {
RustCrate(
fileManifest,
symbolProvider,
DefaultTestPublicModules,
codegenConfig,
) {
val baseDir: Path = fileManifest.baseDir

fun generatedFiles(): List<Path> = fileManifest.files.toList().sorted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.CoreRustSettings
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
import software.amazon.smithy.rust.codegen.core.testutil.DefaultTestPublicModules
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.generatePluginContext
import software.amazon.smithy.rust.codegen.core.testutil.testSymbolProvider
Expand Down Expand Up @@ -75,7 +75,7 @@ internal class TopLevelErrorGeneratorTest {
val rustCrate = RustCrate(
pluginContext.fileManifest,
symbolProvider,
DefaultPublicModules,
DefaultTestPublicModules,
codegenContext.settings.codegenConfig,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.rust.codegen.client.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitorConfig
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
Expand All @@ -25,6 +24,7 @@ import software.amazon.smithy.rust.codegen.core.util.getTrait
import software.amazon.smithy.rust.codegen.server.python.smithy.generators.PythonServerEnumGenerator
import software.amazon.smithy.rust.codegen.server.python.smithy.generators.PythonServerServiceGenerator
import software.amazon.smithy.rust.codegen.server.python.smithy.generators.PythonServerStructureGenerator
import software.amazon.smithy.rust.codegen.server.smithy.DefaultServerPublicModules
import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext
import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenVisitor
import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.ServerProtocol
Expand Down Expand Up @@ -69,7 +69,7 @@ class PythonServerCodegenVisitor(
codegenContext = ServerCodegenContext(model, symbolProvider, service, protocol, settings)

// Override `rustCrate` which carries the symbolProvider.
rustCrate = RustCrate(context.fileManifest, symbolProvider, DefaultPublicModules, settings.codegenConfig)
rustCrate = RustCrate(context.fileManifest, symbolProvider, DefaultServerPublicModules, settings.codegenConfig)
// Override `protocolGenerator` which carries the symbolProvider.
protocolGenerator = protocolGeneratorFactory.buildProtocolGenerator(codegenContext)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.rust.codegen.client.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.CoreRustSettings
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitorConfig
Expand All @@ -42,6 +42,14 @@ import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.Ser
import software.amazon.smithy.rust.codegen.server.smithy.protocols.ServerProtocolLoader
import java.util.logging.Logger

val DefaultServerPublicModules = setOf(
RustModule.Error,
RustModule.Model,
RustModule.Input,
RustModule.Output,
RustModule.Config,
).associateBy { it.name }

/**
* Entrypoint for server-side code generation. This class will walk the in-memory model and
* generate all the needed types by calling the accept() function on the available shapes.
Expand Down Expand Up @@ -92,7 +100,7 @@ open class ServerCodegenVisitor(
settings,
)

rustCrate = RustCrate(context.fileManifest, symbolProvider, DefaultPublicModules, settings.codegenConfig)
rustCrate = RustCrate(context.fileManifest, symbolProvider, DefaultServerPublicModules, settings.codegenConfig)
protocolGenerator = protocolGeneratorFactory.buildProtocolGenerator(codegenContext)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.Visibility
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.DefaultPublicModules
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.generators.protocol.ProtocolSupport
import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.ServerProtocol
Expand Down Expand Up @@ -41,7 +40,7 @@ open class ServerServiceGenerator(
* which assigns a symbol location to each shape.
*/
fun render() {
rustCrate.withModule(DefaultPublicModules["operation"]!!) { writer ->
rustCrate.withModule(RustModule.operation(Visibility.PRIVATE)) { writer ->
ServerProtocolTestGenerator(codegenContext, protocolSupport, protocolGenerator).render(writer)
}

Expand All @@ -52,7 +51,7 @@ open class ServerServiceGenerator(
}
}
}
rustCrate.withModule(RustModule.public("operation_handler", "Operation handlers definition and implementation.")) { writer ->
rustCrate.withModule(RustModule.private("operation_handler", "Operation handlers definition and implementation.")) { writer ->
renderOperationHandler(writer, operations)
}
rustCrate.withModule(
Expand Down