Create analyzer to simulate a "not-a-graph-type-constraint"

[gao-artur]

Another analyzer idea: prevent invalid uses of IGraphType-derived classes, such as: (I did this today)

//valid
GraphQLClrInputTypeReference<string>
//invalid
GraphQLClrInputTypeReference<StringGraphType>

Ditto for GraphQLClrOutputTypeReference<T> and IDataLoaderResult<T>. Unfortunately there's no type constraint that says 'is not IGraphType'. This idea could be expanded to other use cases like return types of FieldBuilder and .Returns<T>().

We do have type constraints for NonNullGraphType<T> and ListGraphType<T> so no analyzer is needed there.

Originally posted by @Shane32 in #3732 (comment)

[gao-artur]

@Shane32 I think the best solution will be creating an attribute

[AttributeUsage(AttributeTargets.GenericParameter)]
public class NotAGraphTypeAttribute : Attribute { }

Usage

public sealed class GraphQLClrOutputTypeReference<[NotAGraphType] T> : InterfaceGraphType, IObjectGraphType

Or I can make it more flexible

[AttributeUsage(AttributeTargets.GenericParameter)]
public class NotAssignableToAttribute<T> : Attribute { }

Usage

public sealed class GraphQLClrOutputTypeReference<[NotAssignableTo<IGraphType>] T> : InterfaceGraphType, IObjectGraphType

Finally, I can create an analyzer for NotAssignableToAttribute<T>, which will take into account derived types

[AttributeUsage(AttributeTargets.GenericParameter)]
public class NotAGraphTypeAttribute : NotAssignableToAttribute<IGraphType> { }

I think the last one is the most flexible and user-friendly.
Anyway, applying an attribute on a generic parameter will change the public API, so it probably should go to v8.

[gao-artur]

This analyzer should also ensure constraints on the derived types. For example, given type A

public class A<[NotAssignableTo<IGraphType>] T> { }

And B derived from A:

public class B<T> : A<T> { }

The constraint must also be present on B

public class B<[NotAssignableTo<IGraphType>] T> : A<T> { }

The code fix should be provided to add missing constraints automatically.
The same applies to generic parameters on methods.

Also, in contrast to other analyzers, this analyzer should be consumed by the GraphQL library.
Long story short: it looks like a job for a separate project. Maybe Roslynator.

[Shane32]

Generic attributes are a relatively new feature and probably(?) not supported by .NET Standard or .NET Framework. I’d suggest the former option.

[Shane32]

It’s a C# 11 / .NET 7 feature

[gao-artur]

Generic was just an example. It can be [NotAssignableTo(typeof(IGraphType))].

[gao-artur]

But anyway, current analyzers project is only referenced, but not consumed by the library itself. For this feature to work in all scenarios it should be consumed by the library and by user's code.
I can make it "half working" without attributes. It will work only on simple scenarios. And it will require chenging analizer every time a new API is added that require the same constraint.

[Shane32]

I like the attribute idea

[Shane32]

Generic was just an example. It can be [NotAssignableTo(typeof(IGraphType))].

That works

[Shane32]

Long story short: it looks like a job for a separate project. Maybe Roslynator.

Ok I missed this before. Whatever you think.

Also, in contrast to other analyzers, this analyzer should be consumed by the GraphQL library.

So like the analyzer references GraphQL ? Seems like that would make a circular reference. Probably better to use reflection to obtain the reference to the NotAssignableToAttribute if that’s the limitation. (??)

[Shane32]

Wouldn’t the attribute be located in the main GraphQL library? Not in the analyzer library. I wouldn’t think that user code should really reference any types in the analyzer.

[Shane32]

I’d be okay making the attribute internal too, just for the analyzer to function.

[gao-artur]

Let me try to elaborate more.

Option 1 (perfect):

This can be implemented as a powerful feature in a separate package that can be reused everywhere, not only in GraphQL project.

[AttributeUsage(AttributeTargets.GenericParameter)]
public class NotAssignableToAttribute<T> : Attribute { }

[AttributeUsage(AttributeTargets.GenericParameter)]
public class NotAGraphTypeAttribute : NotAssignableToAttribute<IGraphType> { }

Applying NotAGraphType attribute should behave similarly to a generic constraint. For example, applying the attribute on the FieldBuilder class

public class FieldBuilder<TSourceType, [NotAGraphType] TReturnType>

Should force applying the same attribute on Returns and other methods.

public virtual FieldBuilder<TSourceType, TNewReturnType> Returns<[NotAGraphType] TNewReturnType>()

If the user derives from the FieldBuilder or creates an extension method with TReturnType generic parameter, he will be forced to apply the attribute too.
This way, the constraint is always preset in relevant places, and it can be easily validated.

Option 2 (limited):

NotAGraphType is just an internal marker attribute. Applying it to the FieldBuilder<> class, you need to remember to apply it to the Returns and other methods. The validation is easy in simple cases when the annotated type/method is used directly:

Field<StringGraphType, string>("name");
or 
Field<StringGraphType>("name").Returns<string>();

But difficult, and sometimes impossible in advanced usages:

public void DoSomething<TReturnType>()
{
    new FieldBuilder<StringGraphType, TReturnType>("name")
}

But perhaps the simple usage is the most common, so it should be ok.

Regarding the attribute location, it should be defined in the using project, not in the Analyzer project. And if it's internal, it should be redefined in every project like GraphQL or GraphQL.DataLoader.