Skip to content
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

[C#] Enforce the property setter for non-nullable properties #1035

Open
YanlongLi opened this issue May 11, 2020 · 1 comment
Open

[C#] Enforce the property setter for non-nullable properties #1035

YanlongLi opened this issue May 11, 2020 · 1 comment

Comments

@YanlongLi
Copy link
Contributor

For both nullable and non-nullable properties, bond simply generates the default getter and setter, we'd prefer to add the null validation for the setter of non-null properties. For example,

struct Foo
{
    0: string ID;
}

it generates code like below, this allows setting the ID to null but the serialization will fail later since the ID is not defined as nullable<string>.

public partial class Foo
{
	[global::Bond.Id(0)]
	public string ID{ get; set; }
}

We prefer a result like below. Since we are using bond over GRPC and the serialization failures are out of the scope of the service implementation, it's hard to catch that error and it's not possible to correlate the error with requests. Generated code like below allows us to catch the failure earlier when we return the response.

public partial class Foo
{
	private string _ID;
	[global::Bond.Id(0), global::Bond.Type(typeof(global::Bond.Tag.nullable<string>))]
	public string ID
	{
		get => _ID;
		set
		{
			 if (value == null)
			 {
				 throw new ArgumentNullException("xxx");
			 }
			 _ID = value;
		}
	}
}
@chwarr
Copy link
Member

chwarr commented May 12, 2020

Using C# 8.0's non-nullable reference types would be more in the spirit of C# Bond. They would only be analyzed at compilation time. I believe they can be added in a way that is backward compatible all the way back to .NET 4.5: they're just ignored by compilers that don't understand them.

Explicit checking could be added to codegen behind a switch. However, any codegen switch like this needs to be carefully considered, as it doubles the test matrix. Right now, my inclination would be only to add the appropriate annotations for non-nullable reference types, since they can be added--I believe--unconditionally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants