Upgrade to 4.0.0 or 4.1.0 omits namespaces in generated code

[realbart]

This is an example of generated code in the new versions. Not that the Azure.Core namespace is missing from "TokenCredential"

//--------------------------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;

namespace MobiManagement.Web.Api
{
    [GeneratedCode("AutomaticInterface", "")]
    public partial interface IConfigurator
    {
        /// <inheritdoc />
        void ConfigureServices(IConfiguration configuration, IServiceCollection services, TokenCredential credential);

        /// <inheritdoc />
        void AddApplicationInsights(IConfiguration configuration, ILoggingBuilder loggingBuilder);

    }
}

In the old version, namespaces were always included, which is perfectly fine (and in this case better) for generated code. (you might even consider putting the "global::" prefix before them, preventing problems in some edge cases)

//--------------------------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;

namespace MobiManagement.Web.Api
{
    [GeneratedCode("AutomaticInterface", "")]
    public partial interface IConfigurator
    {
        /// <inheritdoc />
        void ConfigureServices(Microsoft.Extensions.Configuration.IConfiguration configuration, Microsoft.Extensions.DependencyInjection.IServiceCollection services, Azure.Core.TokenCredential credential);

        /// <inheritdoc />
        void AddApplicationInsights(Microsoft.Extensions.Configuration.IConfiguration configuration, Microsoft.Extensions.Logging.ILoggingBuilder loggingBuilder);

    }
}

[ChristianSauer]

Normally, it should include the used namespaces in the usings section? I intentionally reworked that part because adding all type qualifiers made the generated code unreadable and in turn made testing annoying and hard. Any idea?

[realbart]

I understand code becomes less readable, but usually readability is not required for generated code.

Of course, you have full control over the namespaces used by your test classes. So if you test with very short namespace names, the generated code may become more practical?

Also, at this point in time I think you don’t include the assembly aliashttps://jeanarjean.com/blog/2021-03-10-how-to-create-alias-property-in-your-csproj/ (default “global::”) You may want to consider including that in your final product, as it covers additional edge cases.

Thanks for creating this library. I just thought up the concept and thought “somebody is bound to have thought of this before me” I don’t have any experience in contributing to OSS, but I’ll try find out how I can contribute.

Bart Kemps

Van: Christian Sauer @.> Verzonden: Tuesday, 13 August, 2024 09:00 Aan: codecentric/net_automatic_interface @.> CC: Bart @.>; Author @.> Onderwerp: Re: [codecentric/net_automatic_interface] Upgrade to 4.0.0 or 4.1.0 omits namespaces in generated code (Issue #55)

Normally, it should include the used namespaces in the usings section? I intentionally reworked that part because adding all type qualifiers made the generated code unreadable and in turn made testing annoying and hard. Any idea?

— Reply to this email directly, view it on GitHubhttps://github.com/codecentric/net_automatic_interface/issues/55#issuecomment-2285487145, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABZXLZIZPLMBMKYN2W5AHZ3ZRGVIZAVCNFSM6AAAAABML6FSRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBVGQ4DOMJUGU. You are receiving this because you authored the thread.Message ID: @.**@.>>

[simonmckenzie]

This broke interface generation on one of my classes - I need to investigate further, but it looks like it's because one of the method parameter types was available through an implicit global namespace in the csproj, e.g. <Using Include="System.Text.Json" />.

[realbart]

I am convinced trying to create usings at the top of the file instead of simply inlining the namespaces will unnecessarily complicate things, because you would need to check for naming collisions.

(I do not care what the generated code looks like as long as it works, by the way)

[simonmckenzie]

I was able to reproduce the "broken interface" issue using inheritance:

• Assembly 1
  • Add class with parameter resolved via a global using
• Assembly 2
  • Add class inheriting from Project 1's class, add [GenerateAutomaticInterface]

Result: the generated interface does not include the global using or fully qualify the parameter, resulting in a compile error.

note, BaseClass and Inheritor are in different assemblies

I'm inclined to @realbart's opinion that all references should just be fully qualified, with no usings at all...

[simonmckenzie]

I can see other cases that also fail, where "using aliases" break the interface generation:

using ModelAlias = RootNamespace.Models.Model;

namespace RootNamespace
{
    namespace Models
    {
        public class Model;
    }

    [GenerateAutomaticInterface]
    public class ModelManager
    {
        public ModelAlias GetModel() => null!;
    }
}

Clearly this can be fixed, but it's just another edge case that the code would have to handle.

Thinking back to WinForms development, the .Designer.cs files also used fully qualified references, possibly for similar reasons.

[simonmckenzie]

And one last failing case - using statements inside namespaces also aren't handled:

namespace RootNamespace
{
    namespace Models
    {
        public class Model;
    }

    namespace ModelManager
    {
        using Models;

        [GenerateAutomaticInterface]
        public class ModelManager
        {
            public Model GetModel() => null!;
        }
    }
}

[simonmckenzie]

I would be happy to work on a fix for this (removal of all "harvested" usings, fully qualify all type references) if you agree with the approach @ChristianSauer. I don't want to take it from you @realbart if you're keen to work on this too.

[ChristianSauer]

@simonmckenzie I would be very glad. I am currently busy with work and private things. Could probably work on that fix friday at the earliest

[simonmckenzie]

I will start on the change tomorrow @ChristianSauer. I expect the changes will cause conflicts with #58, so once one is merged, I will rebase the other.

[simonmckenzie]

I have created a PR for this change @ChristianSauer. Please tell me what you think. #60

[ChristianSauer]

I have merged the PR as 5.0.0 Thank you all