Rename Nullable scalars to Optional scalars (#6112)

Co-authored-by: Casper Neo <cneo@google.com>
This commit is contained in:
Casper
2020-09-10 16:04:36 -04:00
committed by GitHub
parent f5ab24bc41
commit 338944d3d9
6 changed files with 37 additions and 37 deletions

View File

@@ -296,7 +296,7 @@ struct FieldDef : public Definition {
shared(false), shared(false),
native_inline(false), native_inline(false),
flexbuffer(false), flexbuffer(false),
nullable(false), optional(false),
nested_flatbuffer(NULL), nested_flatbuffer(NULL),
padding(0) {} padding(0) {}
@@ -315,7 +315,7 @@ struct FieldDef : public Definition {
bool native_inline; // Field will be defined inline (instead of as a pointer) bool native_inline; // Field will be defined inline (instead of as a pointer)
// for native tables if field is a struct. // for native tables if field is a struct.
bool flexbuffer; // This field contains FlexBuffer data. bool flexbuffer; // This field contains FlexBuffer data.
bool nullable; // If True, this field is Null (as opposed to default bool optional; // If True, this field is Null (as opposed to default
// valued). // valued).
StructDef *nested_flatbuffer; // This field contains nested FlatBuffer data. StructDef *nested_flatbuffer; // This field contains nested FlatBuffer data.
size_t padding; // Bytes to always pad after this field. size_t padding; // Bytes to always pad after this field.
@@ -932,7 +932,7 @@ class Parser : public ParserState {
bool SupportsAdvancedUnionFeatures() const; bool SupportsAdvancedUnionFeatures() const;
bool SupportsAdvancedArrayFeatures() const; bool SupportsAdvancedArrayFeatures() const;
bool SupportsNullableScalars() const; bool SupportsOptionalScalars() const;
Namespace *UniqueNamespace(Namespace *ns); Namespace *UniqueNamespace(Namespace *ns);
FLATBUFFERS_CHECKED_ERROR RecurseError(); FLATBUFFERS_CHECKED_ERROR RecurseError();

View File

@@ -110,13 +110,13 @@ class LobsterGenerator : public BaseGenerator {
offsets + ")"; offsets + ")";
} else { } else {
auto defval = field.nullable ? "0" : field.value.constant; auto defval = field.optional ? "0" : field.value.constant;
acc = "buf_.flatbuffers_field_" + GenTypeName(field.value.type) + acc = "buf_.flatbuffers_field_" + GenTypeName(field.value.type) +
"(pos_, " + offsets + ", " + defval + ")"; "(pos_, " + offsets + ", " + defval + ")";
} }
if (field.value.type.enum_def) if (field.value.type.enum_def)
acc = NormalizedName(*field.value.type.enum_def) + "(" + acc + ")"; acc = NormalizedName(*field.value.type.enum_def) + "(" + acc + ")";
if (field.nullable) if (field.optional)
acc += ", buf_.flatbuffers_field_present(pos_, " + offsets + ")"; acc += ", buf_.flatbuffers_field_present(pos_, " + offsets + ")";
code += def + "():\n return " + acc + "\n"; code += def + "():\n return " + acc + "\n";
return; return;
@@ -201,7 +201,7 @@ class LobsterGenerator : public BaseGenerator {
NormalizedName(field) + ":" + LobsterType(field.value.type) + NormalizedName(field) + ":" + LobsterType(field.value.type) +
"):\n b_.Prepend" + GenMethod(field.value.type) + "Slot(" + "):\n b_.Prepend" + GenMethod(field.value.type) + "Slot(" +
NumToString(offset) + ", " + NormalizedName(field); NumToString(offset) + ", " + NormalizedName(field);
if (IsScalar(field.value.type.base_type) && !field.nullable) if (IsScalar(field.value.type.base_type) && !field.optional)
code += ", " + field.value.constant; code += ", " + field.value.constant;
code += ")\n return this\n"; code += ")\n return this\n";
} }

View File

@@ -665,10 +665,10 @@ class RustGenerator : public BaseGenerator {
switch (GetFullType(field.value.type)) { switch (GetFullType(field.value.type)) {
case ftInteger: case ftInteger:
case ftFloat: { case ftFloat: {
return field.nullable ? "None" : field.value.constant; return field.optional ? "None" : field.value.constant;
} }
case ftBool: { case ftBool: {
return field.nullable ? "None" : return field.optional ? "None" :
field.value.constant == "0" ? "false" : "true"; field.value.constant == "0" ? "false" : "true";
} }
case ftUnionKey: case ftUnionKey:
@@ -707,7 +707,7 @@ class RustGenerator : public BaseGenerator {
case ftFloat: case ftFloat:
case ftBool: { case ftBool: {
const auto typname = GetTypeBasic(type); const auto typname = GetTypeBasic(type);
return field.nullable ? "Option<" + typname + ">" : typname; return field.optional ? "Option<" + typname + ">" : typname;
} }
case ftStruct: { case ftStruct: {
const auto typname = WrapInNameSpace(*type.struct_def); const auto typname = WrapInNameSpace(*type.struct_def);
@@ -865,7 +865,7 @@ class RustGenerator : public BaseGenerator {
case ftBool: case ftBool:
case ftFloat: { case ftFloat: {
const auto typname = GetTypeBasic(field.value.type); const auto typname = GetTypeBasic(field.value.type);
return (field.nullable ? return (field.optional ?
"self.fbb_.push_slot_always::<" : "self.fbb_.push_slot_always::<" :
"self.fbb_.push_slot::<") + typname + ">"; "self.fbb_.push_slot::<") + typname + ">";
} }
@@ -910,7 +910,7 @@ class RustGenerator : public BaseGenerator {
case ftFloat: case ftFloat:
case ftBool: { case ftBool: {
const auto typname = GetTypeBasic(type); const auto typname = GetTypeBasic(type);
return field.nullable ? "Option<" + typname + ">" : typname; return field.optional ? "Option<" + typname + ">" : typname;
} }
case ftStruct: { case ftStruct: {
const auto typname = WrapInNameSpace(*type.struct_def); const auto typname = WrapInNameSpace(*type.struct_def);
@@ -994,7 +994,7 @@ class RustGenerator : public BaseGenerator {
case ftFloat: case ftFloat:
case ftBool: { case ftBool: {
const auto typname = GetTypeBasic(type); const auto typname = GetTypeBasic(type);
if (field.nullable) { if (field.optional) {
return "self._tab.get::<" + typname + ">(" + offset_name + ", None)"; return "self._tab.get::<" + typname + ">(" + offset_name + ", None)";
} else { } else {
const auto default_value = GetDefaultScalarValue(field); const auto default_value = GetDefaultScalarValue(field);
@@ -1092,7 +1092,7 @@ class RustGenerator : public BaseGenerator {
} }
bool TableFieldReturnsOption(const FieldDef &field) { bool TableFieldReturnsOption(const FieldDef &field) {
if (field.nullable) return true; if (field.optional) return true;
switch (GetFullType(field.value.type)) { switch (GetFullType(field.value.type)) {
case ftInteger: case ftInteger:
case ftFloat: case ftFloat:
@@ -1416,7 +1416,7 @@ class RustGenerator : public BaseGenerator {
code_ += code_ +=
" pub fn add_{{FIELD_NAME}}(&mut self, {{FIELD_NAME}}: " " pub fn add_{{FIELD_NAME}}(&mut self, {{FIELD_NAME}}: "
"{{FIELD_TYPE}}) {"; "{{FIELD_TYPE}}) {";
if (is_scalar && !field.nullable) { if (is_scalar && !field.optional) {
code_.SetValue("FIELD_DEFAULT_VALUE", code_.SetValue("FIELD_DEFAULT_VALUE",
TableBuilderAddFuncDefaultValue(field)); TableBuilderAddFuncDefaultValue(field));
code_ += code_ +=

View File

@@ -503,7 +503,7 @@ class SwiftGenerator : public BaseGenerator {
auto &create_func_header = *create_header; auto &create_func_header = *create_header;
auto name = Name(field); auto name = Name(field);
auto type = GenType(field.value.type); auto type = GenType(field.value.type);
auto nullable_type = (field.nullable ? type + "?" : type); auto nullable_type = (field.optional ? type + "?" : type);
code_.SetValue("VALUENAME", name); code_.SetValue("VALUENAME", name);
code_.SetValue("VALUETYPE", nullable_type); code_.SetValue("VALUETYPE", nullable_type);
code_.SetValue("OFFSET", name); code_.SetValue("OFFSET", name);
@@ -524,14 +524,14 @@ class SwiftGenerator : public BaseGenerator {
code_ += code_ +=
"{{VALUETYPE}}" + builder_string + "fbb.add(element: {{VALUENAME}}\\"; "{{VALUETYPE}}" + builder_string + "fbb.add(element: {{VALUENAME}}\\";
code_ += field.nullable ? "\\" : (is_enum + ", def: {{CONSTANT}}\\"); code_ += field.optional ? "\\" : (is_enum + ", def: {{CONSTANT}}\\");
code_ += ", at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; code_ += ", at: {{TABLEOFFSET}}.{{OFFSET}}.p) }";
auto default_value = IsEnum(field.value.type) ? GenEnumDefaultValue(field) auto default_value = IsEnum(field.value.type) ? GenEnumDefaultValue(field)
: field.value.constant; : field.value.constant;
create_func_header.push_back("" + name + ": " + nullable_type + " = " + create_func_header.push_back("" + name + ": " + nullable_type + " = " +
(field.nullable ? "nil" : default_value)); (field.optional ? "nil" : default_value));
return; return;
} }
@@ -540,13 +540,13 @@ class SwiftGenerator : public BaseGenerator {
"0" == field.value.constant ? "false" : "true"; "0" == field.value.constant ? "false" : "true";
code_.SetValue("CONSTANT", default_value); code_.SetValue("CONSTANT", default_value);
code_.SetValue("VALUETYPE", field.nullable ? "Bool?" : "Bool"); code_.SetValue("VALUETYPE", field.optional ? "Bool?" : "Bool");
code_ += "{{VALUETYPE}}" + builder_string + code_ += "{{VALUETYPE}}" + builder_string +
"fbb.add(element: {{VALUENAME}},\\"; "fbb.add(element: {{VALUENAME}},\\";
code_ += field.nullable ? "\\" : " def: {{CONSTANT}},"; code_ += field.optional ? "\\" : " def: {{CONSTANT}},";
code_ += " at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; code_ += " at: {{TABLEOFFSET}}.{{OFFSET}}.p) }";
create_func_header.push_back(name + ": " + nullable_type + " = " + create_func_header.push_back(name + ": " + nullable_type + " = " +
(field.nullable ? "nil" : default_value)); (field.optional ? "nil" : default_value));
return; return;
} }
@@ -590,8 +590,8 @@ class SwiftGenerator : public BaseGenerator {
code_.SetValue("VALUETYPE", type); code_.SetValue("VALUETYPE", type);
code_.SetValue("OFFSET", name); code_.SetValue("OFFSET", name);
code_.SetValue("CONSTANT", field.value.constant); code_.SetValue("CONSTANT", field.value.constant);
std::string nullable = field.nullable ? "nil" : "{{CONSTANT}}"; std::string nullable = field.optional ? "nil" : "{{CONSTANT}}";
std::string optional = field.nullable ? "?" : ""; std::string optional = field.optional ? "?" : "";
auto const_string = "return o == 0 ? " + nullable + " : "; auto const_string = "return o == 0 ? " + nullable + " : ";
GenComment(field.doc_comment); GenComment(field.doc_comment);
if (IsScalar(field.value.type.base_type) && !IsEnum(field.value.type) && if (IsScalar(field.value.type.base_type) && !IsEnum(field.value.type) &&
@@ -1105,12 +1105,12 @@ class SwiftGenerator : public BaseGenerator {
} }
default: { default: {
buffer_constructor.push_back(name + " = _t." + name); buffer_constructor.push_back(name + " = _t." + name);
std::string nullable = field.nullable ? "?" : ""; std::string nullable = field.optional ? "?" : "";
if (IsScalar(field.value.type.base_type) && if (IsScalar(field.value.type.base_type) &&
!IsBool(field.value.type.base_type) && !IsEnum(field.value.type)) { !IsBool(field.value.type.base_type) && !IsEnum(field.value.type)) {
code_ += code_ +=
"{{ACCESS_TYPE}} var {{VALUENAME}}: {{VALUETYPE}}" + nullable; "{{ACCESS_TYPE}} var {{VALUENAME}}: {{VALUETYPE}}" + nullable;
if (!field.nullable) if (!field.optional)
base_constructor.push_back(name + " = " + field.value.constant); base_constructor.push_back(name + " = " + field.value.constant);
break; break;
} }
@@ -1128,7 +1128,7 @@ class SwiftGenerator : public BaseGenerator {
code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: Bool" + nullable; code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: Bool" + nullable;
std::string default_value = std::string default_value =
"0" == field.value.constant ? "false" : "true"; "0" == field.value.constant ? "false" : "true";
if (!field.nullable) if (!field.optional)
base_constructor.push_back(name + " = " + default_value); base_constructor.push_back(name + " = " + default_value);
} }
} }

View File

@@ -744,13 +744,13 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
"default values currently only supported for scalars in tables"); "default values currently only supported for scalars in tables");
} }
// Mark the nullable scalars. Note that a side effect of ParseSingleValue is // Mark the optional scalars. Note that a side effect of ParseSingleValue is
// fixing field->value.constant to null. // fixing field->value.constant to null.
if (IsScalar(type.base_type)) { if (IsScalar(type.base_type)) {
field->nullable = (field->value.constant == "null"); field->optional = (field->value.constant == "null");
if (field->nullable && !SupportsNullableScalars()) { if (field->optional && !SupportsOptionalScalars()) {
return Error( return Error(
"Nullable scalars are not yet supported in at least one the of " "Optional scalars are not yet supported in at least one the of "
"the specified programming languages." "the specified programming languages."
); );
} }
@@ -2257,7 +2257,7 @@ CheckedError Parser::CheckClash(std::vector<FieldDef *> &fields,
} }
bool Parser::SupportsNullableScalars() const { bool Parser::SupportsOptionalScalars() const {
return !(opts.lang_to_generate & return !(opts.lang_to_generate &
~(IDLOptions::kRust | IDLOptions::kSwift | IDLOptions::kLobster)); ~(IDLOptions::kRust | IDLOptions::kSwift | IDLOptions::kLobster));
} }

View File

@@ -3427,8 +3427,8 @@ void TestEmbeddedBinarySchema() {
0); 0);
} }
void NullableScalarsTest() { void OptionalScalarsTest() {
// Simple schemas and a "has nullable scalar" sentinal. // Simple schemas and a "has optional scalar" sentinal.
std::vector<std::string> schemas; std::vector<std::string> schemas;
schemas.push_back("table Monster { mana : int; }"); schemas.push_back("table Monster { mana : int; }");
schemas.push_back("table Monster { mana : int = 42; }"); schemas.push_back("table Monster { mana : int = 42; }");
@@ -3452,10 +3452,10 @@ void NullableScalarsTest() {
flatbuffers::Parser parser; flatbuffers::Parser parser;
TEST_ASSERT(parser.Parse(schema->c_str())); TEST_ASSERT(parser.Parse(schema->c_str()));
const auto *mana = parser.structs_.Lookup("Monster")->fields.Lookup("mana"); const auto *mana = parser.structs_.Lookup("Monster")->fields.Lookup("mana");
TEST_EQ(mana->nullable, has_null); TEST_EQ(mana->optional, has_null);
} }
// Test if nullable scalars are allowed for each language. // Test if optional scalars are allowed for each language.
const int kNumLanguages = 17; const int kNumLanguages = 17;
const auto supported = (flatbuffers::IDLOptions::kRust | const auto supported = (flatbuffers::IDLOptions::kRust |
flatbuffers::IDLOptions::kSwift | flatbuffers::IDLOptions::kSwift |
@@ -3475,9 +3475,9 @@ void NullableScalarsTest() {
void ParseFlexbuffersFromJsonWithNullTest() { void ParseFlexbuffersFromJsonWithNullTest() {
// Test nulls are handled appropriately through flexbuffers to exercise other // Test nulls are handled appropriately through flexbuffers to exercise other
// code paths of ParseSingleValue in the nullable scalars change. // code paths of ParseSingleValue in the optional scalars change.
// TODO(cneo): Json -> Flatbuffers test once some language can generate code // TODO(cneo): Json -> Flatbuffers test once some language can generate code
// with nullable scalars. // with optional scalars.
{ {
char json[] = "{\"opt_field\": 123 }"; char json[] = "{\"opt_field\": 123 }";
flatbuffers::Parser parser; flatbuffers::Parser parser;
@@ -3593,7 +3593,7 @@ int FlatBufferTests() {
TestMonsterExtraFloats(); TestMonsterExtraFloats();
FixedLengthArrayTest(); FixedLengthArrayTest();
NativeTypeTest(); NativeTypeTest();
NullableScalarsTest(); OptionalScalarsTest();
ParseFlexbuffersFromJsonWithNullTest(); ParseFlexbuffersFromJsonWithNullTest();
return 0; return 0;
} }