How to deal with NonNull

[shoooe]

Hello,
what's the proper way to deal with NonNull<T> not being T?
Let's assume that you have a resolver like:

public async Task<NonNull<IEnumerable<NonNull<X>>>> AllXs(...)
{
    IEnumerable<X> list = await ...;
    // how to return this?
}

what's the proper way to return list?

Do I have to add some boilerplate code like:

return new NonNull<IEnumerable<NonNull<X>>>(list.Select(x => new NonNull<X>(x)));

?

I've tried to look at both examples, but none of those seem to use NonNull for output types.

[shoooe]

Maybe with C# 8 and .NET Core 3 the library could drop NonNull and just use C#'s Nullable.

[Igorbek]

I was also wondering if C#8 nullability feature could be used instead.
The issue I see with that is changing the default behavior from interpreting a bare reference type from being nullable to being non-nullable.
However, switching from one behavior to another can be done same way as compiler do by examining the assembly if it has this feature turned on.

[infogulch]

Some time ago I tried (and failed) to make a NonNull Attribute. The problem is that you trust your resolver to return nonnull data, but the only way to communicate that result into the schema is to wrap the type in NonNull<>. The idea was that you could mark it as non-null with an attribute for the purposes of communicating nullability in the schema, but not have to do anything different with the types.

But looking at your example it seems that this solution may not be sufficient, since it also includes interior non-nullable types, and a single top-level attribute will necessarily be insufficient to describe the nullability of multiple nested types.

C# 8 Nullability does sound interesting.

[shoooe]

The issue I see with that is changing the default behavior from interpreting a bare reference type from being nullable to being non-nullable.

That should have been the default for GraphQL as well. 😞

[tlil]

return new NonNull<IEnumerable<NonNull<X>>>(list.Select(x => new NonNull<X>(x)));

Yes, unfortunately @shoooe, that's currently the way to go. I've been looking into simplified means of returning data of this structure, incl. using C# 8 nullables. There's a range of challenges with that, in particular around backward-compatibility. Give me some time to continue my current exploration, and I will get back to you.

[BilyachenkoOY]

We've came up with NonNull and NonNullItem attributes which can be used in the following way:

public class ProductModel
{
  public string Id { get; set; }
  /*other fields*/
}
[NonNull, NonNullItem]
public virtual IEnumerable<ProductModel> GetProducts(/*params*/) => Enumerable.Empty<ProductModel>();

// or make model non-nullable by default

[NonNull]
public class ProductModel
{
  public string Id { get; set; }
}
[NonNull]
public virtual IEnumerable<ProductModel> GetProducts(/*params*/) => Enumerable.Empty<ProductModel>();

The same can be used for arguments (you should "enable" execution filters on arguments previously and set own wrappers to ExecutionFilterAttributeHandler.Wrapper by reflection).

[shoe-diamente]

return new NonNull<IEnumerable<NonNull<X>>>(list.Select(x => new NonNull<X>(x)));

Yes, unfortunately @shoooe, that's currently the way to go. I've been looking into simplified means of returning data of this structure, incl. using C# 8 nullables. There's a range of challenges with that, in particular around backward-compatibility. Give me some time to continue my current exploration, and I will get back to you.

Any news? I'd be happy to help.

[shoooe]

I was experimenting with the idea of adding support for nullable reference types while maintaining backward compatibility and I stumbled upon this:

https://stackoverflow.com/questions/56705694/how-to-detect-whether-a-type-can-be-nullable-at-runtime?noredirect=1#comment99974918_56705694

Basically nullable reference types are implemented via attributes, not with data encoded in the type itself. So the TypeInfo available in GraphQL.Conventions.Types.Descriptors.GraphTypeInfo doesn't really have the information regarding nullability.

[shoooe]

Apparently this issue is blocked by:
http://github.com/dotnet/corefx/issues/38087

Once that's implemented I'll work on the non null issue myself if nobody is against it.

[tlil]

Yes, you are right. You'd have to look at the field info.

Once that's implemented I'll work on the non null issue myself if nobody is against it.

That'd be great, thanks!

[Igorbek]

I actually think it's not a blocker at all. I experimented with non-null introspection and found it was very easy to analyse manually by expecting the attribute. It is very well documented and the new API is just a new convenient way to inspect then.

The hardest part for adopting it in the conventions library is to deal the way how types are mapped. Everywhere through the codebase the primary bearer of the type is System.Type, whereas it's not enough now with nullability. The code needs to be changed to introduce a better representation of the type.

[shoe-diamente]

It is very well documented and the new API is just a new convenient way to inspect then.

I couldn't find anything. Could you link to the documentation please? Maybe I missed something.

[Igorbek]

I probably was under wrong impression that it was well documented :)
Here's what I used, an official spec and a couple of blog posts:

• https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md
• https://codeblog.jonskeet.uk/2019/02/10/nullableattribute-and-c-8/
• http://code.fitness/post/2019/02/nullableattribute.html

When I played with it, it was quite easy to extract nullability. If somebody is willing to start to work on this I can help.

[shoooe]

These days I don't have much time. Maybe next month.

[shoe-diamente]

I was trying to add a new property to every GraphEntityInfo (found in src/GraphQL.Conventions/Types/Descriptors/GraphEntityInfo.cs) which would tell if the entity info is nullable or not.

Now, there you get a ICustomAttributeProvider, which seems perfect for the problem. Except that attributeProvider.GetCustomAttributes would require a type, but NullableAttribute is not ever in scope. It doesn't seem to exist.

I read all those links, and they all mention NullableAttribute, but it doesn't seem to exist.
Any help? @Igorbek

[Igorbek]

@shoe-diamente NullableAttribute definitely exists if you compiled a C#8 program with #nullable. This is the way how the compiler injects that information.

I can create a simple gist later this week on how to use it. Sorry, cannot do this today or tomorrow. Meanwhile, make sure you compile C# 8 with nullable reference types enabled.

[shoooe]

@Igorbek I can't compile the entire project with nullable references because the library has to also maintain the 90% of other tests for C# 7-.

By using the #nullable code block flag I still wasn't able to access the NullableAttribute.
I hope your gist will help me understand.

[shoe-diamente]

Even compiling the entire project with <Nullable>enable</Nullable> (which works as I get a bunch of errors regarding nullable reference types) doesn't seem to allow me to get a hand on NullableAttribute:

Types/Descriptors/GraphEntityInfo.cs(16,58): error CS0246: The type or namespace name 'NullableAttribute' could not be found (are you missing a using directive or an assembly reference?) [/Applications/XAMPP/xamppfiles/htdocs/conventions/src/GraphQL.Conventions/GraphQL.Conventions.csproj]

Even though I import:

using System.Runtime.CompilerServices;

and then call it as:

attributeProvider.GetCustomAttributes(typeof(NullableAttribute));

[shoe-diamente]

Under "What’s going on under the hood?" of this article you provided it appears that:

When using reflection (e.g. GetProperties() and GetCustomAttributes() I do see the NullableAttribute, but I cannot reference the type itself; it is as if the compiler did not know that it existed before it emitted the code.

I'm attempting to use Namotion.Reflection.

[Igorbek]

It looks like they changed the way the attribute gets injected since I experimented with it last time.
Before, it was a type defined in "compiler services" assembly (don't remember its exact name).
Now they just embed a new type with full name System.Runtime.CompilerServices.NullableAttribute into the assembly. That means, they technically different types, although all have the same full name.

That being said, the correct way is to use its well-known name and verify, if needed, structurally. We still have access to this type, we just cannot compile-in its statically.

Here's a small example of how some method is encoded:

Source:

using System;

namespace ClassLibrary1
{
    public class TestClass
    {
        public string? Test(string? arg) { return arg; }
    }
}

Compiled:

using System.Runtime.CompilerServices;

namespace ClassLibrary1
{
  public class TestClass
  {
    [return: Nullable(2)]
    public string Test([Nullable(2)] string arg)
    {
      return arg;
    }
  }
}

using Microsoft.CodeAnalysis;
using System.Runtime.InteropServices;

// NOTE: this is the same assembly
namespace System.Runtime.CompilerServices
{
  [CompilerGenerated]
  [Embedded]
  internal sealed class NullableAttribute : Attribute
  {
    public readonly byte[] NullableFlags;

    public NullableAttribute([In] byte obj0): base()
    {
      this.NullableFlags = new byte[1]{ obj0 };
    }

    public NullableAttribute([In] byte[] obj0): base()
    {
      this.NullableFlags = obj0;
    }
  }
}

[shoe-diamente]

Reasonably I don't have time to work on this unfortunately for the next few months.
I was evaluating a possible use of this library but I think I'll ultimately decide against.

This is were I stopped: https://github.com/shoe-diamente/conventions.

I'd recommend using Namotion.Reflection. There's a contextual type type which should replace Type in a few places.

[shoe-diamente]

Nevermind. Even without solving the NonNull<> problem this library has much less ceremony than the alternatives. It's also much more type safe. The fact that it supports partial updates (via Optional<>) was what made us choose this library.

Therefore I'll may have some time to work on either documentation or this issue. Is anybody else willing to help?