SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
887 stars 46 forks source link

Vogen 5.0.5 Moved `MapVogenTypes()` namespace #711

Open viceroypenguin opened 3 days ago

viceroypenguin commented 3 days ago

Describe the bug

Under Vogen 5.0.5, the VogenSwashbuckleExtensions is located in a namespace that matches the project name, instead of the RootNamespace from the .csproj.

Steps to reproduce

  1. Create a project
  2. Change the <RootNamespace /> in the csproj
  3. Note the difference in the namespace the VogenSwashbuckleExtensions type is located in

Expected behaviour

VogenSwashbuckleExtensions should be located in the RootNamespace

SteveDunn commented 3 days ago

Thanks for the report. I think the namespace never used to be emitted. I'm not sure what 'default' namespace is used if none is specified. So I'm not sure if this is a bug or just 'different'

viceroypenguin commented 3 days ago

I'm not sure what 'default' namespace is used if none is specified

This would end up being in the global:: namespace, which is the global namespace without any names. Perhaps it is just "different"; I didn't realize it was in the global namespace before. However, the change is meaningful, since my project has a name of Api and a root namespace of VsaTemplate.Api, which means that I have to add a using that is a) unexpected and b) non-standard.

SteveDunn commented 3 days ago

I think you're right, it should use the default namespace specified in the project. I'll take a look at this shortly.

SteveDunn commented 23 hours ago

I'll revert this change. The driver for this generated code is from an assembly level attribute, so it makes sense to use the global namespace (i.e. don't wrap in a namespace).

Actually, no. I remember why I added this now. It was to prevent naming collisions when Swashbuckle extensions were included in two or more projects. The Consumers solution demonstrates this. It has a webapi project which references a shared project. Both of these projects are configured to generate a Swashbuckle extension method. Without namespaces, they'd both be in the global namespace, which causes difficulties. With namespaces, they can be used like this:

    WebApplication.VogenSwashbuckleExtensions.MapVogenTypes(opt);
    WebApplication.Shared.VogenSwashbuckleExtensions.MapVogenTypes(opt);

When both of the extension methods are generated in the default namespace, it's more difficult to access them, e.g. without the namespaces, and without qualifying each with a global using alias, the user sees errors such as this:

6>Program.cs(33,5): Error CS0436 : The type 'VogenSwashbuckleExtensions' in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs' conflicts with the imported type 'VogenSwashbuckleExtensions' in 'WebApplication.Shared, Version=5.0.0.0, Culture=neutral, PublicKeyToken=null'. Using the type defined in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs'.
6>Program.cs(34,5): Error CS0436 : The type 'VogenSwashbuckleExtensions' in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs' conflicts with the imported type 'VogenSwashbuckleExtensions' in 'WebApplication.Shared, Version=5.0.0.0, Culture=neutral, PublicKeyToken=null'. Using the type defined in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs'.
SteveDunn commented 23 hours ago

since my project has a name of Api and a root namespace of VsaTemplate.Api, which means that I have to add a using that is a) unexpected and b) non-standard.

You could skip including the namespace and instead use it directly: Api.VsaTemplate.Api.MapVogenTypes(opt);

At least then, the change is isolated (but I do agree that the repeated Api in the namespaces isn't ideal.

viceroypenguin commented 22 hours ago

I'll revert this change. The driver for this generated code is from an assembly level attribute, so it makes sense to use the global namespace (i.e. don't wrap in a namespace).

Actually, no. I remember why I added this now. It was to prevent naming collisions when Swashbuckle extensions were included in two or more projects. The Consumers solution demonstrates this. It has a webapi project which references a shared project. Both of these projects are configured to generate a Swashbuckle extension method. Without namespaces, they'd both be in the global namespace, which causes difficulties. With namespaces, they can be used like this:

    WebApplication.VogenSwashbuckleExtensions.MapVogenTypes(opt);
    WebApplication.Shared.VogenSwashbuckleExtensions.MapVogenTypes(opt);

When both of the extension methods are generated in the default namespace, it's more difficult to access them, e.g. without the namespaces, and without qualifying each with a global using alias, the user sees errors such as this:

6>Program.cs(33,5): Error CS0436 : The type 'VogenSwashbuckleExtensions' in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs' conflicts with the imported type 'VogenSwashbuckleExtensions' in 'WebApplication.Shared, Version=5.0.0.0, Culture=neutral, PublicKeyToken=null'. Using the type defined in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs'.
6>Program.cs(34,5): Error CS0436 : The type 'VogenSwashbuckleExtensions' in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs' conflicts with the imported type 'VogenSwashbuckleExtensions' in 'WebApplication.Shared, Version=5.0.0.0, Culture=neutral, PublicKeyToken=null'. Using the type defined in 'E:\git\Vogen\samples\WebApplication\obj\Debug\net8.0\Vogen\Vogen.ValueObjectGenerator\SwashbuckleSchemaExtensions_g.cs'.

I approached this in my IH and IA libraries by including the assembly name as part of the method name. In the case of IH, it's Add<Assembly>Handlers(), etc. This is a pattern I've seen with other libraries as well, such as AutoRegisterInject. This avoids the issue of namespace problems.