Skip to content

Commit

Permalink
Implement support for property builders (#80)
Browse files Browse the repository at this point in the history 
* Implement support for property builders

* Spotless

* Small fixes

* Add test

* Spotless

* detekt

* Check builder not initialized in setters

* Spotless

* Add existing values if setters are present

* I hate detekt tbh

* Merge all properties

* Make the property nullable

* Spotless

* Required bangs

* Another bang
  • Loading branch information
@ZacSweers
ZacSweers authored Nov 16, 2023
1 parent a78e3fa commit 71d081a
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 20 deletions.
17 changes: 14 additions & 3 deletions src/main/kotlin/com/slack/auto/value/kotlin/AutoValueKotlinExtension.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,24 @@ private fun AvkBuilder.Companion.from(
propertyTypes: Map<String, TypeName>,
parseDocs: Element.() -> String?
): AvkBuilder {

// Merge the set of all properties with setters and builders
val allProperties = builderContext.setters().keys + builderContext.propertyBuilders().keys

// Setters
val props =
builderContext.setters().entries.map { (prop, setters) ->
allProperties.map { prop ->
val setters = builderContext.setters()[prop] ?: emptyList()
val propertyBuilder =
builderContext.propertyBuilders()[prop]?.let { propertyBuilder ->
FunSpec.copyOf(propertyBuilder).withDocsFrom(propertyBuilder, parseDocs).build()
}
val type = propertyTypes.getValue(prop)
BuilderProperty(
prop,
type,
setters.mapTo(LinkedHashSet()) { FunSpec.copyOf(it).withDocsFrom(it, parseDocs).build() }
setters.mapTo(LinkedHashSet()) { FunSpec.copyOf(it).withDocsFrom(it, parseDocs).build() },
propertyBuilder
)
}

Expand All @@ -466,11 +476,12 @@ private fun AvkBuilder.Companion.from(
builderMethods += builderContext.buildMethod().get()
}

// TODO propertyBuilders
val propertyBuilders = builderContext.propertyBuilders().values.toSet()

val remainingMethods =
ElementFilter.methodsIn(builderContext.builderType().enclosedElements)
.asSequence()
.filterNot { it in propertyBuilders }
.filterNot { it in builderMethods }
.filterNot { it == builderContext.autoBuildMethod() }
.map { "${it.modifiers.joinToString(" ")} ${it.returnType} ${it.simpleName}(...)" }
Expand Down
128 changes: 117 additions & 11 deletions src/main/kotlin/com/slack/auto/value/kotlin/AvkBuilder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.squareup.kotlinpoet.KModifier.INTERNAL
import com.squareup.kotlinpoet.KModifier.PRIVATE
import com.squareup.kotlinpoet.NOTHING
import com.squareup.kotlinpoet.ParameterSpec
import com.squareup.kotlinpoet.ParameterizedTypeName
import com.squareup.kotlinpoet.PropertySpec
import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec
Expand All @@ -42,7 +43,7 @@ public data class AvkBuilder(
val classAnnotations: List<AnnotationSpec>
) {

@Suppress("LongMethod")
@Suppress("LongMethod", "CyclomaticComplexMethod", "NestedBlockDepth")
public fun createType(messager: Messager): TypeSpec {
val builder =
TypeSpec.classBuilder(name).addModifiers(visibility).addAnnotations(classAnnotations)
Expand All @@ -58,7 +59,9 @@ public data class AvkBuilder(

val propsToCreateWith = mutableListOf<CodeBlock>()

for ((propName, type, setters) in builderProps) {
@Suppress("DestructuringDeclarationWithTooManyEntries")
for (builderProp in builderProps) {
val (propName, type, setters, propertyBuilder) = builderProp
// Add param to constructor
val defaultValue =
if (type.isNullable) {
Expand All @@ -82,6 +85,65 @@ public data class AvkBuilder(
.build()
builder.addProperty(propSpec)

if (propertyBuilder != null) {
val builderPropSpec =
PropertySpec.builder(
builderProp.builderPropName,
propertyBuilder.returnType.copy(nullable = true)
)
.addModifiers(PRIVATE)
.mutable()
.initializer("null")
.build()
builder.addProperty(builderPropSpec)

// Fill in the builder body
// Example:
// if (reactionsBuilder$ == null) {
// reactionsBuilder$ = ImmutableList.builder();
// }
// return reactionsBuilder$;
val rawType =
if (propSpec.type is ParameterizedTypeName) {
(propSpec.type as ParameterizedTypeName).rawType
} else {
propSpec.type
}
val nonNullType = rawType.copy(nullable = false)
val funSpec =
propertyBuilder
.toBuilder()
.beginControlFlow("if (%N == null)", builderPropSpec)
.apply {
addStatement("%N·= %T.builder()", builderPropSpec, nonNullType)
if (setters.isNotEmpty()) {
// Add the previous set value if one is present
// if (files == null) {
// filesBuilder$ = ImmutableList.builder();
// } else {
// filesBuilder$ = ImmutableList.builder();
// filesBuilder$.addAll(files);
// files = null;
// }
beginControlFlow("if (%N != null)", propSpec)
// TODO hacky but works for our cases
val addMethod =
if (type.toString().contains("Map")) {
"putAll"
} else {
"addAll"
}
addStatement("%N!!.$addMethod(%N)", builderPropSpec, propSpec)
addStatement("%N = null", propSpec)
endControlFlow()
}
}
.endControlFlow()
.addStatement("return·%N!!", builderPropSpec)
.build()
builder.addFunction(funSpec)
}

// Add build() assignment block
val extraCheck =
if (type.isNullable || !useNullablePropType) {
Expand All @@ -92,14 +154,34 @@ public data class AvkBuilder(
propsToCreateWith += CodeBlock.of("%1N·=·%1N%2L", propSpec, extraCheck)

for (setter in setters) {
// TODO if there's a builder, check the builder is null first
if (setter.parameters.size != 1) {
messager.printMessage(WARNING, "Setter with surprising params: ${setter.name}")
}
val setterBlock = CodeBlock.of("this.%N·= %N", propSpec, setter.parameters[0])
val setterSpec =
setter
.toBuilder()
// Assume this is a normal setter
.addStatement("return·apply·{·this.%N·= %N }", propSpec, setter.parameters[0])
.apply {
if (propertyBuilder != null) {
// Need to check if the builder is null
// if (reactionsBuilder$ != null) {
// throw new IllegalStateException("Cannot set reactions after calling
// reactionsBuilder()");
// }
beginControlFlow("check(%N == null)", builderProp.builderPropName)
addStatement(
"%S",
"Cannot set ${propSpec.name} after calling ${builderProp.builderPropName}()"
)
endControlFlow()
addStatement("%L", setterBlock)
addStatement("return·this")
} else {
// Assume this is a normal setter
addStatement("return·apply·{·%L }", setterBlock)
}
}
.build()
builder.addFunction(setterSpec)
}
Expand All @@ -113,11 +195,32 @@ public data class AvkBuilder(
builder.addFunction(
autoBuildFun
.toBuilder()
.addStatement(
"return·%T(%L)",
autoBuildFun.returnType!!,
propsToCreateWith.joinToCode(",·")
)
.apply {
// For all builder types, we need to init or assign them first
// Example:
// if (reactionsBuilder$ != null) {
// this.reactions = reactionsBuilder$.build();
// } else if (this.reactions == null) {
// this.reactions = ImmutableList.of();
// }
for (builderProp in builderProps) {
if (builderProp.builder != null) {
beginControlFlow("if (%N != null)", builderProp.builderPropName)
addStatement("this.%N = %N!!.build()", builderProp.name, builderProp.builderPropName)
// property builders can never be nullable
nextControlFlow("else if (this.%N == null)", builderProp.name)
val rawType =
if (builderProp.type is ParameterizedTypeName) {
builderProp.type.rawType
} else {
builderProp.type
}
addStatement("this.%N = %T.of()", builderProp.name, rawType)
endControlFlow()
}
}
}
.addStatement("return·%T(%L)", autoBuildFun.returnType, propsToCreateWith.joinToCode(",·"))
.build()
)

Expand Down Expand Up @@ -145,8 +248,11 @@ public data class AvkBuilder(
public data class BuilderProperty(
val name: String,
val type: TypeName,
val setters: Set<FunSpec>
)
val setters: Set<FunSpec>,
val builder: FunSpec?,
) {
val builderPropName: String = "${name}Builder"
}

// Public for extension
public companion object
Expand Down
6 changes: 3 additions & 3 deletions src/main/kotlin/com/slack/auto/value/kotlin/utils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ internal const val MAX_PARAMS = 7
internal val JSON_CN = Json::class.asClassName()
internal val JSON_CLASS_CN = JsonClass::class.asClassName()

@OptIn(DelicateKotlinPoetApi::class)
@ExperimentalAvkApi
public fun TypeMirror.asSafeTypeName(): TypeName {
return asTypeName().copy(nullable = false).normalize()
Expand Down Expand Up @@ -135,7 +136,7 @@ public fun TypeName.normalize(): TypeName {
@ExperimentalAvkApi
public fun TypeElement.classAnnotations(): List<AnnotationSpec> {
return annotationMirrors
.map { @Suppress("DEPRECATION") AnnotationSpec.get(it) }
.map { AnnotationSpec.get(it) }
.filterNot { (it.typeName as ClassName).packageName == "com.google.auto.value" }
.filterNot { (it.typeName as ClassName).simpleName == "Metadata" }
.map { spec ->
Expand Down Expand Up @@ -175,7 +176,7 @@ public fun deprecatedAnnotation(message: String, replaceWith: String): Annotatio
.build()
}

@Suppress("DEPRECATION", "SpreadOperator")
@Suppress("SpreadOperator")
@ExperimentalAvkApi
public fun FunSpec.Companion.copyOf(method: ExecutableElement): FunSpec.Builder {
var modifiers: Set<Modifier> = method.modifiers
Expand Down Expand Up @@ -216,7 +217,6 @@ public fun ParameterSpec.Companion.parametersWithNullabilityOf(
method: ExecutableElement
): List<ParameterSpec> = method.parameters.map(ParameterSpec.Companion::getWithNullability)

@Suppress("DEPRECATION")
@ExperimentalAvkApi
public fun ParameterSpec.Companion.getWithNullability(element: VariableElement): ParameterSpec {
val name = element.simpleName.toString()
Expand Down
53 changes: 50 additions & 3 deletions src/test/kotlin/com/slack/auto/value/kotlin/AutoValueKotlinExtensionTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class AutoValueKotlinExtensionTest {
import com.squareup.moshi.Json;
import com.squareup.moshi.JsonClass;
import java.util.List;
import com.google.common.collect.ImmutableList;
import org.jetbrains.annotations.Nullable;
@JsonClass(generateAdapter = true, generator = "avm")
Expand All @@ -76,6 +77,8 @@ class AutoValueKotlinExtensionTest {
@Nullable
abstract List<String> nullableCollection();
abstract ImmutableList<String> requiredBuildableCollection();
abstract boolean aBoolean();
abstract char aChar();
abstract byte aByte();
Expand Down Expand Up @@ -110,6 +113,8 @@ class AutoValueKotlinExtensionTest {
abstract Builder nullableValue(@Nullable String value);
abstract Builder collection(List<String> value);
abstract Builder nullableCollection(@Nullable List<String> value);
abstract Builder requiredBuildableCollection(ImmutableList<String> value);
abstract ImmutableList.Builder<String> requiredBuildableCollectionBuilder();
abstract Builder aBoolean(boolean value);
abstract Builder aChar(char value);
abstract Builder aByte(byte value);
Expand Down Expand Up @@ -140,6 +145,7 @@ class AutoValueKotlinExtensionTest {
package test
import android.os.Parcelable
import com.google.common.collect.ImmutableList
import com.slack.auto.`value`.kotlin.Redacted
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
Expand All @@ -160,6 +166,8 @@ class AutoValueKotlinExtensionTest {
val collection: List<String>,
@get:JvmName("nullableCollection")
val nullableCollection: List<String>? = null,
@get:JvmName("requiredBuildableCollection")
val requiredBuildableCollection: ImmutableList<String>,
@get:JvmName("aBoolean")
val aBoolean: Boolean = false,
@get:JvmName("aChar")
Expand Down Expand Up @@ -224,6 +232,17 @@ class AutoValueKotlinExtensionTest {
TODO("Remove this function. Use the above line to auto-migrate.")
}
@JvmSynthetic
@JvmName("-requiredBuildableCollection")
@Deprecated(
message = "Use the property",
replaceWith = ReplaceWith("requiredBuildableCollection"),
)
fun requiredBuildableCollection(): ImmutableList<String> {
requiredBuildableCollection()
TODO("Remove this function. Use the above line to auto-migrate.")
}
@JvmSynthetic
@JvmName("-aBoolean")
@Deprecated(
Expand Down Expand Up @@ -330,14 +349,15 @@ class AutoValueKotlinExtensionTest {
}
internal fun toBuilder(): Builder =
Builder(value = value, nullableValue = nullableValue, collection = collection, nullableCollection = nullableCollection, aBoolean = aBoolean, aChar = aChar, aByte = aByte, aShort = aShort, aInt = aInt, aFloat = aFloat, aLong = aLong, aDouble = aDouble, redactedString = redactedString)
Builder(value = value, nullableValue = nullableValue, collection = collection, nullableCollection = nullableCollection, requiredBuildableCollection = requiredBuildableCollection, aBoolean = aBoolean, aChar = aChar, aByte = aByte, aShort = aShort, aInt = aInt, aFloat = aFloat, aLong = aLong, aDouble = aDouble, redactedString = redactedString)
@Suppress("LongParameterList")
internal class Builder internal constructor(
private var `value`: String? = null,
private var nullableValue: String? = null,
private var collection: List<String>? = null,
private var nullableCollection: List<String>? = null,
private var requiredBuildableCollection: ImmutableList<String>? = null,
private var aBoolean: Boolean = false,
private var aChar: Char = 0.toChar(),
private var aByte: Byte = 0.toByte(),
Expand All @@ -348,6 +368,8 @@ class AutoValueKotlinExtensionTest {
private var aDouble: Double = 0.0,
private var redactedString: String? = null,
) {
private var requiredBuildableCollectionBuilder: ImmutableList.Builder<String>? = null
internal fun `value`(`value`: String): Builder = apply { this.`value` = `value` }
internal fun nullableValue(`value`: String?): Builder = apply { this.nullableValue = `value` }
Expand All @@ -357,6 +379,25 @@ class AutoValueKotlinExtensionTest {
internal fun nullableCollection(`value`: List<String>?): Builder =
apply { this.nullableCollection = `value` }
internal fun requiredBuildableCollectionBuilder(): ImmutableList.Builder<String> {
if (requiredBuildableCollectionBuilder == null) {
requiredBuildableCollectionBuilder = ImmutableList.builder()
if (requiredBuildableCollection != null) {
requiredBuildableCollectionBuilder!!.addAll(requiredBuildableCollection)
requiredBuildableCollection = null
}
}
return requiredBuildableCollectionBuilder!!
}
internal fun requiredBuildableCollection(`value`: ImmutableList<String>): Builder {
check(requiredBuildableCollectionBuilder == null) {
"Cannot set requiredBuildableCollection after calling requiredBuildableCollectionBuilder()"
}
this.requiredBuildableCollection = `value`
return this
}
internal fun aBoolean(`value`: Boolean): Builder = apply { this.aBoolean = `value` }
internal fun aChar(`value`: Char): Builder = apply { this.aChar = `value` }
Expand All @@ -375,8 +416,14 @@ class AutoValueKotlinExtensionTest {
internal fun redactedString(`value`: String): Builder = apply { this.redactedString = `value` }
internal fun build(): Example =
Example(`value` = `value` ?: error("value == null"), nullableValue = nullableValue, collection = collection ?: error("collection == null"), nullableCollection = nullableCollection, aBoolean = aBoolean, aChar = aChar, aByte = aByte, aShort = aShort, aInt = aInt, aFloat = aFloat, aLong = aLong, aDouble = aDouble, redactedString = redactedString ?: error("redactedString == null"))
internal fun build(): Example {
if (requiredBuildableCollectionBuilder != null) {
this.requiredBuildableCollection = requiredBuildableCollectionBuilder!!.build()
} else if (this.requiredBuildableCollection == null) {
this.requiredBuildableCollection = ImmutableList.of()
}
return Example(`value` = `value` ?: error("value == null"), nullableValue = nullableValue, collection = collection ?: error("collection == null"), nullableCollection = nullableCollection, requiredBuildableCollection = requiredBuildableCollection ?: error("requiredBuildableCollection == null"), aBoolean = aBoolean, aChar = aChar, aByte = aByte, aShort = aShort, aInt = aInt, aFloat = aFloat, aLong = aLong, aDouble = aDouble, redactedString = redactedString ?: error("redactedString == null"))
}
fun placeholder(): Nothing {
// TODO This is a placeholder to mention the following methods need to be moved manually over:
Expand Down

0 comments on commit 71d081a

Please sign in to comment.