Proposal: Implement compiler attributes to allow constructor property / field declarations a la typescript

[williamBurgeson]

I note the issue #1087 however I have a slightly different angle.

See the fairly standard typescript declaration below:

class MyClass {
  constructor(private _myDep: MyDependency) { }
}

In typescript, putting an access modifier on the argument will make the param a class member (OK, in typescript the best practice isn't as strict as C# with regards to properties and fields, however leave that aside for the time being).

What about a new Attribute which is picked up by the compiler which defines a constructor param as being a property or field, public or private?

eg

public class MyClass
{
  public MyClass([ConstructorField] MyDependency _myDep) { }
}

I would suggest 2 attributes: [ConstructorField] and [ConstructorProperty].

They could also have arguments which would define the visibility: public, private, protected etc, which would just be an enum: [ConstructorProperty(MemberVisibility.Protected)]

The default visibility for [ConstructorField] would be MemberVisibility.Private, and for [ConstructorField] it would be MemberVisibility.Public.

As far as I can consider it, the [ConstructorProperty] would simply create a simple get/set property in the manner with which we are already familiar.

I'm certainly not precious about the exact name of the enum, or for that matter the exact syntax for this feature, however moving between C# and typescript, C# does feel very noisy now without a feature similar to this.

The rest is up for discussion....

[MkazemAkhgary]

hmmm, I haven't seen any attribute with that power, magically declaring fields. how that identifier is going to be handled by Roslyn syntax analyzer? it maybe possible though but Records will do better job at keeping these stuff short and concise.

[williamBurgeson]

@MkazemAkhgary I think the AttributeUsageAttribute is an example of an attribute which has an effect on the actual compilability of the code. In that case if you are creating a custom attribute class and you do not define (or use) the AttributeUsage correctly you will get a compiler (at least I think so, it's been a while since I wrote code like that).

The under-used CodeContracts feature can also enforce compiler-correctness on code based essentially on imperative statements, and uses the rewriting functionality underneath.

Obviously in terms of usage it's syntactic sugar, so the actual IDE would need to support this language feature - the IL would just be the same as if the example I gave above had been written out long-hand:

public class MyClass
{
  private MyDependency _myDep;
  public MyClass(MyDependency _myDep) 
  { 
    this._myDep = _myDep;
  }
}

I don't see why, immediately upon having added the given parameter to the constructor, intellisense couldn't internally construct things in the way I have shown here and any call to members of _myDep would work and be validated as normal.

I have to admit I haven't used a lot of the add-ons such as StyleCop and Resharper (I have used them a little bit, but not a lot), so there may be work to do on their side but I don't think it's impossible.

[iam3yal]

@williamBurgeson Have you looked at #39?

[williamBurgeson]

Thanks @eyalsk I have just skimmed down that thread, and also the more concrete proposal at https://github.com/dotnet/csharplang/blob/master/proposals/records.md

Having played about a bit with F# in the past it looked similar to that feature (I have to confess I did just skim down both pages) however this proposal is not so much about bringing a completely new feature into the language, but rather adding a slightly easier way of doing what many people have been doing in C#.NET at least since 2008/9 when IoC/DI became popular with the introduction of MVC (not that many hadn't been doing DI before that of course).

The IL would probably look the same as how I suggested in the reply to @MkazemAkhgary , and it is entirely possible that in decompiling with eg DotPeek there would be no difference as if the code had been done the current way.

As I say, when you're going between typescript and C# it really does scream out as noisy boilerplate code that you effectively need 3 times as much code to accept constructor parameters which will simply be assigned into private DI fields. If you've got say 6 such params, that gives 12 extra lines of code (not including the blank lines between sections to keep it readable and tidy)

[MkazemAkhgary]

just FYI you can always use regions which will give you ability to collapse or expand part of code. I tend to use them for medium or bigger sized classes.

#region my fields
    private X _x;
    private Y _y;
    private Z _z;
#endregion

after you collapse them, only thing you see in editor will be

#region my fields

with a + sign on left side so you can expand it again.

[williamBurgeson]

@MkazemAkhgary ha yes I did think of that - but if there's a way to avoid that that would be better!

In terms of code cleanliness I might compare it to the shorthand get/set properties which were introduced in C# 3. It must be better, IMO, to be able to replace 3 lines of code with 1.

No doubt there may be things which actually prevent this from being put into practice, however I hope the dev team will consider this feature.

[iam3yal]

@williamBurgeson

Having played about a bit with F# in the past it looked similar to that feature (I have to confess I did just skim down both pages) however this proposal is not so much about bringing a completely new feature into the language, but rather adding a slightly easier way of doing what many people have been doing in C#.NET at least since 2008/9 when IoC/DI became popular with the introduction of MVC (not that many hadn't been doing DI before that of course).

You don't want to introduce a new feature but a new feature is going to be introduced that covers this proposal and more.

Not to mention that at some point in the future, probably far future, source generation is yet another feature that can cover this proposal and many other use-cases so there's this too.

[MkazemAkhgary]

also in VS 17 when you type public MyClass(MyDependency _myDep a lightbulb comes up. click on lightbulb or press control plus dot keys to get completion suggestion. simply press enter or click create and initialize property to get the completion on fly. you can go ahead and type other parameters as well and initialize them in this way.

[williamBurgeson]

It's not so much to save typing, but also a way to emit less code.

It might be a purist approach, and obviously we can't go back and change C# from the beginning of the century when it was created, but I can't help thinking that features like this such as code completion are actually working around a problem which could be resolved with a more expressive feature such as this one.

IMO it's a code smell inherent in the language that 3 lines of code are required for each dependency. Like most new features in a dev language, nobody notices it missing until they see it working well in a different language. @MkazemAkhgary and @eyalsk, are you familiar with the suggested feature in typescript?

[iam3yal]

@williamBurgeson Yes and Kotlin has this feature too.

[HaloFour]

@eyalsk

Through primary constructors, yes.

[iam3yal]

@williamBurgeson

but I can't help thinking that features like this such as code completion are actually working around a problem which could be resolved with a more expressive feature such as this one.

You really think that adding attributes is more expressive?

IMO it's a code smell inherent in the language that 3 lines of code are required for each dependency.

Just because you can't express this in the language doesn't make it a code smell.

Like most new features in a dev language, nobody notices it missing until they see it working well in a different language.

I'm sure @MadsTorgersen speaks to Anders Hejlsberg and the design teams at Microsoft share ideas, it's an assumption but one that I can bet on.

[williamBurgeson]

Well remind me to buy you a pint if they do put it in for C# 9 :-)

[iam3yal]

@HaloFour Yes, I think that adding this to secondary constructors is going too far.

[Pzixel]

See #1087 .

@MkazemAkhgary it's more a question about maintaining such a method than write it. It's easy to write, but hard to read. For example compare:

internal sealed class DocumentDbBasketRepository : IBasketRepository
{
	private readonly IDocumentDbBasketMapper _domainToDataDocumentDbBasketMapper;
	private readonly DocumentClient _basketClient;
	private readonly DocumentDBContextInfo _documentDbContextInfo;
	private readonly IBasketOwnerMapper _basketOwnerMapper;
	private readonly IBasketItemMapper _basketItemMapperToDomain;
	private readonly IGetProductImageUrlsQueryHandler _getProductImageUrlsQueryHandler;

	internal DocumentDbBasketRepository(
		DocumentClient basketClient,
		DocumentDBContextInfo documentDbContextInfo,
		IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
		IBasketOwnerMapper basketOwnerMapper,
		IBasketItemMapper basketItemMapperToDomain,
		IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler)
	{
		_basketClient = basketClient;
		_domainToDataDocumentDbBasketMapper = domainToDataDocumentDbBasketMapper;
		_basketOwnerMapper = basketOwnerMapper;
		_basketItemMapperToDomain = basketItemMapperToDomain;
		_getProductImageUrlsQueryHandler = getProductImageUrlsQueryHandler;
		_documentDbContextInfo = documentDbContextInfo;
	}

	// Methods removed
}	

and

internal sealed class DocumentDbBasketRepository : IBasketRepository
   with constructor(
		DocumentClient basketClient,
		DocumentDBContextInfo documentDbContextInfo,
		IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
		IBasketOwnerMapper basketOwnerMapper,
		IBasketItemMapper basketItemMapperToDomain,
		IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler)
{        
}

[williamBurgeson]

@Pzixel the way you have described is possibly more C-sharpy, one thing my suggestion has its favour is that there is potentially more flexibility - as I said, [ConstructorField] can default to being private, and [ConstructorProperty] can default to being public, but within there I can't see why with parameterisation of the attributes you couldn't create, say, a property which has a public getter but a private setter, and for that matter for both properties and fields optionally set them to be readonly (I can't recall ever seeing a constructor param which was not read-only in terms of usage unless it was a particularly special case).
But we are in agreement that the current state of things is not ideal.

[HaloFour]

This is something that could be handled by the mythical source generators. The generator could emit a partial class with the fields/properties and replacement constructors to populate them.

[alrz]

This is something that could be handled by the mythical source generators.

Writing such tool is not hard with Roslyn now. Code generators usually use a file format like xml for input, one could parse a C# file using roslyn and generate boilerplate code. easy peasy.