open-toast · andrewparmet · Jun 8, 2024 · Jun 8, 2024 · Jun 8, 2024 · Jun 8, 2024
diff --git a/README.md b/README.md
@@ -12,10 +12,7 @@ Supports only version 3 of the Protocol Buffers language.
 
 ### Features
 - Idiomatic and concise [Kotlin builder DSL](#generated-code)
-- Protokt-specific options: [non-null types (dangerous)](#non-null-fields),
-[wrapper types](#wrapper-types),
-[interface implementation](#interface-implementation),
-and more
+- Protokt-specific options: [wrapper types](#wrapper-types), [interface implementation](#interface-implementation), and more
 - Representation of the well-known types as Kotlin nullable types: `StringValue`
 is represented as `String?`, etc.
 - Multiplatform support for Kotlin JS (beta)
@@ -635,8 +632,7 @@ dependencies {
 ```
 
 Wrapper types that wrap protobuf messages are nullable. For example,
-`java.time.Instant` wraps the well-known type `google.protobuf.Timestamp`. They
-can be made non-nullable by using the non-null option described below.
+`java.time.Instant` wraps the well-known type `google.protobuf.Timestamp`.
 
 Wrapper types that wrap protobuf primitives, for example `java.util.UUID`
 which wraps `bytes`, are nullable when they cannot wrap their wrapped type's 
@@ -658,8 +654,6 @@ google.protobuf.BytesValue nullable_uuid = 3 [
 ];
 ```
 
-This behavior can be overridden with the [`non_null` option](#non-null-fields).
-
 Wrapper types can be repeated:
 
 ```protobuf
@@ -686,41 +680,6 @@ _N.b. Well-known type nullability is implemented with
 for each message defined in
 [wrappers.proto](https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/wrappers.proto)._
 
-### Non-null fields
-If a message has no meaning whatsoever when a particular non-scalar field is
-missing, you can emulate proto2's `required` key word by using the
-`(protokt.v1.property).non_null` option:
-
-```protobuf
-message Sample {}
-
-message NonNullSampleMessage {
-  Sample non_null_sample = 1 [
-    (protokt.v1.property).non_null = true
-  ];
-}
-```
-
-Generated code will not have a nullable type, so the field can be referenced
-without using Kotlin's `!!`.
-
-Oneof fields can also be declared non-null:
-
-```protobuf
-message NonNullSampleMessage {
-  oneof non_null_oneof {
-    option (protokt.v1.oneof).non_null = true;
-
-    string message = 1;
-  }
-}
-```
-
-Note that deserialization of a message with a non-nullable field will fail if the
-message being decoded does not contain an instance of the required field.
-
-This functionality will likely be removed.
-
 ### Interface implementation
 
 #### Messages
@@ -759,21 +718,19 @@ dependencies {
 }
 ```
 
-Messages can also implement interfaces by delegation to one of their fields;
-in this case the delegated interface need not live in a separate project, as
-protokt requires no inspection of it:
+Messages can also implement interfaces by delegation to one of their fields:
 
 ```protobuf
 message ImplementsWithDelegate {
   option (protokt.v1.class).implements = "Model2 by modelTwo";
 
-  ImplementsModel2 model_two = 1 [
-    (protokt.v1.property).non_null = true
-  ];
+  ImplementsModel2 model_two = 1;
 }
 ```
 
-Note that the `by` clause references the field by its lower camel case name.
+Note that the `by` clause references the field by its lower camel case name. 
+Properties on delegate interfaces must be nullable since message fields themselves
+are nullable and may not be present on the wire.
 
 #### Oneof Fields
 
@@ -782,7 +739,7 @@ Oneof fields can declare that they implement an interface with the
 also implement the interface. This allows access of common properties without a
 `when` statement that always ultimately extracts the same property.
 
-Suppose you have a domain object MyObjectWithConfig that has a non-null configuration
+Suppose you have a domain object MyObjectWithConfig that has configuration
 that specifies a third-party server for communication. For flexibility, this
 configuration will be modifiable by the server and versioned by a simple integer.
 To hasten subsequent loading of the configuration, a client may save a resolved
@@ -816,7 +773,6 @@ message MyObjectWithConfig {
   ];
 
   oneof Config {
-    option (protokt.v1.oneof).non_null = true;
     option (protokt.v1.oneof).implements = "com.toasttab.example.Config";
 
     ServerSpecified server_specified = 2;
@@ -836,9 +792,7 @@ message ServerSpecified {
 message ClientResolved {
   option (protokt.v1.class).implements = "com.toasttab.example.Config by config";
 
-  ServerSpecified config = 1 [
-    (protokt.v1.property).non_null = true
-  ];
+  ServerSpecified config = 1;
 
   bytes last_known_address = 2 [
     (protokt.v1.property).wrap = "java.net.InetAddress"
@@ -853,7 +807,7 @@ Protokt will generate:
 public class MyObjectWithConfig private constructor(
   @GeneratedProperty(1)
   public val id: UUID?,
-  public val config: Config,
+  public val config: Config?,
   public val unknownFields: UnknownFieldSet = UnknownFieldSet.empty()
 ) : AbstractMessage() {
 
@@ -906,18 +860,10 @@ accessing the property via a `when` expression:
 
 ```kotlin
 fun printVersion(config: MyObjectWithConfig.Config) {
-    println(config.version)
+    println(config?.version)
 }
 ```
 
-This pattern is dangerous: since the oneof must be marked non-nullable, you
-cannot compatibly add new implementing fields to a producer before a consumer
-is updated with the new generated code. The old consumer will attempt to
-deserialize the new field as an unknown field and the non-null assertion on the
-oneof field during the constructor call will fail.
-
-This functionality will likely be removed.
-
 ### BytesSlice
 
 When reading messages that contain other serialized messages as `bytes` fields,

diff --git a/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api b/extensions/protokt-extensions-lite/api/protokt-extensions-lite.api
@@ -369,14 +369,13 @@ public final class protokt/v1/EnumValueOptions$Deserializer : protokt/v1/Abstrac
 
 public final class protokt/v1/FieldOptions : protokt/v1/AbstractMessage {
 	public static final field Deserializer Lprotokt/v1/FieldOptions$Deserializer;
-	public synthetic fun <init> (ZLjava/lang/String;ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public synthetic fun <init> (Ljava/lang/String;ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun copy (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/FieldOptions;
 	public static fun deserialize (Lprotokt/v1/Reader;)Lprotokt/v1/FieldOptions;
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getBytesSlice ()Z
 	public final fun getDeprecationMessage ()Ljava/lang/String;
 	public final fun getKeyWrap ()Ljava/lang/String;
-	public final fun getNonNull ()Z
 	public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet;
 	public final fun getValueWrap ()Ljava/lang/String;
 	public final fun getWrap ()Ljava/lang/String;
@@ -393,14 +392,12 @@ public final class protokt/v1/FieldOptions$Builder {
 	public final fun getBytesSlice ()Z
 	public final fun getDeprecationMessage ()Ljava/lang/String;
 	public final fun getKeyWrap ()Ljava/lang/String;
-	public final fun getNonNull ()Z
 	public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet;
 	public final fun getValueWrap ()Ljava/lang/String;
 	public final fun getWrap ()Ljava/lang/String;
 	public final fun setBytesSlice (Z)V
 	public final fun setDeprecationMessage (Ljava/lang/String;)V
 	public final fun setKeyWrap (Ljava/lang/String;)V
-	public final fun setNonNull (Z)V
 	public final fun setUnknownFields (Lprotokt/v1/UnknownFieldSet;)V
 	public final fun setValueWrap (Ljava/lang/String;)V
 	public final fun setWrap (Ljava/lang/String;)V
@@ -593,13 +590,12 @@ public final class protokt/v1/MethodOptions$Deserializer : protokt/v1/AbstractDe
 
 public final class protokt/v1/OneofOptions : protokt/v1/AbstractMessage {
 	public static final field Deserializer Lprotokt/v1/OneofOptions$Deserializer;
-	public synthetic fun <init> (ZLjava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Lprotokt/v1/UnknownFieldSet;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun copy (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions;
 	public static fun deserialize (Lprotokt/v1/Reader;)Lprotokt/v1/OneofOptions;
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getDeprecationMessage ()Ljava/lang/String;
 	public final fun getImplements ()Ljava/lang/String;
-	public final fun getNonNull ()Z
 	public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet;
 	public fun hashCode ()I
 	public static final fun invoke (Lkotlin/jvm/functions/Function1;)Lprotokt/v1/OneofOptions;
@@ -613,11 +609,9 @@ public final class protokt/v1/OneofOptions$Builder {
 	public final fun build ()Lprotokt/v1/OneofOptions;
 	public final fun getDeprecationMessage ()Ljava/lang/String;
 	public final fun getImplements ()Ljava/lang/String;
-	public final fun getNonNull ()Z
 	public final fun getUnknownFields ()Lprotokt/v1/UnknownFieldSet;
 	public final fun setDeprecationMessage (Ljava/lang/String;)V
 	public final fun setImplements (Ljava/lang/String;)V
-	public final fun setNonNull (Z)V
 	public final fun setUnknownFields (Lprotokt/v1/UnknownFieldSet;)V
 }
 

diff --git a/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto b/extensions/protokt-extensions-lite/src/extensions-proto/protokt/v1/protokt.proto
@@ -46,18 +46,6 @@ extend google.protobuf.MessageOptions {
 }
 
 message FieldOptions {
-  // Makes a message-type field non-nullable in the generated Kotlin code.
-  // Beware that deserialization will NPE if the field is missing from the
-  // protobuf payload. Adding a non-null field to an existing message is a
-  // backwards-incompatible change.
-  //
-  // For example:
-  //
-  // message Foo {
-  //   string id = 1 [(protokt.v1.property).non_null = true];
-  // }
-  bool non_null = 1;
-
   // Expose a wrapper class instead of a raw protobuf type.
   //
   // For example:
@@ -76,14 +64,14 @@ message FieldOptions {
   // full qualification is optional.
   //
   // This option can be applied to repeated fields.
-  string wrap = 2;
+  string wrap = 1;
 
   // Maps a bytes field to BytesSlice. If deserialized from a byte array,
   // BytesSlice will point to the source array without copying the subarray.
-  bool bytes_slice = 3;
+  bool bytes_slice = 2;
 
   // Provides a message for deprecation
-  string deprecation_message = 4;
+  string deprecation_message = 3;
 
   // Expose a wrapper class instead of a raw protobuf type for the key type of
   // a map.
@@ -100,7 +88,7 @@ message FieldOptions {
   // class Foo(val map: Map<FooId, String>) ...
   //
   // Scoping rules  are the same as those for declaring regular field wrapper types.
-  string key_wrap = 5;
+  string key_wrap = 4;
 
   // Expose a wrapper class instead of a raw protobuf type for the value type of
   // a map.
@@ -117,38 +105,21 @@ message FieldOptions {
   // class Foo(val map: Map<Int, FooId>) ...
   //
   // Scoping rules  are the same as those for declaring regular field wrapper types.
-  string value_wrap = 6;
+  string value_wrap = 5;
 }
 
 extend google.protobuf.FieldOptions {
   FieldOptions property = 1072;
 }
 
 message OneofOptions {
-  // Makes a oneof field non-nullable in generated Kotlin code. Beware that
-  // deserialization will NPE if the field is missing from the protobuf payload.
-  // Adding a non-null field to an existing message is a backwards-incompatible
-  // change.
-  //
-  // For example:
-  //
-  // message Message {
-  //   oneof some_field_name {
-  //     option (protokt.v1.oneof).non_null = true;
-  //
-  //     string id = 1;
-  //   }
-  // }
-  //
-  bool non_null = 1;
-
   // Make the sealed class implement an interface, enforcing the presence of a
   // property in each possible variant. Scoping rules  are the same as those
   // for declaring wrapper types.
-  string implements = 2;
+  string implements = 1;
 
   // Provides a message for deprecation
-  string deprecation_message = 3;
+  string deprecation_message = 2;
 }
 
 extend google.protobuf.OneofOptions {

diff --git a/gradle-plugin-integration-test/multiplatform/src/main/proto/testing/test.proto b/gradle-plugin-integration-test/multiplatform/src/main/proto/testing/test.proto
@@ -34,6 +34,6 @@ message TestMessage {
   }
 
   message Submessage {
-    TestMessage qux = 1 [(protokt.v1.property).non_null = true];
+    TestMessage qux = 1;
   }
 }
diff --git a/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/DeserializerSupport.kt b/protokt-codegen/src/main/kotlin/protokt/v1/codegen/generate/DeserializerSupport.kt
@@ -17,7 +17,6 @@ package protokt.v1.codegen.generate
 
 import com.squareup.kotlinpoet.CodeBlock
 import com.squareup.kotlinpoet.buildCodeBlock
-import com.squareup.kotlinpoet.withIndent
 import protokt.v1.reflect.FieldType
 
 internal fun deserializeVarInitialState(p: PropertyInfo) =
@@ -28,33 +27,15 @@ internal fun deserializeVarInitialState(p: PropertyInfo) =
     }
 
 internal fun wrapDeserializedValueForConstructor(p: PropertyInfo) =
-    if (p.nonNullOption) {
-        buildCodeBlock {
-            beginControlFlow("requireNotNull(%N)", p.name)
-            add("StringBuilder(\"%N\")\n", p.name)
-            withIndent {
-                add(
-                    (
-                        ".append(\" specified nonnull with (protokt." +
-                            "${if (p.oneof) "oneof" else "property"}).non_null " +
-                            "but was null\")"
-                        ).bindSpaces()
-                )
-                add("\n")
-            }
-            endControlFlowWithoutNewline()
-        }
+    if (p.isMap) {
+        CodeBlock.of("%M(%N)", unmodifiableMap, p.name)
+    } else if (p.repeated) {
+        CodeBlock.of("%M(%N)", unmodifiableList, p.name)
     } else {
-        if (p.isMap) {
-            CodeBlock.of("%M(%N)", unmodifiableMap, p.name)
-        } else if (p.repeated) {
-            CodeBlock.of("%M(%N)", unmodifiableList, p.name)
-        } else {
-            buildCodeBlock {
-                add("%N", p.name)
-                if (p.wrapped && !p.nullable) {
-                    add(" ?: %L", p.defaultValue)
-                }
+        buildCodeBlock {
+            add("%N", p.name)
+            if (p.wrapped && !p.nullable) {
+                add(" ?: %L", p.defaultValue)
             }
         }
     }