Testura / Testura.Code

Testura.Code is a wrapper around the Roslyn API and used for generation, saving and compiling C# code. It provides methods and helpers to generate classes, methods, statements and expressions.
MIT License
297 stars 30 forks source link

a quick route to allow "string?" #93

Closed 274188A closed 2 years ago

274188A commented 2 years ago

involved adding a optional param - see below.

// Namespace path to method >>  Testura.Code.Generators.Common.Create

public static TypeSyntax Create(Type type, bool isNullableString = false)

then handling it like so:

if (type == typeof(string))
{
    if (nullableString)
    {
        typeSyntax = PredefinedType(Token(default, SyntaxKind.StringKeyword, "string?", "?", default));
    }
    else
    {
        typeSyntax = PredefinedType(Token(SyntaxKind.StringKeyword));
    }
}
MilleBo commented 2 years ago

A good idea but maybe we should make the flag usable for more types than string? I guess it could be good for all reference types? The problem is that it also could be a bit confusing with a flag. For say that we for example do it like this:

public static TypeSyntax Create(Type type, bool isNullable = false)

Then you call it like this:

TypeGenerator.Create(typeof(int?), isNullable: false); 

Would it create a nullable or not?

So another idea to prevent that would be to create it as a totally different method instead:

public static TypeSyntax CreateNullable(Type type) 

That simply force everything to nullable whatever you write typeof(int) or typeof(int?). This of course also means that these two calls would create the same thing:

TypeGenerator.Create(typeof(int?));
TypeGenerator.CreateNullable(typeof(int));

But I guess that should'nt be an issue. Any thoughts?

274188A commented 2 years ago

Yes totally agree on avoiding the overload - I'll take another look

Seems to me that type 'string?' is the only outlier one we are trying to handle here - am I right i thinking that? If so we could be better off with a custom method especially for that one case

TypeGenerator.CreateNullableString();
MilleBo commented 2 years ago

Wouldn't it be needed for all kind reference types if you set <Nullable>enable</Nullable> in your .csproj file?

For example if you have that enabled and try to do this:

public class Class2
{
    public void Test(Class1 class1)
    {
        if (class1 == null) // Will say that this is alaways false according to nullable reference type annotations
        {

        }
    }
}

The only way to get that warning away is to actually set it explicit nullable:

public class Class2
{
    public void Test(Class1? class1)
    {
        if (class1 == null) // Now it don't give a warning
        {

        }
    }
}

But know when I think about it a bit more I guss you could also just useCustomType.Generate(..)? So for example to get string? you would simply do TypeGenerator.Create(CustomType.Generate("string?")).

It save us a bit of a headache of doing the special case, it already exist and it is actually meant to use for these kind of things.

274188A commented 2 years ago

I think TypeGenerator.Create(CustomType.Generate("string?")) is the way to go - I'll close this an delete my PR