IDE code style options should have `hidden` severity by default

[mavasani]

All the IDE code style suggestions should always be hidden severity by default, otherwise they tend to add lot of noise, both in the editor and error list.

For example, consider IDE0090 (new expression can be simplified), notice the ... and error list entry:

Default value below should be true with silent enforcement: https://github.com/dotnet/roslyn/blob/e2e303acb1fab9c8c4f811035091d436bd36a67b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/CodeStyle/CSharpCodeStyleOptions.cs#L267-L271

It seems half of the code style options have silent enforcement as the default, and half of them have suggestion enforcement, and there does not seem to any reasoning behind choosing one or the other.

Proposal: Switch all code style options to have silent enforcement as the default.

[mavasani]

Tagging @sharwell for brining up in next design meeting.

[CyrusNajmabadi]

I honestly don't think it's correct. We used suggestion for the items we thought should be on be default and visible to users. These were intentional and we picked the triple dots as the lightweight way to surface these to users by default.

[mavasani]

If so, then there are couple of problems:

1. All DiagnosticDescriptors for IDE code analyzers have hidden default severity. I believe this should match with the default severity of code style option. https://github.com/dotnet/roslyn/blob/e2e303acb1fab9c8c4f811035091d436bd36a67b/src/Analyzers/Core/Analyzers/AbstractBuiltInCodeStyleDiagnosticAnalyzer_Core.cs#L58

2. Where do we have the doc or guidelines on which rule qualifies to be a user visible suggestion vs hidden diagnostic?

[CyrusNajmabadi]

All DiagnosticDescriptors for IDE code analyzers have hidden default severity

Sure. But we set the severity with our diagnostics when we report it. So it shouldn't realy matter what the descriptor is right? THe descriptor is just there to be a template for all the values prior to any specific changes we make at report time.

Where do we have the doc or guidelines on which rule qualifies to be a user visible suggestion vs hidden diagnostic?

We discuss it during the PR. Generally it's a balance of usefulness/noise. i.e. if we expect the suggestion to be high quality with few false positives, then we enable it. This is esp. true of suggestions that move you to more modern constructs. If it's a suggestion on a default that has broad support in either direction, we generally don't have it on by default.

[mavasani]

Sure. But we set the severity with our diagnostics when we report it. So it shouldn't realy matter what the descriptor is right? THe descriptor is just there to be a template for all the values prior to any specific changes we make at report time.

No, this has user visible effects. The IDExxx diagnostic in analyzers node shows hidden as the default severity, while the analyzer reports suggestions by default. Same thing will happen when @jmarolf implements the editorconfig UX - unless he adds complex logic to crack code style options, the default severity for IDE analyzers will come from default severity in the descriptor.

IMO, descriptor is a contract from analyzer that if there is no end user configuration, the diagnostic with this ID will be reported with the specified default severity in the descriptor, and that contract is broken here. I think all the problems in Code style space come from the fact that code style options themselves have severities, and the compiler or any other tooling has no knowledge of this, they only see descriptors from analyzers through public API surface.

[CyrusNajmabadi]

IMO, descriptor is a contract from analyzer that if there is no end user configuration, the diagnostic with this ID will be reported with the specified default severity in the descriptor, and that contract is broken here.

Yeah, that contract was never something i think anyone working on analyzers/fixers knew :) We just had a helpful subclass to prevent a lot of repetition, and we only did what was necessary to make the front-end UI look fine.

Can we have the subclass just set those descriptor defualts based on whatever is in editor_config?

[CyrusNajmabadi]

, and the compiler or any other tooling has no knowledge of this,

I guess it's completely unclear to me why the compiler or other tooling would know or care about this? We have our PerLanguageOptions and whatnot. They encode the defaults here. Can we not just drive our experiences off of that? I thought that's why we had those types, and had them specify things like the editor_config storage keys.

[allisonchou]

Notes from today's design meeting (10/26): By design / Manish plans on making whatever tweaks are needed.

[DataJuggler]

Why am I still getting these messages? How do I get rid of this for every new expression in my project, and all projects?

[CyrusNajmabadi]

@DataJuggler you can disable any analyzers you don't want on by default.

and all projects?

Have a top level editorconfig file and it will affect all projects below it.

[DataJuggler]

By all projects, I mean every project I have ever created, or will create in the future.

I figure out how to put the codes in the Build to ignore, but I never asked for this warnings, and most are stupid.

Thanks for the reply, I wish you had to opt into these warnings, rather than just show them.

[CyrusNajmabadi]

By all projects, I mean every project I have ever created, or will create in the future.

Yes. An editorconfig in the root directory of all your projects (now and for the future) should do this.

Thanks for the reply, I wish you had to opt into these warnings, rather than just show them.

I'm not sure what you're referring to here. I don't believe the ide has enabled any warnings by default.

[CyrusNajmabadi]

I never asked for this warnings,

New products come with new features. We make them available on by default as they are used by the majority.

and most are stupid.

Can you give examples of what you mean? If the features aren't doing things well, we'd like to improve them.

[DataJuggler]

I get warnings on all my projects, ‘new initializer can be simplified’, and something to do with setting a variable to a null, which I have done for 20 years in C#, now gets a message something like ‘setting a value is unnecessary.’

I just want a ‘Never show a again’, or better yet, never show them.

I have to add this to a project to not get the messages:

1701;1702;IDE0090;IDE0059;IDE0017;

Different projects require more.

Rosyln has a major bug since 2015, that one of your engineers verified, and 2013 VS didn’t have it, so I have it to hit tab one space forward because Rosyln compiler can’t figure out Regions are indented by one.

But that’s another story, and the price of Visual Studio community is reasonable, so I don’t complain (much).

[CyrusNajmabadi]

I get warnings on all my projects, ‘new initializer can be simplified’

There should be no warning here. Just a message that the firm you are using can be simplified.

[CyrusNajmabadi]

now gets a message something like ‘setting a value is unnecessary.’

Sure. The message is telling you that you are writing additional code that isn't necessary.

This sort of message is there as some people don't realize this and write code that isn't needed.

As mentioned above though, it's easy to disable this for all your projects.

[CyrusNajmabadi]

I just want a ‘Never show a again’, or better yet, never show them.

As mentioned above, this is possible with editorconfig.

[CyrusNajmabadi]

Rosyln has a major bug since 2015, that one of your engineers verified, ... so I have it to hit tab one space forward because Rosyln compiler can’t figure out Regions are indented by one.

I don't know what this is referring to. If you're having an issue with regions and indentation, please open an issue on it. Otherwise I don't believe anything would change here.

and 2013 VS didn’t have it,

We'll need more info. For all I know the was buggy behavior that may have been fixed. A complete big report with a repro will allow us to see. Thanks!

[sharwell]

Rosyln has a major bug since 2015, that one of your engineers verified, ... so I have it to hit tab one space forward because Rosyln compiler can’t figure out Regions are indented by one.

I don't know what this is referring to. If you're having an issue with regions and indentation, please open an issue on it. Otherwise I don't believe anything would change here.

and 2013 VS didn’t have it,

We'll need more info. For all I know the was buggy behavior that may have been fixed. A complete big report with a repro will allow us to see. Thanks!

This is a reference to https://github.com/dotnet/roslyn/issues/50063. The formatter doesn't (and never did) support the formatting used by the project in question, but it's a bug that the smart indent algorithm is affected by the layout of an unrelated section of the file. Correct indentation underneath if, try, etc. that appears as the first token on a line, should be determined solely by the placement of the if/try/etc. keyword, and not by any other code preceding it.

[levicki]

@CyrusNajmabadi

Sorry to necro this issue, but I just wanted you to reflect on what you wrote here in one of your responses and try to clarify that both for yourself and me because frankly, I am confused.

New products come with new features. We make them available on by default as they are used by the majority.

If a feature did not exist until it was added and made on by default, then how can you claim it is used by the majority and cite that as a reason to make it on by default? What kind of circular logic is that?

The way all Microsoft product engineering teams seem to be introducing new features is a recurring pattern which goes like this:

1. Add a new feature (that often cannot be disabled) and make it a new default
2. People complain about it and ask for a way to disable it (if not possible yet) or for a way to change the default
3. Microsoft ignores customer feedback claiming that not enough people voted for it

The problem with that approach is two-pronged:

1. All usage telemetry derived from this way of introducing a new feature is useless and should be discarded -- if new feature is a new default of course everyone is going to use it at least once (or forever if it cannot be disabled).
2. Customer feedback does not seem to be aggregated (multiple complaints for the same problem with different wording, reports in different languages, etc) so the critical treshold is extremely hard to reach for less "popular" complaints.

There is a simple and effective way to make a new feature discoverable -- you show a dialog to the user on first launch.

A mock of such dialog:

Foobar Decombobulation

We made decombobulation of foobars easier for people with disabilities.
You can now do it by pressing Ctrl+D instead of LeftAlt+RightShift+Numpad0 while foobar is selected.

[Turn on] [Leave off] [Change later in Settings]

Clicking close button or using keyboard shortcuts (Alt+F4) to close the dialog should also count as clicking [Change later in Settings].

That way you:

• Inform the user what the new feature does, making it truly discoverable without forcing it on
• Get accurate usage telemetry -- you can discover how many users really wanted the feature (those who clicked [Turn on]), how many users didn't want the feature (those who clicked [Leave off]), and how many users didn't want to decide right away (those who thought it wasn't important enough).
• For those who didn't want to decide right away, you can chart an adoption trend to help decide whether the feature should be made a new default or not

I sincerely wish we the customers had more agency when it came to new features and new defaults and that goes for all Microsoft products (Teams, Office, Windows, Visual Studio). Sadly, my thoughts on this subject can only reach individual developers or teams, never someone higher up the corporate ladder who could replace Microsoft's arrogant and forceful "we know what's best for you / our way or the highway" attitude towards customers when it comes to introducing new features with a more sensible and, dare I say, more tolerant and inclusive approach which I just described above.

[CyrusNajmabadi]

if new feature is a new default of course everyone is going to use it at least once (or forever if it cannot be disabled).

This is not true. We are not counting if the feature is shown, but if the user actually invokes the feature.

[CyrusNajmabadi]

Customer feedback does not seem to be aggregated

Customer feedback is aggregated.

[DataJuggler]

Rosyln has a major bug since 2015, that one of your engineers verified, ... so I have it to hit tab one space forward because Rosyln compiler can’t figure out Regions are indented by one.

I don't know what this is referring to. If you're having an issue with regions and indentation, please open an issue on it. Otherwise I don't believe anything would change here.

and 2013 VS didn’t have it,

We'll need more info. For all I know the was buggy behavior that may have been fixed. A complete big report with a repro will allow us to see. Thanks!

I have created an issue for this, twice. It has to do with using a Methods region for example. I indent all code in the Methods region by one tab, and Roslyn compiler has never liked this. Twice you told me 'Not enough people voted for this', so I just learned how to tab over to fix it (constantly).

[CyrusNajmabadi]

@DataJuggler can you link me to the two issues you created? Searching for them showed nothing: https://github.com/dotnet/roslyn/issues?q=is%3Aissue+author%3Adatajuggler+

From your description it sounds like issues that would fall into our general bucket of wanting to create and expose an extensible formatting API. That's something that's currently a work in progress. But our hope is that it means when individuals want to do custom formatting they have an easy option available to them.

[CyrusNajmabadi]

Twice you told me 'Not enough people voted for this',

It's quite possible this is true. I'm not sure why that's inappropriate. We have limited resources and a huge user base. We're going to focus out efforts on things we feel will benefit much more people over those that benefit a tiny group (or only one person).

Afaict, nearly all software projects work this way. This is, however, one of the reasons that we are open source. Being open source means that if something is now important to you than it is for the rest of the community, you have options you can take to effect change.

[DataJuggler]

I don’t know how to find old issues. Pre Git Hub days (for me anyway). I remember once an engineer from Microsoft confirmed the bug in VS 2015, but it wasn’t in 2013 VS. The bug is still there in VS 2022.

@.***

Often for no reason at all, with all Formatting turned off, VS decides to indent the top line and region for no apparent reason:

Not sure what event fires when all formatting is turned off, but I can provide a video if needed. I will create another issue, but I suspect the same result. No one else uses regions like I have for 20 years.

[CyrusNajmabadi]

@DataJuggler Please file an issue with a repro and someone can take a look.

but I suspect the same result. No one else uses regions like I have for 20 years.

If it's a bug we would likely take a community contribution here to fix it. This is a strong benefit of open source. The team can focus on issues and features impacting high amounts of users, but individuals can also contribute to areas they deeply care about, but which are not used by anyone else.

[levicki]

@CyrusNajmabadi

This is not true. We are not counting if the feature is shown, but if the user actually invokes the feature.

What about all the features that change previous editor behavior and do not require explicit invocation such as Tab sometimes inserting spaces instead of Tabs because of smart formatting feature which was introduced on by default?

Or another real-world example:

1. You add a feature that shows a lightbulb next to a line of code in Visual Studio 2015
2. I see an annoying lightbulb and interract with it trying to find a way to disable it
3. You count me as "using the feature" when I actually hate it and want a global off switch

I remember how many feedback reports were for that thing, and there is still a bunch of duplicate Stack Overflow questions about it and it still cannot be disabled globally -- people went as far as to disable whole margin by writing an extension.

So much for listening to, and aggregating feedback.

I stand by my claim that Microsoft product development teams are introducing new features in a user-hostile way.

[sharwell]

@DataJuggler can you link me to the two issues you created? Searching for them showed nothing: https://github.com/dotnet/roslyn/issues?q=is%3Aissue+author%3Adatajuggler+

From your description it sounds like issues that would fall into our general bucket of wanting to create and expose an extensible formatting API. That's something that's currently a work in progress. But our hope is that it means when individuals want to do custom formatting they have an easy option available to them.

I already provided details for this in earlier comment https://github.com/dotnet/roslyn/issues/48839#issuecomment-963334884.

[CyrusNajmabadi]

What about all the features that change previous editor behavior and do not require explicit invocation such as Tab sometimes inserting spaces instead of Tabs because of smart formatting feature which was introduced on by default?

This would not count as a user explicit invocation of a feature. Explicit invocation must involve a particular non-standard invocation, and something particular about the commit behavior that again woudl not be part of typing. Hitting tab is a normal keyboard interaction just for editing a document, so it would not count as an explicit user choice to use a feature.

Note: it's also unclear what feature you're talking about here. Hitting tab to insert a tab should do that. If you're seeing it inserting spaces, please file an issue with a repro so someone can take a look.

You count me as "using the feature" when I actually hate it and want a global off switch

This would not count as using the feature. If you did happen to invoke it, it would only count as a single hit, and we look in aggregate at how many peolpe are using it repeatedly, as well as things like how often it is even used when it is shown. A single one-off accidental invocation while trying to figure out how to shut something off will not affect the data here.

people went as far as to disable whole margin by writing an extension.

yes. but looking at the extension, it has around 1k installs, for a user base of millions. The product is not, and will not, ever be fully configurable for the needs of every user who uses it. As mentioned above, we will prioritize the needs and benefit to the majority of users. However, VS is extensible to allow for things like extensions like this to allow community members to fill gaps they feel are important to them.

[CyrusNajmabadi]

I remember how many feedback reports were for that thing,

@levicki It's unclear to me what issue you're referring to. Can you link me to one of the issues you've mentioned, or provide a specific repro for the case you're concerned about?

Note: a request to disable the lightbulb menu would not be roslyn (we don't control that), it would need to be filed with VS as they own the shell here and would have to provide an option to not have this part of the UI be shown.

[CyrusNajmabadi]

I already provided details for this in earlier comment https://github.com/dotnet/roslyn/issues/48839#issuecomment-963334884.

Your linked issue is about do, but @DataJuggler mentioned:

I have created an issue for this, twice. It has to do with using a Methods region for example. I indent all code in the Methods region by one tab,

I don't see teh connection between the issues. His case appears (afaict from reading what he wrote) to be a case where he wants items within a region indented one more level.

[levicki]

@CyrusNajmabadi

Hitting tab is a normal keyboard interaction just for editing a document, so it would not count as an explicit user choice to use a feature.

Then how did you know that Use adaptive formatting feature should be on by default when it was first introduced?

Do you realize that you just admitted you didn't have any telemetry for that?

Worse yet, you didn't bother to ask the users during first Visual Studio launch after the update whether they want Use adaptive formatting feature on or off -- you just went ahead and enabled it for everyone, because you apparently don't know how to make new features discoverable except by making them a new default (and that goes for all Microsoft products, not just Visual Studio).

Hitting tab to insert a tab should do that.

Oh we agree on that one.

But in the file which has spaces it doesn't do that by design, unless you go out of your way to remember to first click an indicator on the bottom document margin to switch from spaces to tabs before you hit tab.

A single one-off accidental invocation while trying to figure out how to shut something off will not affect the data here.

But since it cannot be disabled, people who are annoyed by it will click on it again and again to see what in their code it is complaining about this time, so that won't be a one-off click anymore.

yes. but looking at the extension, it has around 1k installs, for a user base of millions.

I hope you have also read the description which says that the extension also disables quick actions?

So people can choose between giving up on quick actions, and being distracted by a sometimes flashing and potentially seizure inducing lightbulb.

The product is not, and will not, ever be fully configurable for the needs of every user who uses it.

I understand that, and I don't expect everything to be fully configurable all the time, but I question the value of adding any new feature in a state in which it cannot be controlled by the user.

The way I see it, the crux of the problem is that you want a minimum viable product (feature?) out of the door as soon as possible, and some users (me included) value the ability to turn a feature on and off above the feature itself.

After all, I am sure you wouldn't want a wall-mounted LED lamp which can't be turned off once installed, and would rather wait for the company producing the LED lamp to add a power switch to it before "updating" your old wall-mounted incadescent lamp with it.

Note: By "you" in the above text I mean "Microsoft", not you personally.

[CyrusNajmabadi]

Then how did you know that Use adaptive formatting feature should be on by default when it was first introduced?

For features that are part of the inner typing loop, this is done with user studies, long dogfooding periods. Long periods of requests for feedback from community, etc.

Note: Use adaptive formatting is not roslyn. If you have issues with that feature, please file feedback using:

[CyrusNajmabadi]

But since it cannot be disabled, people who are annoyed by it will click on it again and again to see what in their code it is complaining about this time, so that won't be a one-off click anymore.

That would still be a one-off click. We aren't marking a bit saying "this user used this" that triggers after a handful of invocations. We're tracking % of users continually using such features.

[CyrusNajmabadi]

So people can choose between giving up on quick actions, and being distracted by a sometimes flashing and potentially seizure inducing lightbulb.

This is not a roslyn issue. Please file feedback using hte system shown above if you are having a medical issue with a piece of UI.

[levicki]

For features that are part of the inner typing loop, this is done with user studies, long dogfooding periods

It would seem that those studies do not include enough diverse people.

As for dogfooding, it is hard to believe it is really happening considering what kind of stuff gets released in Microsoft's product lineup.

This is not a roslyn issue. Please file feedback using hte system shown above if you are having a medical issue with a piece of UI.

You are right, this is not a roslyn issue, it is not my medical issue, and I apologize for off-topic.

The issue is the mentality of Microsoft product managers and their methodology of deciding the new defaults and it shows in each and every issue reported on any product.

Sadly, there is no "Send feedback" button for that anywhere.

[CyrusNajmabadi]

Sadly, there is no "Send feedback" button for that anywhere.

Yes there is. I showed above where that send feedback button is.

It would seem that those studies do not include enough diverse people.

No. It just means that it's possible, as mentioned above, that a determination was still made overall that having something on by default was appropriate.

[levicki]

Yes there is. I showed above where that send feedback button is.

And in doing so you are being condescending as usual because I am very well aware of how to send feedback for Visual Studio.

What I wanted is to send feedback to Microsoft's upper management but after talking to you I am under impression that even if there was a way it would be an exercise in futility. You people are so set in your ways that you don't even want to consider other viewpoints.

[CyrusNajmabadi]

You people are so set in your ways that you don't even want to consider other viewpoints.

Feedback is always considered. But it needs to be submitted. The value of this as well is that others can vote and add their signals to the mix. And these signals do change outcomes.

What I wanted is to send feedback to Microsoft's upper management

Management is very interested in feedback and we've changed directions many times at the upper levels precisely due to feedback.

My recommendation remains the same here. If you want to see changes in these features, get the product teams to notice, or get management aware of these issues, the best and definitely viable path is filling feedback and having other community members do the same (or have them participate on your feedback items).

Wrt Roslyn, if you find issues with us, please file feedback here. We genuinely fix tons of user reported issues, and we work with community members when they are interested in fixing this super important to themselves.

The only thing that doesn't work though is not participating.