dotnet / runtime

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

Introduce Attribute and Analyzer for better discovery of Factory-Methods #42314

Open dersia opened 4 years ago

dersia commented 4 years ago

Background and Motivation

There should be a Analyzer or Tooling support to find Facotry-Methods easier.

As a library author I am often providing facotry-methods for my types to make construction of types more comfortable for users. The Factory-Pattern is being used more and more within ASPNET and also in the BCL. With the readonly structs and the new new() syntax it is harder for developers to find the right way to contruct types. Especially with structs, that have a public empty constructor, we should make the experience better for developers to discover factory-methods

I propose to introduce an attribute, so that analyzers and/or editor(intellisense) can react to those and offer better discovery of facotry methods.

Proposed API

I propose to introduce an attribute like this:

namespace System.Diagnostics.CodeAnalysis
{
     [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
     public sealed class ConstructingMethodAttribute : Attribute
     {
         public ConstructingMethodAttribute() { }
     }
}

Usage Examples

This could then be used by a library author like this:

public readonly struct MyStruct 
{
    string Name { get; set; }
    string Value { get; set; }

    [ContructingMethod]
    public static MyStruct CreateFromKeyValuePair(KeyValuePair<string, string> Entry) {}
}

And when the caller than types var myCoolValue = new MyStruct() the analyzer would hint with blue squiggles that there is a better way to construct this type. If we have editor support for this intellisense could then already suggest the facotry-methods as soon as the user starts typing new.

Alternative Designs

It is also possible to use FactoryAttribute as the attribute name, but I think this might clash with Facotry-Method support coming in C#10. This solution can also be backported and made available for lower versions of c#, assuming factory-methods have editor support with c#10.

Risks

Older libraries and blc would need to update code to make it available, but I don't think there is any risk in this.

Dotnet-GitSync-Bot commented 4 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 4 years ago

Tagging subscribers to this area: @tommcdon, @krwq See info in area-owners.md if you want to be subscribed.

SingleAccretion commented 4 years ago

Linking #41686 as a potential use case in the BCL.

KalleOlaviNiemitalo commented 4 years ago

Instead of needing an attribute, perhaps code editors could search for static methods that have a Create or From prefix and the correct return type. I don't think analyzers could require the use of those, though; it would cause too many warnings for correct code.

<completionlist cref="…"/> in XML documentation comments is another way to tell Visual Studio how to get instances of the type. That would have the same problem in analyzers.

The proposal https://github.com/dotnet/csharplang/issues/2926 would not let MyStruct x = CreateFromKeyValuePair(kvp); bind to MyStruct.CreateFromKeyValuePair because the CreateFromKeyValuePair(kvp) expression is not an unqualified identifier.

Especially with structs, that have a public empty constructor, we should make the experience better for developers to discover factory-methods

https://github.com/dotnet/csharplang/issues/99 and https://github.com/dotnet/csharplang/issues/146 could help avoid the empty constructor.