dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.04k stars 4.68k forks source link

[API Proposal]: GeneratedCodeAttribute should have additional constructors #108443

Open baywet opened 2 hours ago

baywet commented 2 hours ago

Background and motivation

When applying a GeneratedCodeAttribute, the generator might not want to set the tool name or version. The version is especially challenging when the generation tool gets updated, generated code refreshed. Now all of a sudden all generated files produce a diff, even if they have no other changes. This class currently has single constructor where the parameters are nullable. And it does not perform any validation on the passed values. Nor does it use them afterwards beyond exposing them as properties. Yes a generator could pass null for the version, but this is not elegant and slows down compilation for large code bases (one more symbol to link * n attributes)

My suggestion is to either make the parameters optional or to add two more constructors (one without parameters, one with just the name)

API Proposal

namespace System.CodeDom.Compiler;

[AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = false)]
public sealed class GeneratedCodeAttribute : Attribute
{
    //This constructor exists like that today
    public GeneratedCodeAttribute(string? tool, string? version)
    {} //omitted for brevity
    // suggesting to update to this
    public GeneratedCodeAttribute(string? tool = null, string? version = null)
    {} //omitted for brevity 
}

API Usage

[GeneratedCode]
public class Foo
{}

Alternative Designs

Overload constructors like this

namespace System.CodeDom.Compiler;

[AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = false)]
public sealed class GeneratedCodeAttribute : Attribute
{
    //This constructor exists like that today
    public GeneratedCodeAttribute(string? tool, string? version)
    {} //omitted for brevity
    // suggesting to add these
    public GeneratedCodeAttribute(string? tool):this(tool, null) {}
    public GeneratedCodeAttribute():this(null, null) {}
}

Risks

none I can think of.

dotnet-policy-service[bot] commented 2 hours ago

Tagging subscribers to this area: @dotnet/area-system-codedom See info in area-owners.md if you want to be subscribed.

CyrusNajmabadi commented 2 hours ago

Yes a generator could pass null for the version, but this is not elegant and slows down compilation for large code bases (one more symbol to link * n attributes)

I don't see why elegance matters here. It's generated code. There's also no slowdown here. The compiler is going to be faster going through the route where two literals are passed to required arguments vs no arguments passed to optional arguments.

baywet commented 2 hours ago

Thank you for the additional information.

So the advice here would be to pass null and move on?