NeVeSpl / NTypewriter

File/code generator using Scriban text templates populated with C# code metadata from Roslyn API.
https://nevespl.github.io/NTypewriter/
MIT License
124 stars 24 forks source link

IType.IsAbsract and .IsInterface aren't mutually exclusive (and apply to primitives sometimes?) #61

Closed Prinsn closed 2 years ago

Prinsn commented 2 years ago

I'm not in the best development state to give any information here, but I'm migrating a migration from Typewriter that someone else did and am finding that their initial pass isn't sufficient for our more robust code state.

However:
For

        public static string TypescriptModelName(IType type)
        {
            type = TypeFunctions.Unwrap(type);
            string tsname = TypeFunctions.ToTypeScriptType(type);

            if (type.IsPrimitive)
                return tsname;

            if (type.IsAbstract /*&& !type.IsInterface*/)
                return $"modelDto.{tsname}{{{type.Name}}}";

            if (type.IsEnum)
                return $"modelEnums.{tsname}";

            if (tsname.ToLowerInvariant().EndsWith("dto"))
                return "modelDto." + tsname;

            return "model." + tsname;
        }
Custom.TypescriptModelName method.ReturnType

on

IQueryable<bool>

Outputs

modelDto.boolean[]{IQueryable<bool>}

If I pair IsAbstract && !IsInterface it appears to work exactly as expected.

I'm trying to get the simplest example in code of something that would be useful, I'm absolutely seeing this problem elsewhere, but looking at this example I'm more confused, because bool shouldn't be true on either abstract nor interface?

This may indicate a completely different problem than my initial expectation

Prinsn commented 2 years ago

Okay, so I figured out part of my problem, there's double wrapping so there's an outer type that I wasn't aware of.

However, I don't thinkIQueryable should be testing as an abstract interface?

NeVeSpl commented 2 years ago

It looks like from the compiler's perspective interface is an abstract type. I do not understand your problem, and the code that you provided definitely does not give the above results. Bool is a primitive type, thus TypescriptModelName will end on the first if.

public IQueryable<bool> Foo()
{
    return null;
}

public bool Foo2()
{
    return false;
}
{{- for method in class.Methods | Symbols.ThatArePublic }}
    {{ method.Name  }}  
    {{ method.ReturnType}}
    {{ Custom.TypescriptModelName method.ReturnType }}
    {{ method.ReturnType.IsAbstract }}
    {{ method.ReturnType.IsInterface }}
    {{ method.ReturnType.IsPrimitive }}
{{- end }}
Foo  
IQueryable<bool>
boolean
true
true
false
Foo2  
bool
boolean
false
false
true
Prinsn commented 2 years ago

How is it ever valid for an interface to be abstract...?

On Tue, Aug 23, 2022, 3:28 AM NeVeS @.***> wrote:

It looks like from the compiler's perspective interface is an abstract type. I do not understand your problem, and the code that you provided definitely does not give the above results. Bool is a primitive type, thus TypescriptModelName will end on the first if.

— Reply to this email directly, view it on GitHub https://github.com/NeVeSpl/NTypewriter/issues/61#issuecomment-1223663922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYR7G5WHZVMPFIDLCTP7VDV2R4QJANCNFSM57I2Z52Q . You are receiving this because you authored the thread.Message ID: @.***>

RudeySH commented 2 years ago

Just because you can't use the abstract keyword when typing an interface, that doesn't mean an interface is not abstract.

Consider this about abstract classes:

Interfaces are abstract.

If you think this is incorrect, then reflection is incorrect. The following writes "True":

Console.WriteLine(typeof(IEnumerable).IsAbstract); // True
Prinsn commented 2 years ago

I'll concede this isn't an issue of this project, but I will maintain the reflection is wrong.

It is a useless ambiguation.

On Tue, Aug 23, 2022, 8:53 AM RudeySH @.***> wrote:

Just because you can't use the abstract keyword when typing an interface, that doesn't mean an interface is not abstract.

Consider this about abstract classes:

  • You can't instantiate abstract classes with new. The same is true for interfaces.
  • When inheriting from an abstract class, you have to implement all abstract methods. This is similar to interfaces, when you implement an interface you have to implement all methods.

Interfaces are abstract.

If you think this is incorrect, then reflection is incorrect. The following writes "True":

Console.WriteLine(typeof(IEnumerable).IsAbstract); // True

— Reply to this email directly, view it on GitHub https://github.com/NeVeSpl/NTypewriter/issues/61#issuecomment-1224032403, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYR7G7K2NKX6APKH6XPR43V2TCTPANCNFSM57I2Z52Q . You are receiving this because you authored the thread.Message ID: @.***>

NeVeSpl commented 2 years ago

Reflection is not wrong, it is how the C# language was designed. And you will find many more similar cases when you spend more time with static code analysis.

Prinsn commented 2 years ago

I disagree.