Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
aajtodd committed Oct 19, 2023
1 parent d05f471 commit 1b7920f
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package software.amazon.smithy.kotlin.codegen

import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.kotlin.codegen.lang.isValidPackageName
import software.amazon.smithy.kotlin.codegen.utils.getOrNull
import software.amazon.smithy.kotlin.codegen.utils.toCamelCase
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.NullableIndex.CheckMode
Expand Down Expand Up @@ -239,12 +240,12 @@ val CheckMode.kotlinPluginSetting: String

enum class DefaultValueSerializationMode(val value: String) {
/**
* Always serialize default values even if they are set to the default
* Always serialize values even if they are set to the modeled default
*/
ALWAYS("always"),

/**
* Only serialize default values when they differ from the default given in the model.
* Only serialize values when they differ from the modeled default or are marked `@required`
*/
WHEN_DIFFERENT("whenDifferent"),
;
Expand Down Expand Up @@ -274,8 +275,14 @@ data class ApiSettings(
const val DEFAULT_VALUE_SERIALIZATION_MODE = "defaultValueSerializationMode"

fun fromNode(node: Optional<ObjectNode>): ApiSettings = node.map {
val visibility = Visibility.fromValue(node.get().getStringMemberOrDefault(VISIBILITY, "public"))
val checkMode = checkModefromValue(node.get().getStringMemberOrDefault(NULLABILITY_CHECK_MODE, "clientCareful"))
val visibility = node.get()
.getStringMember(VISIBILITY)
.map { Visibility.fromValue(it.value) }
.getOrNull() ?: Visibility.PUBLIC
val checkMode = node.get()
.getStringMember(NULLABILITY_CHECK_MODE)
.map { checkModefromValue(it.value) }
.getOrNull() ?: CheckMode.CLIENT_CAREFUL
val defaultValueSerializationMode = DefaultValueSerializationMode.fromValue(
node.get()
.getStringMemberOrDefault(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
.toBuilder()
.apply {
val isNullable = nullableIndex.isMemberNullable(shape, settings.api.nullabilityCheckMode)
if (isNullable) nullable()

// only use @default if type is `T`
if (!isNullable) {
if (isNullable) {
nullable()
} else {
// only use @default if type is `T`
shape.getTrait<DefaultTrait>()?.let {
defaultValue(it.getDefaultValue(targetShape))
}
Expand Down Expand Up @@ -286,15 +286,10 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
/**
* Creates a symbol builder for the shape with the given type name in the root namespace.
*/
private fun createSymbolBuilder(shape: Shape?, typeName: String, nullable: Boolean = false): Symbol.Builder {
val builder = Symbol.builder()
private fun createSymbolBuilder(shape: Shape?, typeName: String): Symbol.Builder =
Symbol.builder()
.putProperty(SymbolProperty.SHAPE_KEY, shape)
.name(typeName)
if (nullable) {
builder.nullable()
}
return builder
}

private fun getDefaultValueForNumber(type: ShapeType, value: String) = when (type) {
ShapeType.LONG -> "${value}L"
Expand All @@ -314,8 +309,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
shape: Shape?,
typeName: String,
namespace: String,
nullable: Boolean = false,
): Symbol.Builder = createSymbolBuilder(shape, typeName, nullable).namespace(namespace, ".")
): Symbol.Builder = createSymbolBuilder(shape, typeName).namespace(namespace, ".")
}

// Add a reference and it's children
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class StructureGenerator(
val prefix = if (shape.isError && memberName == "message") {
val targetShape = model.expectShape(memberShape.target)
if (!targetShape.isStringShape) {
throw CodegenException("Message is a reserved name for exception types and cannot be used for any other property")
throw CodegenException("message is a reserved name for exception types and cannot be used for any other property")
}
"override"
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ open class SerializeStructGenerator(
""
}
val fn = serializerFn.format(memberShape, "input.$memberName")
writer.write("${defaultCheck}${fn}$postfix")
writer.write("$defaultCheck$fn$postfix")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class ExceptionGeneratorTest {
val e = assertFailsWith<CodegenException> {
StructureGenerator(renderingCtx).render()
}
e.message.shouldContainOnlyOnceWithDiff("Message is a reserved name for exception types and cannot be used for any other property")
e.message.shouldContainOnlyOnceWithDiff("message is a reserved name for exception types and cannot be used for any other property")
}

class BaseExceptionGeneratorTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ internal class SmokeTestOperationDeserializer: HttpDeserialize<SmokeTestResponse
}

@Test
fun itValidatesRequiredAndNonBlankURIBindings() {
fun itValidatesRequiredAndNonBlankUriBindings() {
val model = """
@http(method: "POST", uri: "/foo/{bar}/{baz}")
operation Foo {
Expand Down
2 changes: 0 additions & 2 deletions tests/codegen/nullability-tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ val generateSdk = tasks.register<SmithyBuild>("generateSdk") {
classpath = codegen
inputs.file(projectDir.resolve("smithy-build.json"))
inputs.files(codegen)
doLast {
}
}

tasks.named<SmithyValidate>("smithyValidate") {
Expand Down

0 comments on commit 1b7920f

Please sign in to comment.