-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: support Smithy default
trait
#857
Changes from all commits
0f155c2
51f8dbb
1aff46c
4aebfe7
528a21d
24f6c67
292dc53
1284dd2
de93d87
fc2ff86
10cb7b0
345b411
09719fe
2f84ed7
6b5e7e4
cf9d353
e66c579
42ad105
ccff67e
84f69c7
f419475
eb084e9
781ac4f
ea1fbc1
3a638e1
939ce26
4a3afb5
8a51691
defa0ce
aee25a0
42bbd66
367d8d5
80e2ce8
f8ccaf9
844b970
137a013
1988148
c0ca0ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"id": "d3ec877c-68c6-4c21-881f-8767d1f87e1c", | ||
"type": "feature", | ||
"description": "Support Smithy default trait", | ||
"issues": [ | ||
"https://github.com/awslabs/smithy-kotlin/issues/718" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,11 @@ import software.amazon.smithy.codegen.core.* | |
import software.amazon.smithy.kotlin.codegen.KotlinSettings | ||
import software.amazon.smithy.kotlin.codegen.lang.kotlinReservedWords | ||
import software.amazon.smithy.kotlin.codegen.model.* | ||
import software.amazon.smithy.kotlin.codegen.utils.dq | ||
import software.amazon.smithy.model.Model | ||
import software.amazon.smithy.model.knowledge.NullableIndex | ||
import software.amazon.smithy.model.shapes.* | ||
import software.amazon.smithy.model.traits.DefaultTrait | ||
import software.amazon.smithy.model.traits.SparseTrait | ||
import software.amazon.smithy.model.traits.StreamingTrait | ||
import java.util.logging.Logger | ||
|
@@ -75,31 +77,29 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
|
||
override fun longShape(shape: LongShape): Symbol = numberShape(shape, "Long", "0L") | ||
|
||
override fun floatShape(shape: FloatShape): Symbol = numberShape(shape, "Float", "0.0f") | ||
override fun floatShape(shape: FloatShape): Symbol = numberShape(shape, "Float", "0f") | ||
|
||
override fun doubleShape(shape: DoubleShape): Symbol = numberShape(shape, "Double", "0.0") | ||
|
||
private fun numberShape(shape: Shape, typeName: String, defaultValue: String = "0"): Symbol = | ||
createSymbolBuilder(shape, typeName, namespace = "kotlin") | ||
.defaultValue(defaultValue) | ||
.build() | ||
createSymbolBuilder(shape, typeName, namespace = "kotlin").defaultValue(defaultValue).build() | ||
|
||
override fun bigIntegerShape(shape: BigIntegerShape?): Symbol = createBigSymbol(shape, "BigInteger") | ||
|
||
override fun bigDecimalShape(shape: BigDecimalShape?): Symbol = createBigSymbol(shape, "BigDecimal") | ||
|
||
private fun createBigSymbol(shape: Shape?, symbolName: String): Symbol = | ||
createSymbolBuilder(shape, symbolName, namespace = "java.math", boxed = true).build() | ||
createSymbolBuilder(shape, symbolName, namespace = "java.math", nullable = true).build() | ||
|
||
override fun stringShape(shape: StringShape): Symbol = if (shape.isEnum) { | ||
createEnumSymbol(shape) | ||
} else { | ||
createSymbolBuilder(shape, "String", boxed = true, namespace = "kotlin").build() | ||
createSymbolBuilder(shape, "String", nullable = true, namespace = "kotlin").build() | ||
} | ||
|
||
private fun createEnumSymbol(shape: Shape): Symbol { | ||
val namespace = "$rootNamespace.model" | ||
return createSymbolBuilder(shape, shape.defaultName(service), namespace, boxed = true) | ||
return createSymbolBuilder(shape, shape.defaultName(service), namespace, nullable = true) | ||
.definitionFile("${shape.defaultName(service)}.kt") | ||
.build() | ||
} | ||
|
@@ -110,7 +110,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
override fun structureShape(shape: StructureShape): Symbol { | ||
val name = shape.defaultName(service) | ||
val namespace = "$rootNamespace.model" | ||
val builder = createSymbolBuilder(shape, name, namespace, boxed = true) | ||
val builder = createSymbolBuilder(shape, name, namespace, nullable = true) | ||
.definitionFile("$name.kt") | ||
|
||
// add a reference to each member symbol | ||
|
@@ -149,7 +149,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
override fun listShape(shape: ListShape): Symbol { | ||
val reference = toSymbol(shape.member) | ||
val valueType = if (shape.hasTrait<SparseTrait>()) "${reference.name}?" else reference.name | ||
return createSymbolBuilder(shape, "List<$valueType>", boxed = true) | ||
return createSymbolBuilder(shape, "List<$valueType>", nullable = true) | ||
.addReferences(reference) | ||
.putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableListOf<$valueType>") | ||
.putProperty(SymbolProperty.IMMUTABLE_COLLECTION_FUNCTION, "listOf<$valueType>") | ||
|
@@ -160,7 +160,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
val reference = toSymbol(shape.value) | ||
val valueType = if (shape.hasTrait<SparseTrait>()) "${reference.name}?" else reference.name | ||
|
||
return createSymbolBuilder(shape, "Map<String, $valueType>", boxed = true) | ||
return createSymbolBuilder(shape, "Map<String, $valueType>", nullable = true) | ||
.addReferences(reference) | ||
.putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableMapOf<String, $valueType>") | ||
.putProperty(SymbolProperty.IMMUTABLE_COLLECTION_FUNCTION, "mapOf<String, $valueType>") | ||
|
@@ -172,11 +172,17 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
val targetShape = | ||
model.getShape(shape.target).orElseThrow { CodegenException("Shape not found: ${shape.target}") } | ||
|
||
val targetSymbol = if (nullableIndex.isMemberNullable(shape, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT)) { | ||
toSymbol(targetShape).toBuilder().boxed().build() | ||
} else { | ||
toSymbol(targetShape) | ||
} | ||
val targetSymbol = toSymbol(targetShape) | ||
.toBuilder() | ||
.apply { | ||
if (nullableIndex.isMemberNullable(shape, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT)) nullable() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given kotlin SDK is not GA, could And note, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're stuck using Internal reference: P83747977 We can hold off on merging this PR until it's resolved internally, and then I can update to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. got it. if this needs to be merged sooner for some reason, as long as there's tracking / TODO somewhere to change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I spoke with the team, we'll merge the PR now and add a tracking item to update to |
||
|
||
shape.getTrait<DefaultTrait>()?.let { | ||
defaultValue(it.getDefaultValue(targetShape), DefaultValueType.MODELED) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so if a member has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
.build() | ||
|
||
// figure out if we are referencing an event stream or not. | ||
// NOTE: unlike blob streams we actually re-use the target (union) shape which is why we can't do this | ||
// when visiting a unionShape() like we can for blobShape() | ||
|
@@ -197,9 +203,18 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
} | ||
} | ||
|
||
private fun DefaultTrait.getDefaultValue(targetShape: Shape): String? = when { | ||
toNode().toString() == "null" || targetShape is BlobShape && toNode().toString() == "" -> null | ||
toNode().isNumberNode -> getDefaultValueForNumber(targetShape, toNode().toString()) | ||
toNode().isArrayNode -> "listOf()" | ||
toNode().isObjectNode -> "mapOf()" | ||
toNode().isStringNode -> toNode().toString().dq() | ||
else -> toNode().toString() | ||
} | ||
|
||
override fun timestampShape(shape: TimestampShape?): Symbol { | ||
val dependency = KotlinDependency.CORE | ||
return createSymbolBuilder(shape, "Instant", boxed = true) | ||
return createSymbolBuilder(shape, "Instant", nullable = true) | ||
.namespace("${dependency.namespace}.time", ".") | ||
.addDependency(dependency) | ||
.build() | ||
|
@@ -208,7 +223,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
override fun blobShape(shape: BlobShape): Symbol = if (shape.hasTrait<StreamingTrait>()) { | ||
RuntimeTypes.Core.Content.ByteStream.asNullable() | ||
} else { | ||
createSymbolBuilder(shape, "ByteArray", boxed = true, namespace = "kotlin").build() | ||
createSymbolBuilder(shape, "ByteArray", nullable = true, namespace = "kotlin").build() | ||
} | ||
|
||
override fun documentShape(shape: DocumentShape?): Symbol = | ||
|
@@ -217,7 +232,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
override fun unionShape(shape: UnionShape): Symbol { | ||
val name = shape.defaultName(service) | ||
val namespace = "$rootNamespace.model" | ||
val builder = createSymbolBuilder(shape, name, namespace, boxed = true) | ||
val builder = createSymbolBuilder(shape, name, namespace, nullable = true) | ||
.definitionFile("$name.kt") | ||
|
||
// add a reference to each member symbol | ||
|
@@ -243,16 +258,23 @@ 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, boxed: Boolean = false): Symbol.Builder { | ||
private fun createSymbolBuilder(shape: Shape?, typeName: String, nullable: Boolean = false): Symbol.Builder { | ||
val builder = Symbol.builder() | ||
.putProperty(SymbolProperty.SHAPE_KEY, shape) | ||
.name(typeName) | ||
if (boxed) { | ||
builder.boxed() | ||
if (nullable) { | ||
builder.nullable() | ||
} | ||
return builder | ||
} | ||
|
||
private fun getDefaultValueForNumber(shape: Shape, value: String) = when (shape) { | ||
is LongShape -> "${value}L" | ||
is FloatShape -> "${value}f" | ||
is DoubleShape -> if (value.matches("[0-9]*\\.[0-9]+".toRegex())) value else "$value.0" | ||
else -> value | ||
} | ||
|
||
/** | ||
* Creates a symbol builder for the shape with the given type name in a child namespace relative | ||
* to the root namespace e.g. `relativeNamespace = bar` with a root namespace of `foo` would set | ||
|
@@ -262,8 +284,8 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli | |
shape: Shape?, | ||
typeName: String, | ||
namespace: String, | ||
boxed: Boolean = false, | ||
): Symbol.Builder = createSymbolBuilder(shape, typeName, boxed).namespace(namespace, ".") | ||
nullable: Boolean = false, | ||
): Symbol.Builder = createSymbolBuilder(shape, typeName, nullable).namespace(namespace, ".") | ||
} | ||
|
||
// Add a reference and it's children | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,11 @@ object SymbolProperty { | |
// The key that holds the default value for a type (symbol) as a string | ||
const val DEFAULT_VALUE_KEY: String = "defaultValue" | ||
|
||
// Boolean property indicating this symbol should be boxed | ||
const val BOXED_KEY: String = "boxed" | ||
// The key that holds the type of default value | ||
const val DEFAULT_VALUE_TYPE_KEY: String = "defaultValueType" | ||
|
||
// Boolean property indicating this symbol is nullable | ||
const val NULLABLE_KEY: String = "nullable" | ||
|
||
// the original shape the symbol was created from | ||
const val SHAPE_KEY: String = "shape" | ||
|
@@ -47,21 +50,21 @@ object SymbolProperty { | |
} | ||
|
||
/** | ||
* Test if a symbol is boxed | ||
* Test if a symbol is nullable | ||
*/ | ||
val Symbol.isBoxed: Boolean | ||
get() = getProperty(SymbolProperty.BOXED_KEY).map { | ||
val Symbol.isNullable: Boolean | ||
get() = getProperty(SymbolProperty.NULLABLE_KEY).map { | ||
when (it) { | ||
is Boolean -> it | ||
else -> false | ||
} | ||
}.orElse(false) | ||
|
||
/** | ||
* Test if a symbol is not boxed | ||
* Test if a symbol is not nullable | ||
*/ | ||
val Symbol.isNotBoxed: Boolean | ||
get() = !isBoxed | ||
val Symbol.isNotNullable: Boolean | ||
get() = !isNullable | ||
|
||
enum class PropertyTypeMutability { | ||
/** | ||
|
@@ -82,6 +85,21 @@ enum class PropertyTypeMutability { | |
} | ||
} | ||
|
||
enum class DefaultValueType { | ||
/** | ||
* A default value which has been inferred, such as 0f for floats and false for booleans | ||
*/ | ||
INFERRED, | ||
|
||
/** | ||
* A default value which has been modeled using Smithy's default trait. | ||
*/ | ||
MODELED, | ||
} | ||
|
||
val Symbol.defaultValueType: DefaultValueType? | ||
get() = getProperty(SymbolProperty.DEFAULT_VALUE_TYPE_KEY, DefaultValueType::class.java).getOrNull() | ||
|
||
/** | ||
* Get the property type mutability of this symbol if set. | ||
*/ | ||
|
@@ -92,27 +110,30 @@ val Symbol.propertyTypeMutability: PropertyTypeMutability? | |
|
||
/** | ||
* Gets the default value for the symbol if present, else null | ||
* @param defaultBoxed the string to pass back for boxed values | ||
* @param defaultNullable the string to pass back for nullable values | ||
*/ | ||
fun Symbol.defaultValue(defaultBoxed: String? = "null"): String? { | ||
// boxed types should always be defaulted to null | ||
if (isBoxed) { | ||
return defaultBoxed | ||
} | ||
|
||
fun Symbol.defaultValue(defaultNullable: String? = "null"): String? { | ||
val default = getProperty(SymbolProperty.DEFAULT_VALUE_KEY, String::class.java) | ||
return if (default.isPresent) default.get() else null | ||
|
||
// nullable types should default to null if there is no modeled default | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I probably don't understand how this all fits, but why is isNullable not sufficient? Seems simpler if it ended up like that. What test case would break without the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason for the With public val foo: String? = "my modeled default value" Without public val foo: String? = null // (discards the modeled default value) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought if something has a non-null default value, it won't be nullable. Atleast when it uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, but they can be nullable because of |
||
if (isNullable && (!default.isPresent || defaultValueType == DefaultValueType.INFERRED)) { | ||
return defaultNullable | ||
} | ||
return default.getOrNull() | ||
} | ||
|
||
/** | ||
* Mark a symbol as being boxed (nullable) i.e. `T?` | ||
* Mark a symbol as being nullable (i.e. `T?`) | ||
*/ | ||
fun Symbol.Builder.boxed(): Symbol.Builder = apply { putProperty(SymbolProperty.BOXED_KEY, true) } | ||
fun Symbol.Builder.nullable(): Symbol.Builder = apply { putProperty(SymbolProperty.NULLABLE_KEY, true) } | ||
|
||
/** | ||
* Set the default value used when formatting the symbol | ||
*/ | ||
fun Symbol.Builder.defaultValue(value: String): Symbol.Builder = apply { putProperty(SymbolProperty.DEFAULT_VALUE_KEY, value) } | ||
fun Symbol.Builder.defaultValue(value: String?, type: DefaultValueType = DefaultValueType.INFERRED): Symbol.Builder = apply { | ||
putProperty(SymbolProperty.DEFAULT_VALUE_KEY, value) | ||
putProperty(SymbolProperty.DEFAULT_VALUE_TYPE_KEY, type) | ||
} | ||
|
||
/** | ||
* Convenience function for specifying kotlin namespace | ||
|
@@ -177,7 +198,7 @@ val Symbol.shape: Shape? | |
/** | ||
* Get the nullable version of a symbol | ||
*/ | ||
fun Symbol.asNullable(): Symbol = toBuilder().boxed().build() | ||
fun Symbol.asNullable(): Symbol = toBuilder().nullable().build() | ||
|
||
/** | ||
* Check whether a symbol represents an extension | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: so all number shapes having a INFERRED default value means the shape knows what the value would be, but the shape can be
nullable
too, and it's thenullable
check that'll decide how the code is generated, and if it is not nullable, only then that defaultValue is relevant? Trying to understand the motivation for maintaining INFERRED v/s MODELED.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The motivation behind
INFERRED
/MODELED
is because we're using.defaultValue()
in two ways now. The previous way is inferring that numbers and booleans have a default value (0, 0f, 0.0, false). The new way we use.defaultValue()
is when it's modeled on a shape.If a shape has an
INFERRED
default value but it's nullable, we set it to null. If a shape has aMODELED
default value but it's nullable, we will use the default value.