DotNetAnalyzers / StyleCopAnalyzers

An implementation of StyleCop rules using the .NET Compiler Platform
MIT License
2.66k stars 507 forks source link

Rule proposal: Maintainability rule - Remove unnecessary parenthesis for object initializers. #750

Open Przemyslaw-W opened 9 years ago

Przemyslaw-W commented 9 years ago
Product p = new Product() { Id= 10, Name = "Cup" }; 

should be

Product p = new Product { Id= 10, Name = "Cup" };

Originally posted by ericslattery http://stylecop.codeplex.com/workitem/6793

follow up discussion:

andyr wrote Feb 4, 2011 at 9:47 PM Jason, please review and comment. Then I'll fix/add.

andyr wrote Mar 15, 2011 at 5:32 PM From Jason, "Love it"

Tratcher wrote Jul 19, 2012 at 7:04 PM Dislike. If we must choose one or the other I'd rather require the parens than prohibit them.

Keeping the parens is more consistent with normal constructor requirements. E.g. If I already had new Products(); adding {...} should not force me to remove the parens.

Also, the parens will always be required for any constructors with parameters, so it is inconsistent to prohibit them on the parameter-less constructor.

Przemyslaw-W commented 9 years ago

This was rejected in original StyleCop and I agree with this. For the reasons stated by Tratcher above. IMHO we should add opposite rule - always use parens in ctor call.

sharwell commented 9 years ago

I don't think we can require the parentheses without it creating a noticeable contrast with the SA1410/SA1411 rules. For better or worse, StyleCop already established a pattern of preferring to omit unnecessary empty parentheses rather than include them, and I believe we should either continue to follow that pattern or simply not check this syntax.

I'm neutral regarding the rule as originally proposed, (I it makes sense to cover this case if we are already covering the other two cases, but I'm not sure it's a high-value rule). I am against the alternative proposal of requiring parentheses since it is clearly (in my view) not consistent with other StyleCop rules.

Przemyslaw-W commented 9 years ago

There is a difference though. SA1410 and SA1411 are both about parens in declarative context. Ctor call is a invocation and as such should have parenthesis to denote it. Omitting parens in ctor call feels a bit vb

2015-04-27 23:25 GMT+02:00 Sam Harwell notifications@github.com:

I don't think we can require the parentheses without it creating a noticeable contrast with the SA1410/SA1411 rules. For better or worse, StyleCop already established a pattern of preferring to omit unnecessary empty parentheses rather than include them, and I believe we should either continue to follow that pattern or simply not check this syntax.

I'm weakly in favor of the rule as originally proposed, primarily because it makes sense to cover this case if we are already covering the other two cases. I am against the alternative proposal of requiring parentheses since it is clearly (in my view) not consistent with other StyleCop rules.

— Reply to this email directly or view it on GitHub https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/750#issuecomment-96826607 .

Przemyslaw-W commented 9 years ago

Either way, if we are going to extend this project with complementary rules, then implementing both flavors has merit. The question would be which one to enable by default (if any)

sharwell commented 9 years ago

SA1411 specifically targets parentheses of an optional argument list (the only other optional argument list in the language?), as opposed to SA1410 which is a parameter list.

Przemyslaw-W commented 9 years ago

Syntactically, it is argument list, yes. Semantically, applied attribute is kinda declarative - it is not "live" code, which is invoked in runtime.

zizhengtai commented 2 years ago

I know this was from five years ago. Has anything been implemented for this? I searched through the documentation but didn't find anything.