dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.05k stars 4.04k forks source link

Support enforcing "Code Style" in the IDE #7071

Closed Pilchie closed 7 years ago

Pilchie commented 8 years ago
sharwell commented 8 years ago

I think the Roslyn (referring to this repository) should only be responsible for a subset of these, plus one you didn't mention:

This would allow the analyzers themselves to be implemented and distributed independently, and developed at a faster rate in a lighter-weight environment.

I believe that moving the analyzers themselves out of this repository and in general not giving them "special treatment" that doesn't apply to other analyzers will substantially benefit the entire analyzer ecosystem. And if you're already rewriting the formatter as a set of analyzers, I doubt you'll ever have a better time to do so. :smile:

Pilchie commented 8 years ago

For your third bullet we had tentatively decided not to support it, and instead offer something more like VSCode's configuration experience - show a readonly view of our default .editorconfig file and allow you to override it per-user.

There are definitely tradeoffs to moving the rest of the work outside of the Roslyn repo - we'd like to bring this into the box as part of the default experience, and that's MUCH harder if we don't have everything packaged together. That isn't to say that we don't want to re-use a bunch of code, but we need to figure out the right approach to that.

Regarding rewriting the formatter as a bunch of analyzers, that's unlikely. Generally the formatting code is VERY performance sensitive, even when it has to make thousands of changes, and we're unlikely to be able to achieve that as a set of independent code fixes as opposed to the rewriter we have today. Additionally, we're not sure that the level of granularity StyleCop exposes around whitespace formatting (and distinct error codes, etc) are actually beneficial to anyone, so we were thinking of consolidating it into a simpler "Your code's whitespace is not consistent" sort of diagnostic, the fix for which would be to run the formatting engine.

AdamSpeight2008 commented 8 years ago

Is there a way to bring the tab spacing options into this, that is to make them solution specific. I personally like 2 space indentation, this project uses 4 space indentation.

Pilchie commented 8 years ago

@AdamSpeight2008 - yes, we'd want to solve that, likely as part of #7060.

sharwell commented 8 years ago

Additionally, we're not sure that the level of granularity StyleCop exposes around whitespace formatting (and distinct error codes, etc) are actually beneficial to anyone, so we were thinking of consolidating it into a simpler "Your code's whitespace is not consistent" sort of diagnostic, the fix for which would be to run the formatting engine.

Note that I wasn't saying StyleCopAnalyzers is necessarily the location for these, or that you should take the same rules. The takeaway from that experience with respect to this issue is the fact that we've been able to make rapid progress on the features while working in a small code base, and get these changes out to a reasonably large number of users without too much trouble. The analyzers mentioned in this issue are similar - they involve supporting a wide variety of strongly-held individual opinions. I believe they would benefit from the same type of development and distribution effort.

alrz commented 8 years ago

I'm not sure if this is relevant to this issue but it would be nice that code formatter preserve this style

using( ... ) try {
  ...
} catch {
 ...
}
Pilchie commented 8 years ago

@alrz That's not a style I've ever seen used before, and it's not something we have been tracking. Do you know of any codebases that actually use this style?

alrz commented 8 years ago

@Pilchie Probably it was Java, Java IDEs preserve this style. (I don't remember where I saw it though)

mkrueger commented 8 years ago

The roslyn formatter is very inflexible. However it's enough to support the allman code style and most K&R variants. So it is ok.

If you're really want to improve that and searching for ideas. I can recommend looking at Resharper or Jetbrains IDEA for a good set of formatting options. Or looking at the old NRefactory 5 option set:

https://github.com/icsharpcode/NRefactory/blob/master/ICSharpCode.NRefactory.CSharp/Formatter/CSharpFormattingOptions.cs

If it's worth implementing it - I don't know :)

+1 for making the options more fine grained & making the option API more usable.

Pilchie commented 8 years ago

Moving this to 2.0 (Preview) to continue working on it.

alrz commented 8 years ago

An option in "formatting" category to break method parameter list (or any other code if possible) to multiple lines (one per param) when it's longer than 80 chars would be great.

lorcanmooney commented 7 years ago

I see #3798 (Options can be populated from .editorconfig files), but will there be support for going the other way - exporting our current style preferences into an .editorconfig file?

Pilchie commented 7 years ago

We want to do that, but we're not sure we'll get there before RTM?

jmarolf commented 7 years ago

@Pilchie can we track this as a project instead of an issue?

Pilchie commented 7 years ago

I still haven't looked into "Projects", but it's on my todo list. agree that this isn't a great system.

Pilchie commented 7 years ago

Okay - I was going to triage and add more items to this list, and decided to just switch now. @kuhlenh Take a look at https://github.com/dotnet/roslyn/projects/4.